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:

quoted

Jeff King [off-list ref] writes:

quoted

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