Thread (35 messages) 35 messages, 4 authors, 3d ago

Re: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions

From: Jeff King <hidden>
Date: 2026-07-07 20:10:47

On Tue, Jul 07, 2026 at 09:33:06AM -0700, Junio C Hamano wrote:
Among the four we see here, I agree that calling _clone and _update
on an already discarded or finalized context should be caught as an
error. As I alluded to earlier, though, I am not sure about
_final. The asymmetry in a design that allows _discard after _final
but not _final after _final disturbs me slightly, but perhaps that
is only because my morning caffeine has not yet kicked in. 
There was more discussion in the earlier thread:

  https://lore.kernel.org/git/20260706000105.GA2301945@coredump.intra.peff.net/ (local)

But basically the asymmetry comes from the fact that the finalize is
trying to _do_ something, whereas discard is just, well, discarding.

So what should:

  git_hash_discard(&ctx);
  git_hash_finalize(result, &ctx);

put into result? It is probably one of:

  1. the null hash

  2. the hash you get from init() + no updates + final()

  3. nothing, BUG() instead

It seems nice at first that (1) or (2) won't cause the program to crash,
but ultimately they are probably the sign of a bug in the program. So
complaining loudly via BUG() is probably our best bet. We could always
loosen it later if somebody actually adds code where another behavior
makes sense (we know there are not such paths now, as they'd segfault
under openssl's heap-based backend).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help