Re: [PATCH 1/9] csum-file: drop discard_hashfile()

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jul 02, 2026 at 05:06:01PM -0400, Jeff King wrote:
> On Thu, Jul 02, 2026 at 11:19:04AM -0700, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > So now we have two functions, discard_hashfile() and free_hashfile(),
> > > and we only need one. Which one do we want to keep?
> > >
> > > The only difference between them is that the discard variant also closes
> > > the descriptors held in the struct. Let's look at the three callers:
> > > ...
> > > Note that I said "descriptors" plural above. Those callers all care
> > > about the "fd" member of the struct. But discard_hashfile() also closes
> > > check_fd. That is only used if the struct is initialized with
> > > hashfd_check(), and neither of its two callers call either discard or
> > > free (they always "finalize" instead). So closing it is irrelevant for
> > > the current callers.
> > >
> > > I think we're better off sticking with the simpler free_hashfile()
> > > interface, and the handful of callers can decide how to handle the
> > > descriptors themselves.
> > 
> > Sonds good.
> > 
> > Our resident naming czar (already Cc'ed) may have preference about
> > the names and word order, though ;-)
> 
> Heh, yes, it should be hashfile_free() but that would require changing
> the whole interface. We could do that on top, which might also be a good
> time to do s/free/discard/ without worrying about a subtle behavior
> change.

Heh :P

I think this being called a "free" function makes perfect sense, because
ultimately that's all we do here. So the semantics align with other free
functions.

We could of course fix the ordering while at it, but I don't want to
tack that onto this series. It already makes the codebase a better
place, so I'm happy enough.

Patrick