Re: [PATCH v2 2/3] object-file: emit corruption errors when detected

From: Jonathan Tan <jonathantanmy@google.com>
Date: Dec 7, 2022, 23:26 UTC
Message-ID: <20221207232623.1439026-1-jonathantanmy@google.com>
In-Reply-To: <221207.86pmcva2s8.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Yeah, I think that's even better, although...

[snip]

Show 15 quoted lines

> > It's also kind of weird that map_loose_object_1() is a noop on a
> > negative descriptor. That technically makes this correct, but I think it
> > would be much less surprising to always take a valid descriptor, and
> > this code should do:
> >
> >   if (fd)
> > 	return -1;
> >   return map_loose_object_1(r, path, fd, size);
> >
> > If we are going to make map_loose_object_1() less confusing (and I think
> > that is worth doing), let's go all the way.
> 
> ...maybe we should go further in the other direction. I.e. with my
> earlier suggestion we're left with the mess that the "fd" ownership
> isn't clear.

With Peff's suggestion I think we can make the function always close the fd. If we find it not to be clear, we can rename the function to ..._close_fd() or something like that.

Thanks to both of you for your suggestions.