Re: [PATCH] csum-file: introduce discard_hashfile()

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Jul 26, 2024 at 12:42:16AM -0400, Jeff King wrote:
>> On Thu, Jul 25, 2024 at 04:07:28PM -0700, Junio C Hamano wrote:
>> 
>> > Introduce discard_hashfile() API function to allow them to release
>> > the resources held by a hashfile structure the callers want to
>> > dispose of, and use that in read-cache.c:do_write_index(), which is
>> > a central place that writes the index file.
>> 
>> Nicely explained, and the patch looks good to me.
>> 
>> A few small comments (that probably do not need any changes):
>> 
>> > +void discard_hashfile(struct hashfile *f)
>> > +{
>> > +	if (0 <= f->check_fd)
>> > +		close(f->check_fd);
>> > +	if (0 <= f->fd)
>> > +		close(f->fd);
>> > +	free_hashfile(f);
>> > +}
>
> Are we sure that this is always correct? A valid file descriptor may
> have a zero value, and we wouldn't end up closing it here.

I thought that these two fd members use -1 for their "zero value"
for that exact reason.