[PATCH v2 0/7] git_hash_*() quality-of-life improvements

[Jeff King <peff@peff.net>]

On Tue, Jul 07, 2026 at 12:55:57AM -0400, Jeff King wrote:

> This implements the "idempotent git_hash_discard()" discussed in this
> subthread:
> 
>   https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/
> 
> with associated cleanups.

Here's a v2 addressing the comments so far. Mostly minor changes:

  - fixed typos noticed by Patrick

  - dropped extra braces added by coccinelle

  - dropped a trailing blank line from patch 1 (this gets fixed in a
    later patch as we add more content after the blank line, but I
    noticed "git apply" complaining)

  - a bit more explanation in patch 7 about why we don't support
    idempotent final() calls

Patch list below, followed by range diff.

  [1/7]: hash: use git_hash_init() consistently
  [2/7]: hash: convert remaining direct function calls
  [3/7]: hash: document function pointers and wrappers
  [4/7]: hash: make git_hash_discard() idempotent
  [5/7]: csum-file: use idempotent git_hash_discard()
  [6/7]: http: use idempotent git_hash_discard()
  [7/7]: hash: check ctx->active flag in all wrapper functions

 builtin/fast-import.c       |  4 +--
 builtin/index-pack.c        |  6 ++--
 builtin/patch-id.c          |  2 +-
 builtin/receive-pack.c      |  6 ++--
 builtin/submodule--helper.c | 10 +++---
 builtin/unpack-objects.c    |  4 +--
 csum-file.c                 | 23 +++++---------
 diff.c                      |  4 +--
 hash.c                      | 16 ++++++++++
 hash.h                      | 44 +++++++++++++++++++-------
 http-push.c                 |  2 +-
 http.c                      |  9 ++----
 http.h                      |  1 -
 object-file.c               | 14 ++++-----
 pack-check.c                |  2 +-
 pack-write.c                |  6 ++--
 read-cache.c                |  6 ++--
 rerere.c                    |  2 +-
 t/helper/test-hash-speed.c  |  2 +-
 t/helper/test-hash.c        |  2 +-
 t/helper/test-synthesize.c  | 33 ++++++++++---------
 t/unit-tests/u-hash.c       |  2 +-
 tools/coccinelle/hash.cocci | 63 +++++++++++++++++++++++++++++++++++++
 trace2/tr2_sid.c            |  2 +-
 24 files changed, 177 insertions(+), 88 deletions(-)
 create mode 100644 tools/coccinelle/hash.cocci


1:  2f1c8cbc98 ! 1:  911cf0dfcd hash: use git_hash_init() consistently
    @@ object-file.c: static int start_loose_object_common(struct odb_source_loose *loo
      	stream->next_out = buf;
      	stream->avail_out = buflen;
     -	algo->init_fn(c);
    --	if (compat && compat_c)
    --		compat->init_fn(compat_c);
     +	git_hash_init(c, algo);
    -+	if (compat && compat_c) {
    + 	if (compat && compat_c)
    +-		compat->init_fn(compat_c);
     +		git_hash_init(compat_c, compat);
    -+	}
      
      	/*  Start to feed header to zlib stream */
      	stream->next_in = (unsigned char *)hdr;
    @@ read-cache.c: static size_t read_eoie_extension(const char *mmap, size_t mmap_si
     
      ## rerere.c ##
     @@ rerere.c: static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz
    - 	struct git_hash_ctx ctx;
      	struct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;
      	int has_conflicts = 0;
    --	if (hash)
    + 	if (hash)
     -		the_hash_algo->init_fn(&ctx);
    -+	if (hash) {
     +		git_hash_init(&ctx, the_hash_algo);
    -+	}
      
      	while (!io->getline(&buf, io)) {
      		if (is_cmarker(buf.buf, '<', marker_size)) {
    @@ tools/coccinelle/hash.cocci (new)
     +- ALGO->init_fn(CTX);
     ++ git_hash_init(CTX, ALGO);
     +  ...>}
    -+
     
      ## trace2/tr2_sid.c ##
     @@ trace2/tr2_sid.c: static void tr2_sid_append_my_sid_component(void)
2:  cf88edda3f ! 2:  879962bf47 hash: convert remaining direct function calls
    @@ t/helper/test-synthesize.c: static int generate_pack_with_large_object(const cha
     
      ## tools/coccinelle/hash.cocci ##
     @@ tools/coccinelle/hash.cocci: struct git_hash_ctx *CTX;
    + - ALGO->init_fn(CTX);
      + git_hash_init(CTX, ALGO);
        ...>}
    - 
    ++
     +@@
     +identifier f != git_hash_clone;
     +expression ALGO;
3:  3c302bbe74 ! 3:  f06387a467 hash: document function pointers and wrappers
    @@ hash.h: struct git_hash_algo {
      
     +/*
     + * Prepare an uninitialized hash context for use. You must eventually release
    -+ * the context with with git_hash_final() (or final_oid()) or by calling
    ++ * the context with git_hash_final() (or final_oid()) or by calling
     + * git_hash_discard().
     + */
      void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop);
4:  e8b50b164a = 4:  2d876c17b1 hash: make git_hash_discard() idempotent
5:  5488debdae = 5:  3d3d5d2c63 csum-file: use idempotent git_hash_discard()
6:  15fd04f519 = 6:  91bda10e58 http: use idempotent git_hash_discard()
7:  5370cd31a8 ! 7:  2b366bc79b hash: check ctx->active flag in all wrapper functions
    @@ Commit message
         context onto the heap). Checking the flag lets us catch problems
         consistently on every build.
     
    -    Note that we can't do the same for git_init_hash(). Even though it would
    +    Note that we can't do the same for git_hash_init(). Even though it would
         cause a leak to call it twice (without an intervening final/discard),
         the point of the function is that the contents of the struct are
         undefined before the call. But calling it twice is an even less likely
         error to make, so not covering it is OK.
     
    +    We leave git_hash_discard() alone, as its idempotent behavior is
    +    convenient for callers. We _could_ try to do something similar for
    +    git_hash_final(), allowing:
    +
    +      git_hash_final(result, &ctx);
    +      git_hash_final(other_result, &ctx);
    +
    +    but it does not make much sense. After the first final() call we have
    +    thrown away the state, so we cannot produce the same output. We could
    +    come up with some sensible output (the null hash, or the empty hash),
    +    but double-calls like this are more likely a bug, so our best bet is to
    +    complain loudly (whereas the current code produces either nonsense
    +    output or undefined behavior, depending on the backend).
    +
         Signed-off-by: Jeff King <peff@peff.net>
     
      ## hash.c ##