Re: [PATCH] csum-file: introduce discard_hashfile()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 26, 2024, 15:36 UTC
- Message-ID
- <xmqqplr0xj0u.fsf@gitster.g>
- In-Reply-To
- <ZqORDmTj4GT0Eeqq@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 23 quoted lines
> 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.