{"thread":{"id":"65938","subject":"[PATCH 0/7] git_hash_*() quality-of-life improvements","startedAt":"2026-07-07T04:55:58Z","lastAt":"2026-07-08T08:06:06Z","messageCount":35,"participants":["Jeff King","Patrick Steinhardt","Junio C Hamano","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"547285","messageId":"20260707045556.GA1288172@coredump.intra.peff.net","threadId":"65938","inReplyTo":null,"subject":"[PATCH 0/7] git_hash_*() quality-of-life improvements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T04:55:56Z","receivedAt":"2026-07-07T04:55:58Z","isPatch":true,"body":"This implements the \"idempotent git_hash_discard()\" discussed in this\nsubthread:\n\n https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/\n\nwith associated cleanups.\n\nIt should be applied on top of jk/hash-algo-leak-fixes.\n\n [1/7]: hash: use git_hash_init() consistently\n [2/7]: hash: convert remaining direct function calls\n [3/7]: hash: document function pointers and wrappers\n [4/7]: hash: make git_hash_discard() idempotent\n [5/7]: csum-file: use idempotent git_hash_discard()\n [6/7]: http: use idempotent git_hash_discard()\n [7/7]: hash: check ctx->active flag in all wrapper functions\n\n builtin/fast-import.c | 4 +--\n builtin/index-pack.c | 6 ++--\n builtin/patch-id.c | 2 +-\n builtin/receive-pack.c | 6 ++--\n builtin/submodule--helper.c | 10 +++---\n builtin/unpack-objects.c | 4 +--\n csum-file.c | 23 +++++---------\n diff.c | 4 +--\n hash.c | 16 ++++++++++\n hash.h | 44 +++++++++++++++++++-------\n http-push.c | 2 +-\n http.c | 9 ++----\n http.h | 1 -\n object-file.c | 17 +++++-----\n pack-check.c | 2 +-\n pack-write.c | 6 ++--\n read-cache.c | 6 ++--\n rerere.c | 5 +--\n t/helper/test-hash-speed.c | 2 +-\n t/helper/test-hash.c | 2 +-\n t/helper/test-synthesize.c | 33 ++++++++++---------\n t/unit-tests/u-hash.c | 2 +-\n tools/coccinelle/hash.cocci | 63 +++++++++++++++++++++++++++++++++++++\n trace2/tr2_sid.c | 2 +-\n 24 files changed, 181 insertions(+), 90 deletions(-)\n create mode 100644 tools/coccinelle/hash.cocci\n\n-Peff\n"},{"id":"547286","messageId":"20260707050141.GA1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 1/7] hash: use git_hash_init() consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:01:41Z","receivedAt":"2026-07-07T05:01:42Z","isPatch":true,"body":"We'd like to add more logic to git_hash_init(), but many callers skip it\nand call algop->init_fn() directly. Let's make sure we're consistently\nusing the wrapper by adding a coccinelle rule.\n\nBesides the coccinelle file itself, this is a purely mechanical\nconversion based on the patch it generates. There should be no bare\ninit_fn() calls left (except for the one in the wrapper).\n\nSigned-off-by: Jeff King \n---\nIt feels like the \"expression ALGO\" in the rule should be a\n\"git_hash_algo\", but I had trouble getting coccinelle to recognize all\ncases when I did that. Probably not worth digging too far into, as\nthe presence of the git_hash_ctx type means we should never hit any\nfalse positives.\n\n builtin/fast-import.c | 4 ++--\n builtin/index-pack.c | 6 +++---\n builtin/patch-id.c | 2 +-\n builtin/receive-pack.c | 6 +++---\n builtin/submodule--helper.c | 2 +-\n builtin/unpack-objects.c | 4 ++--\n csum-file.c | 6 +++---\n diff.c | 4 ++--\n http-push.c | 2 +-\n http.c | 4 ++--\n object-file.c | 17 +++++++++--------\n pack-check.c | 2 +-\n pack-write.c | 6 +++---\n read-cache.c | 6 +++---\n rerere.c | 5 +++--\n t/helper/test-hash-speed.c | 2 +-\n t/helper/test-hash.c | 2 +-\n t/helper/test-synthesize.c | 4 ++--\n t/unit-tests/u-hash.c | 2 +-\n tools/coccinelle/hash.cocci | 10 ++++++++++\n trace2/tr2_sid.c | 2 +-\n 21 files changed, 55 insertions(+), 43 deletions(-)\n create mode 100644 tools/coccinelle/hash.cocci\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex f6473dcc8e..6692f7cd81 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -969,7 +969,7 @@ static int store_object(\n \n \thdrlen = format_object_header((char *)hdr, sizeof(hdr), type,\n \t\t\t\t dat->len);\n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \tgit_hash_update(&c, hdr, hdrlen);\n \tgit_hash_update(&c, dat->buf, dat->len);\n \tgit_hash_final_oid(&oid, &c);\n@@ -1131,7 +1131,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \n \thdrlen = format_object_header((char *)out_buf, out_sz, OBJ_BLOB, len);\n \n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \tgit_hash_update(&c, out_buf, hdrlen);\n \n \tcrc32_begin(pack_file);\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex f396658468..53a8cb9dd7 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -374,7 +374,7 @@ static const char *open_pack_file(const char *pack_name)\n \t\toutput_fd = -1;\n \t\tnothread_data.pack_fd = input_fd;\n \t}\n-\tthe_hash_algo->init_fn(&input_ctx);\n+\tgit_hash_init(&input_ctx, the_hash_algo);\n \treturn pack_name;\n }\n \n@@ -481,7 +481,7 @@ static void *unpack_entry_data(off_t offset, size_t size,\n \n \tif (!is_delta_type(type)) {\n \t\thdrlen = format_object_header(hdr, sizeof(hdr), type, size);\n-\t\tthe_hash_algo->init_fn(&c);\n+\t\tgit_hash_init(&c, the_hash_algo);\n \t\tgit_hash_update(&c, hdr, hdrlen);\n \t} else\n \t\toid = NULL;\n@@ -1291,7 +1291,7 @@ static void parse_pack_objects(unsigned char *hash)\n \n \t/* Check pack integrity */\n \tflush();\n-\tthe_hash_algo->init_fn(&tmp_ctx);\n+\tgit_hash_init(&tmp_ctx, the_hash_algo);\n \tgit_hash_clone(&tmp_ctx, &input_ctx);\n \tgit_hash_final(hash, &tmp_ctx);\n \tif (!hasheq(fill(the_hash_algo->rawsz), hash, the_repository->hash_algo))\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 57d9bd4a65..22f36ecf80 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -73,7 +73,7 @@ static size_t get_one_patchid(struct object_id *next_oid, struct object_id *resu\n \tchar pre_oid_str[GIT_MAX_HEXSZ + 1], post_oid_str[GIT_MAX_HEXSZ + 1];\n \tstruct git_hash_ctx ctx;\n \n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \toidclr(result, the_repository->hash_algo);\n \n \twhile (strbuf_getwholeline(line_buf, stdin, '\\n') != EOF) {\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 19eb6a1b61..faf0f120ac 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -615,7 +615,7 @@ static void hmac_hash(unsigned char *out,\n \t/* RFC 2104 2. (1) */\n \tmemset(key, '\\0', GIT_MAX_BLKSZ);\n \tif (the_hash_algo->blksz < key_len) {\n-\t\tthe_hash_algo->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, the_hash_algo);\n \t\tgit_hash_update(&ctx, key_in, key_len);\n \t\tgit_hash_final(key, &ctx);\n \t} else {\n@@ -629,13 +629,13 @@ static void hmac_hash(unsigned char *out,\n \t}\n \n \t/* RFC 2104 2. (3) & (4) */\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tgit_hash_update(&ctx, k_ipad, sizeof(k_ipad));\n \tgit_hash_update(&ctx, text, text_len);\n \tgit_hash_final(out, &ctx);\n \n \t/* RFC 2104 2. (6) & (7) */\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tgit_hash_update(&ctx, k_opad, sizeof(k_opad));\n \tgit_hash_update(&ctx, out, the_hash_algo->rawsz);\n \tgit_hash_final(out, &ctx);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 1cc82a134d..bf114a7856 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -550,7 +550,7 @@ static void create_default_gitdir_config(const char *submodule_name)\n \n \t/* Case 2.4: If all the above failed, try a hash of the name as a last resort */\n \theader_len = snprintf(header, sizeof(header), \"blob %zu\", strlen(submodule_name));\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tthe_hash_algo->update_fn(&ctx, header, header_len);\n \tthe_hash_algo->update_fn(&ctx, \"\\0\", 1);\n \tthe_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name));\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex f3849bb654..93a9caa582 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -670,10 +670,10 @@ int cmd_unpack_objects(int argc,\n \t\t/* We don't take any non-flag arguments now.. Maybe some day */\n \t\tusage(unpack_usage);\n \t}\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tunpack_all();\n \tgit_hash_update(&ctx, buffer, offset);\n-\tthe_hash_algo->init_fn(&tmp_ctx);\n+\tgit_hash_init(&tmp_ctx, the_hash_algo);\n \tgit_hash_clone(&tmp_ctx, &ctx);\n \tgit_hash_final_oid(&oid, &tmp_ctx);\n \tif (strict) {\ndiff --git a/csum-file.c b/csum-file.c\nindex b166f89624..7e81391524 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -175,7 +175,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,\n \tf->skip_hash = 0;\n \n \tf->algop = unsafe_hash_algo(algop);\n-\tf->algop->init_fn(&f->ctx);\n+\tgit_hash_init(&f->ctx, f->algop);\n \n \tf->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE;\n \tf->buffer = xmalloc(f->buffer_len);\n@@ -200,7 +200,7 @@ void hashfile_checkpoint_init(struct hashfile *f,\n \t\t\t struct hashfile_checkpoint *checkpoint)\n {\n \tmemset(checkpoint, 0, sizeof(*checkpoint));\n-\tf->algop->init_fn(&checkpoint->ctx);\n+\tgit_hash_init(&checkpoint->ctx, f->algop);\n }\n \n void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpoint)\n@@ -252,7 +252,7 @@ int hashfile_checksum_valid(const struct git_hash_algo *algop,\n \tif (total_len < algop->rawsz)\n \t\treturn 0; /* say \"too short\"? */\n \n-\talgop->init_fn(&ctx);\n+\tgit_hash_init(&ctx, algop);\n \tgit_hash_update(&ctx, data, data_len);\n \tgit_hash_final(got, &ctx);\n \ndiff --git a/diff.c b/diff.c\nindex 1568f0ed9c..589c1969e4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -6855,7 +6855,7 @@ void flush_one_hunk(struct object_id *result, struct git_hash_ctx *ctx)\n \tint i;\n \n \tgit_hash_final(hash, ctx);\n-\tthe_hash_algo->init_fn(ctx);\n+\tgit_hash_init(ctx, the_hash_algo);\n \t/* 20-byte sum, with carry */\n \tfor (i = 0; i < the_hash_algo->rawsz; ++i) {\n \t\tcarry += result->hash[i] + hash[i];\n@@ -6899,7 +6899,7 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid\n \tstruct git_hash_ctx ctx;\n \tstruct patch_id_t data;\n \n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tmemset(&data, 0, sizeof(struct patch_id_t));\n \tdata.ctx = &ctx;\n \toidclr(oid, the_repository->hash_algo);\ndiff --git a/http-push.c b/http-push.c\nindex 3c23cbba27..60f6f8f054 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -776,7 +776,7 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {\n \t\t\tlock->token = xstrdup(ctx->cdata);\n \n-\t\t\tthe_hash_algo->init_fn(&hash_ctx);\n+\t\t\tgit_hash_init(&hash_ctx, the_hash_algo);\n \t\t\tgit_hash_update(&hash_ctx, lock->token, strlen(lock->token));\n \t\t\tgit_hash_final(lock_token_hash, &hash_ctx);\n \ndiff --git a/http.c b/http.c\nindex 63abbaae8a..0341de5031 100644\n--- a/http.c\n+++ b/http.c\n@@ -2879,7 +2879,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \n \tgit_inflate_init(&freq->stream);\n \n-\tthe_hash_algo->init_fn(&freq->c);\n+\tgit_hash_init(&freq->c, the_hash_algo);\n \tfreq->hash_ctx_valid = 1;\n \n \tfreq->url = get_remote_object_url(base_url, hex, 0);\n@@ -2916,7 +2916,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \t\tgit_inflate_end(&freq->stream);\n \t\tmemset(&freq->stream, 0, sizeof(freq->stream));\n \t\tgit_inflate_init(&freq->stream);\n-\t\tthe_hash_algo->init_fn(&freq->c);\n+\t\tgit_hash_init(&freq->c, the_hash_algo);\n \t\tif (prev_posn>0) {\n \t\t\tprev_posn = 0;\n \t\t\tlseek(freq->localfile, 0, SEEK_SET);\ndiff --git a/object-file.c b/object-file.c\nindex e3c68cfb66..f292683c2d 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -124,7 +124,7 @@ int stream_object_signature(struct repository *r,\n \thdrlen = format_object_header(hdr, sizeof(hdr), st->type, st->size);\n \n \t/* Sha1.. */\n-\tr->hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, r->hash_algo);\n \tgit_hash_update(&c, hdr, hdrlen);\n \tfor (;;) {\n \t\tchar buf[1024 * 16];\n@@ -320,7 +320,7 @@ static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_c\n \t\t\t struct object_id *oid,\n \t\t\t char *hdr, size_t *hdrlen)\n {\n-\talgo->init_fn(c);\n+\tgit_hash_init(c, algo);\n \tgit_hash_update(c, hdr, *hdrlen);\n \tgit_hash_update(c, buf, len);\n \tgit_hash_final_oid(oid, c);\n@@ -681,9 +681,10 @@ static int start_loose_object_common(struct odb_source_loose *loose,\n \tgit_deflate_init(stream, cfg->zlib_compression_level);\n \tstream->next_out = buf;\n \tstream->avail_out = buflen;\n-\talgo->init_fn(c);\n-\tif (compat && compat_c)\n-\t\tcompat->init_fn(compat_c);\n+\tgit_hash_init(c, algo);\n+\tif (compat && compat_c) {\n+\t\tgit_hash_init(compat_c, compat);\n+\t}\n \n \t/* Start to feed header to zlib stream */\n \tstream->next_in = (unsigned char *)hdr;\n@@ -1141,7 +1142,7 @@ static int hash_blob_stream(struct odb_write_stream *stream,\n \n \theader_len = format_object_header((char *)buf, sizeof(buf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\thash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, hash_algo);\n \tgit_hash_update(&ctx, buf, header_len);\n \n \twhile (!stream->is_finished) {\n@@ -1313,7 +1314,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas\n \n \theader_len = format_object_header((char *)obuf, sizeof(obuf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\ttransaction->base.source->odb->repo->hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo);\n \tgit_hash_update(&ctx, obuf, header_len);\n \n \t/*\n@@ -1560,7 +1561,7 @@ static int check_stream_oid(git_zstream *stream,\n \tunsigned long total_read;\n \tint status = Z_OK;\n \n-\talgop->init_fn(&c);\n+\tgit_hash_init(&c, algop);\n \tgit_hash_update(&c, hdr, stream->total_out);\n \n \t/*\ndiff --git a/pack-check.c b/pack-check.c\nindex 5adfb3f272..c3b8db7c5c 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -69,7 +69,7 @@ static int verify_packfile(struct repository *r,\n \tif (!is_pack_valid(p))\n \t\treturn error(\"packfile %s cannot be accessed\", p->pack_name);\n \n-\tr->hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, r->hash_algo);\n \tdo {\n \t\tunsigned long remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\ndiff --git a/pack-write.c b/pack-write.c\nindex 83eaf88541..24033a9101 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -402,8 +402,8 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \tchar *buf;\n \tssize_t read_result;\n \n-\thash_algo->init_fn(&old_hash_ctx);\n-\thash_algo->init_fn(&new_hash_ctx);\n+\tgit_hash_init(&old_hash_ctx, hash_algo);\n+\tgit_hash_init(&new_hash_ctx, hash_algo);\n \n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n@@ -455,7 +455,7 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \t\t\t * pack, which also means making partial_pack_offset\n \t\t\t * big enough not to matter anymore.\n \t\t\t */\n-\t\t\thash_algo->init_fn(&old_hash_ctx);\n+\t\t\tgit_hash_init(&old_hash_ctx, hash_algo);\n \t\t\tpartial_pack_offset = ~partial_pack_offset;\n \t\t\tpartial_pack_offset -= MSB(partial_pack_offset, 1);\n \t\t}\ndiff --git a/read-cache.c b/read-cache.c\nindex 7c1cdcf696..5fa747e6fc 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1722,7 +1722,7 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)\n \tif (oideq(&oid, null_oid(the_hash_algo)))\n \t\treturn 0;\n \n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \tgit_hash_update(&c, hdr, size - the_hash_algo->rawsz);\n \tgit_hash_final(hash, &c);\n \tif (!hasheq(hash, start, the_repository->hash_algo))\n@@ -2957,7 +2957,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t */\n \tif (offset && record_eoie()) {\n \t\tCALLOC_ARRAY(eoie_c, 1);\n-\t\tthe_hash_algo->init_fn(eoie_c);\n+\t\tgit_hash_init(eoie_c, the_hash_algo);\n \t}\n \n \t/*\n@@ -3598,7 +3598,7 @@ static size_t read_eoie_extension(const char *mmap, size_t mmap_size)\n \t *\t \"REUC\" + )\n \t */\n \tsrc_offset = offset;\n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \twhile (src_offset < mmap_size - the_hash_algo->rawsz - EOIE_SIZE_WITH_HEADER) {\n \t\t/* After an array of active_nr index entries,\n \t\t * there can be arbitrary number of extended\ndiff --git a/rerere.c b/rerere.c\nindex 8232542585..2e932439a4 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -438,8 +438,9 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz\n \tstruct git_hash_ctx ctx;\n \tstruct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;\n \tint has_conflicts = 0;\n-\tif (hash)\n-\t\tthe_hash_algo->init_fn(&ctx);\n+\tif (hash) {\n+\t\tgit_hash_init(&ctx, the_hash_algo);\n+\t}\n \n \twhile (!io->getline(&buf, io)) {\n \t\tif (is_cmarker(buf.buf, '<', marker_size)) {\ndiff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c\nindex fbf67fe6bd..89b0268011 100644\n--- a/t/helper/test-hash-speed.c\n+++ b/t/helper/test-hash-speed.c\n@@ -5,7 +5,7 @@\n \n static inline void compute_hash(const struct git_hash_algo *algo, struct git_hash_ctx *ctx, uint8_t *final, const void *p, size_t len)\n {\n-\talgo->init_fn(ctx);\n+\tgit_hash_init(ctx, algo);\n \tgit_hash_update(ctx, p, len);\n \tgit_hash_final(final, ctx);\n }\ndiff --git a/t/helper/test-hash.c b/t/helper/test-hash.c\nindex f0ee61c8b4..1f7163695f 100644\n--- a/t/helper/test-hash.c\n+++ b/t/helper/test-hash.c\n@@ -29,7 +29,7 @@ int cmd_hash_impl(int ac, const char **av, int algo, int unsafe)\n \t\t\tdie(\"OOPS\");\n \t}\n \n-\talgop->init_fn(&ctx);\n+\tgit_hash_init(&ctx, algop);\n \n \twhile (1) {\n \t\tssize_t sz, this_sz;\ndiff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c\nindex 3fa534fbdf..7719fb3a76 100644\n--- a/t/helper/test-synthesize.c\n+++ b/t/helper/test-synthesize.c\n@@ -97,7 +97,7 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,\n \t/* Write the data as uncompressed zlib */\n \twrite_uncompressed_zlib(f, pack_ctx, data, len, algo);\n \n-\talgo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, algo);\n \tobject_header_len = format_object_header(object_header,\n \t\t\t\t\t\t sizeof(object_header),\n \t\t\t\t\t\t type, len);\n@@ -430,7 +430,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,\n \n \tf = xfopen(path, \"wb\");\n \n-\talgo->init_fn(&pack_ctx);\n+\tgit_hash_init(&pack_ctx, algo);\n \n \t/* Write pack header */\n \tfwrite_or_die(f, &pack_header, sizeof(pack_header));\ndiff --git a/t/unit-tests/u-hash.c b/t/unit-tests/u-hash.c\nindex bd4ac6a6e1..19f4efd410 100644\n--- a/t/unit-tests/u-hash.c\n+++ b/t/unit-tests/u-hash.c\n@@ -12,7 +12,7 @@ static void check_hash_data(const void *data, size_t data_length,\n \t\tunsigned char hash[GIT_MAX_HEXSZ];\n \t\tconst struct git_hash_algo *algop = &hash_algos[i];\n \n-\t\talgop->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, algop);\n \t\tgit_hash_update(&ctx, data, data_length);\n \t\tgit_hash_final(hash, &ctx);\n \ndiff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci\nnew file mode 100644\nindex 0000000000..5a7af6c544\n--- /dev/null\n+++ b/tools/coccinelle/hash.cocci\n@@ -0,0 +1,10 @@\n+@@\n+identifier f != git_hash_init;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+@@\n+ f(...) {<...\n+- ALGO->init_fn(CTX);\n++ git_hash_init(CTX, ALGO);\n+ ...>}\n+\ndiff --git a/trace2/tr2_sid.c b/trace2/tr2_sid.c\nindex 1c1d27b0ee..131b4f5a62 100644\n--- a/trace2/tr2_sid.c\n+++ b/trace2/tr2_sid.c\n@@ -45,7 +45,7 @@ static void tr2_sid_append_my_sid_component(void)\n \tif (xgethostname(hostname, sizeof(hostname)))\n \t\tstrbuf_add(&tr2sid_buf, \"Localhost\", 9);\n \telse {\n-\t\talgo->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, algo);\n \t\tgit_hash_update(&ctx, hostname, strlen(hostname));\n \t\tgit_hash_final(hash, &ctx);\n \t\thash_to_hex_algop_r(hex, hash, algo);\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547287","messageId":"20260707050417.GB1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 2/7] hash: convert remaining direct function calls","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:04:17Z","receivedAt":"2026-07-07T05:04:19Z","isPatch":true,"body":"The previous patch added a coccinelle rule to make sure callers always\nuse git_hash_init() rather than direct function pointers from the algo\nstruct.\n\nLet's do the same for the rest of the git_hash_*() wrappers. I split\nthese out because they're a bit different: they implicitly use the algop\npointer in the git_hash_ctx. So when we convert:\n\n -algo->update_fn(&ctx, buf, len);\n +git_hash_update(&ctx, buf, len);\n\nwe drop the reference to algo entirely! But this is always going to be\nthe right thing. If \"algo\" does not match what is in ctx.algop, then\nwe'd already be invoking undefined behavior.\n\nSo in addition to making it possible to add more logic to the\ngit_hash_*() functions, we're avoiding the need to pass around the extra\nalgo pointer and make sure that it matches what's in \"ctx\".\n\nThe rest of the patch is the mechanical application of that coccinelle\npatch, plus a minor cleanup in test-synthesize.c to drop a now-unused\nfunction parameter (since we don't have to pass around the algo\nseparately anymore).\n\nSigned-off-by: Jeff King \n---\n builtin/submodule--helper.c | 8 +++---\n t/helper/test-synthesize.c | 29 ++++++++++----------\n tools/coccinelle/hash.cocci | 53 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 71 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex bf114a7856..510f193a15 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -551,10 +551,10 @@ static void create_default_gitdir_config(const char *submodule_name)\n \t/* Case 2.4: If all the above failed, try a hash of the name as a last resort */\n \theader_len = snprintf(header, sizeof(header), \"blob %zu\", strlen(submodule_name));\n \tgit_hash_init(&ctx, the_hash_algo);\n-\tthe_hash_algo->update_fn(&ctx, header, header_len);\n-\tthe_hash_algo->update_fn(&ctx, \"\\0\", 1);\n-\tthe_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name));\n-\tthe_hash_algo->final_fn(raw_name_hash, &ctx);\n+\tgit_hash_update(&ctx, header, header_len);\n+\tgit_hash_update(&ctx, \"\\0\", 1);\n+\tgit_hash_update(&ctx, submodule_name, strlen(submodule_name));\n+\tgit_hash_final(raw_name_hash, &ctx);\n \thash_to_hex_algop_r(hex_name_hash, raw_name_hash, the_hash_algo);\n \tstrbuf_reset(&gitdir_path);\n \trepo_git_path_append(the_repository, &gitdir_path, \"modules/%s\", hex_name_hash);\ndiff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c\nindex 7719fb3a76..fd116c87ba 100644\n--- a/t/helper/test-synthesize.c\n+++ b/t/helper/test-synthesize.c\n@@ -25,8 +25,7 @@ static const unsigned char zeros[BLOCK_SIZE];\n * Updates the pack checksum context.\n */\n static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n-\t\t\t\t const void *data, size_t len,\n-\t\t\t\t const struct git_hash_algo *algo)\n+\t\t\t\t const void *data, size_t len)\n {\n \tunsigned char zlib_header[2] = { 0x78, 0x01 }; /* CMF, FLG */\n \tunsigned char block_header[5];\n@@ -37,7 +36,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n \n \t/* Write zlib header */\n \tfwrite_or_die(f, zlib_header, sizeof(zlib_header));\n-\talgo->update_fn(pack_ctx, zlib_header, 2);\n+\tgit_hash_update(pack_ctx, zlib_header, 2);\n \n \t/* Write uncompressed blocks (max 64KB each) */\n \tdo {\n@@ -52,11 +51,11 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n \t\tblock_header[4] = block_header[2] ^ 0xff;\n \n \t\tfwrite_or_die(f, block_header, sizeof(block_header));\n-\t\talgo->update_fn(pack_ctx, block_header, 5);\n+\t\tgit_hash_update(pack_ctx, block_header, 5);\n \n \t\tif (block_len) {\n \t\t\tfwrite_or_die(f, block_data, block_len);\n-\t\t\talgo->update_fn(pack_ctx, block_data, block_len);\n+\t\t\tgit_hash_update(pack_ctx, block_data, block_len);\n \t\t\tadler = adler32(adler, block_data, block_len);\n \t\t}\n \n@@ -68,7 +67,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n \t/* Write adler32 checksum */\n \tput_be32(adler_buf, adler);\n \tfwrite_or_die(f, adler_buf, sizeof(adler_buf));\n-\talgo->update_fn(pack_ctx, adler_buf, 4);\n+\tgit_hash_update(pack_ctx, adler_buf, 4);\n }\n \n /*\n@@ -92,24 +91,24 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,\n \t\t\t\t\t\t sizeof(pack_header),\n \t\t\t\t\t\t type, len);\n \tfwrite_or_die(f, pack_header, pack_header_len);\n-\talgo->update_fn(pack_ctx, pack_header, pack_header_len);\n+\tgit_hash_update(pack_ctx, pack_header, pack_header_len);\n \n \t/* Write the data as uncompressed zlib */\n-\twrite_uncompressed_zlib(f, pack_ctx, data, len, algo);\n+\twrite_uncompressed_zlib(f, pack_ctx, data, len);\n \n \tgit_hash_init(&ctx, algo);\n \tobject_header_len = format_object_header(object_header,\n \t\t\t\t\t\t sizeof(object_header),\n \t\t\t\t\t\t type, len);\n-\talgo->update_fn(&ctx, object_header, object_header_len);\n+\tgit_hash_update(&ctx, object_header, object_header_len);\n \tif (data)\n-\t\talgo->update_fn(&ctx, data, len);\n+\t\tgit_hash_update(&ctx, data, len);\n \telse {\n \t\tfor (size_t i = len / BLOCK_SIZE; i; i--)\n-\t\t\talgo->update_fn(&ctx, zeros, BLOCK_SIZE);\n-\t\talgo->update_fn(&ctx, zeros, len % BLOCK_SIZE);\n+\t\t\tgit_hash_update(&ctx, zeros, BLOCK_SIZE);\n+\t\tgit_hash_update(&ctx, zeros, len % BLOCK_SIZE);\n \t}\n-\talgo->final_oid_fn(oid, &ctx);\n+\tgit_hash_final_oid(oid, &ctx);\n }\n \n /*\n@@ -434,7 +433,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,\n \n \t/* Write pack header */\n \tfwrite_or_die(f, &pack_header, sizeof(pack_header));\n-\talgo->update_fn(&pack_ctx, &pack_header, sizeof(pack_header));\n+\tgit_hash_update(&pack_ctx, &pack_header, sizeof(pack_header));\n \n \t/* 1. Write the large blob */\n \twrite_pack_object(f, &pack_ctx, OBJ_BLOB, NULL, blob_size, &blob_oid, algo);\n@@ -472,7 +471,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,\n \twrite_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &final_commit_oid, algo);\n \n \t/* Write pack trailer (checksum) */\n-\talgo->final_fn(pack_hash, &pack_ctx);\n+\tgit_hash_final(pack_hash, &pack_ctx);\n \tfwrite_or_die(f, pack_hash, algo->rawsz);\n \tif (fclose(f))\n \t\tdie_errno(_(\"could not close '%s'\"), path);\ndiff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci\nindex 5a7af6c544..d0e2e5f4b1 100644\n--- a/tools/coccinelle/hash.cocci\n+++ b/tools/coccinelle/hash.cocci\n@@ -8,3 +8,56 @@ struct git_hash_ctx *CTX;\n + git_hash_init(CTX, ALGO);\n ...>}\n \n+@@\n+identifier f != git_hash_clone;\n+expression ALGO;\n+struct git_hash_ctx *SRC;\n+struct git_hash_ctx *DST;\n+@@\n+ f(...) {<...\n+- ALGO->clone_fn(DST, SRC);\n++ git_hash_clone(DST, SRC);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_update;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+expression list ARGS;\n+@@\n+ f(...) {<...\n+- ALGO->update_fn(CTX, ARGS);\n++ git_hash_update(CTX, ARGS);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_final;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+expression list ARGS;\n+@@\n+ f(...) {<...\n+- ALGO->final_fn(ARGS, CTX);\n++ git_hash_final(ARGS, CTX);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_final_oid;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+expression list ARGS;\n+@@\n+ f(...) {<...\n+- ALGO->final_oid_fn(ARGS, CTX);\n++ git_hash_final_oid(ARGS, CTX);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_discard;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+@@\n+ f(...) {<...\n+- ALGO->discard_fn(CTX);\n++ git_hash_discard(CTX);\n+ ...>}\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547288","messageId":"20260707050557.GC1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 3/7] hash: document function pointers and wrappers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:05:57Z","receivedAt":"2026-07-07T05:05:58Z","isPatch":true,"body":"We want people to use the git_hash_*() wrappers rather than the bare\nfunction pointers in the git_hash_algo struct. Let's document them\nrather than the bare pointers, and warn people away from the pointers.\nCoccinelle will eventually force the use of the wrappers, but it's\nhelpful to lead readers in the right direction from the start.\n\nWhile we're here we can document a few other bits of wisdom I've turned\nup while working in this area:\n\n - You have to initialize the destination of a git_hash_clone(). This\n is something we may eventually change for efficiency, but we should\n definitely document the requirement for now.\n\n - You must eventually finalize or discard a hash, since some backends\n may allocate resources during initialization.\n\nSigned-off-by: Jeff King \n---\n hash.h | 43 ++++++++++++++++++++++++++++++++-----------\n 1 file changed, 32 insertions(+), 11 deletions(-)\n\ndiff --git a/hash.h b/hash.h\nindex 0a23ef4dfd..5686914b71 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -309,22 +309,15 @@ struct git_hash_algo {\n \t/* The block size of the hash. */\n \tsize_t blksz;\n \n-\t/* The hash initialization function. */\n+\t/*\n+\t * Low-level implementation hooks. Callers should use the git_hash_*\n+\t * wrappers below rather than invoking these directly.\n+\t */\n \tgit_hash_init_fn init_fn;\n-\n-\t/* The hash context cloning function. */\n \tgit_hash_clone_fn clone_fn;\n-\n-\t/* The hash update function. */\n \tgit_hash_update_fn update_fn;\n-\n-\t/* The hash finalization function. */\n \tgit_hash_final_fn final_fn;\n-\n-\t/* The hash finalization function for object IDs. */\n \tgit_hash_final_oid_fn final_oid_fn;\n-\n-\t/* Discard an initialized hash without finalizing. */\n \tgit_hash_discard_fn discard_fn;\n \n \t/* The OID of the empty tree. */\n@@ -341,12 +334,40 @@ struct git_hash_algo {\n };\n extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];\n \n+/*\n+ * Prepare an uninitialized hash context for use. You must eventually release\n+ * the context with with git_hash_final() (or final_oid()) or by calling\n+ * git_hash_discard().\n+ */\n void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop);\n+\n+/*\n+ * Clone the state of a hash. Both src and dst must have been initialized with\n+ * git_hash_init().\n+ */\n void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src);\n+\n+/*\n+ * Add more data to an initialized hash context.\n+ */\n void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len);\n+\n+/*\n+ * Retrieve the final hash value from a context, releasing any resources.\n+ */\n void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx);\n+\n+/*\n+ * Like git_hash_final(), but write the result into an object_id.\n+ */\n void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx);\n+\n+/*\n+ * Discard a hash context without computing the final value, but still\n+ * releasing any resources.\n+ */\n void git_hash_discard(struct git_hash_ctx *ctx);\n+\n const struct git_hash_algo *hash_algo_ptr_by_number(uint32_t algo);\n struct git_hash_ctx *git_hash_alloc(void);\n void git_hash_free(struct git_hash_ctx *ctx);\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547289","messageId":"20260707050700.GD1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 4/7] hash: make git_hash_discard() idempotent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:07:00Z","receivedAt":"2026-07-07T05:07:01Z","isPatch":true,"body":"You must always either finalize or discard a hash context to release any\nresources, but you must call only one such function. This creates extra\nwork for some callers, since their cleanup code paths need to know\nwhether they got there via their happy path (and the finalization\nhappened) or due to an error (in which case they need to discard).\n\nLet's add an \"active\" flag that turns a redundant discard into a noop.\nThat lets you safely do this:\n\n git_hash_init(&ctx, algo);\n ...\n if (some_error)\n goto out;\n ...\n git_hash_final(result, &ctx);\n\n out:\n git_hash_discard(&ctx);\n\nThis should avoid future errors, and will also let us simplify a few\nexisting callers (in future patches).\n\nSigned-off-by: Jeff King \n---\n hash.c | 6 ++++++\n hash.h | 1 +\n 2 files changed, 7 insertions(+)\n\ndiff --git a/hash.c b/hash.c\nindex 55d1d41770..b1296f0018 100644\n--- a/hash.c\n+++ b/hash.c\n@@ -285,6 +285,7 @@ void git_hash_free(struct git_hash_ctx *ctx)\n void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)\n {\n \talgop->init_fn(ctx);\n+\tctx->active = true;\n }\n \n void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)\n@@ -300,16 +301,21 @@ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)\n void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)\n {\n \tctx->algop->final_fn(hash, ctx);\n+\tctx->active = false;\n }\n \n void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)\n {\n \tctx->algop->final_oid_fn(oid, ctx);\n+\tctx->active = false;\n }\n \n void git_hash_discard(struct git_hash_ctx *ctx)\n {\n+\tif (!ctx->active)\n+\t\treturn;\n \tctx->algop->discard_fn(ctx);\n+\tctx->active = false;\n }\n \n uint32_t hash_algo_by_name(const char *name)\ndiff --git a/hash.h b/hash.h\nindex 5686914b71..f97f7b9ff4 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -281,6 +281,7 @@ struct git_hash_ctx {\n \t\tgit_SHA_CTX_unsafe sha1_unsafe;\n \t\tgit_SHA256_CTX sha256;\n \t} state;\n+\tbool active;\n };\n \n typedef void (*git_hash_init_fn)(struct git_hash_ctx *ctx);\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547290","messageId":"20260707050750.GE1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 5/7] csum-file: use idempotent git_hash_discard()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:07:50Z","receivedAt":"2026-07-07T05:07:52Z","isPatch":true,"body":"Now that it is safe to call git_hash_discard() even after finalizing it,\nwe can simplify our cleanup logic a bit. This is mostly undoing a few\nbits of 64337aecde (csum-file: always finalize or discard hash,\n2026-07-02):\n\n - We no longer need a separate free_hashfile_memory() function for\n finalize_hashfile(). It can just call free_hashfile(), which will\n now discard (or not) the hash as appropriate.\n\n - When f->skip_hash is set, we don't need to discard; we can rely on\n free_hashfile() to do it.\n\nSigned-off-by: Jeff King \n---\n csum-file.c | 17 +++++------------\n 1 file changed, 5 insertions(+), 12 deletions(-)\n\ndiff --git a/csum-file.c b/csum-file.c\nindex 7e81391524..fe18ee1de3 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -55,32 +55,25 @@ void hashflush(struct hashfile *f)\n \t}\n }\n \n-static void free_hashfile_memory(struct hashfile *f)\n+void free_hashfile(struct hashfile *f)\n {\n+\tgit_hash_discard(&f->ctx);\n \tfree(f->buffer);\n \tfree(f->check_buffer);\n \tfree(f);\n }\n \n-void free_hashfile(struct hashfile *f)\n-{\n-\tgit_hash_discard(&f->ctx);\n-\tfree_hashfile_memory(f);\n-}\n-\n int finalize_hashfile(struct hashfile *f, unsigned char *result,\n \t\t enum fsync_component component, unsigned int flags)\n {\n \tint fd;\n \n \thashflush(f);\n \n-\tif (f->skip_hash) {\n-\t\tgit_hash_discard(&f->ctx);\n+\tif (f->skip_hash)\n \t\thashclr(f->buffer, f->algop);\n-\t} else {\n+\telse\n \t\tgit_hash_final(f->buffer, &f->ctx);\n-\t}\n \n \tif (result)\n \t\thashcpy(result, f->buffer, f->algop);\n@@ -105,7 +98,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result,\n \t\tif (close(f->check_fd))\n \t\t\tdie_errno(\"%s: sha1 file error on close\", f->name);\n \t}\n-\tfree_hashfile_memory(f);\n+\tfree_hashfile(f);\n \treturn fd;\n }\n \n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547291","messageId":"20260707050814.GF1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 6/7] http: use idempotent git_hash_discard()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:08:14Z","receivedAt":"2026-07-07T05:08:16Z","isPatch":true,"body":"Now that it is OK to call git_hash_discard() even after finalizing the\nhash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http:\ndiscard hash in dumb-http http_object_request, 2026-07-02).\n\nSigned-off-by: Jeff King \n---\n http.c | 5 +----\n http.h | 1 -\n 2 files changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 0341de5031..caccf2108e 100644\n--- a/http.c\n+++ b/http.c\n@@ -2880,7 +2880,6 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \tgit_inflate_init(&freq->stream);\n \n \tgit_hash_init(&freq->c, the_hash_algo);\n-\tfreq->hash_ctx_valid = 1;\n \n \tfreq->url = get_remote_object_url(base_url, hex, 0);\n \n@@ -2989,7 +2988,6 @@ int finish_http_object_request(struct http_object_request *freq)\n \t}\n \n \tgit_hash_final_oid(&freq->real_oid, &freq->c);\n-\tfreq->hash_ctx_valid = 0;\n \tif (freq->zret != Z_STREAM_END) {\n \t\tunlink_or_warn(freq->tmpfile.buf);\n \t\treturn -1;\n@@ -3030,8 +3028,7 @@ void release_http_object_request(struct http_object_request **freq_p)\n \tcurl_slist_free_all(freq->headers);\n \tstrbuf_release(&freq->tmpfile);\n \tgit_inflate_end(&freq->stream);\n-\tif (freq->hash_ctx_valid)\n-\t\tgit_hash_discard(&freq->c);\n+\tgit_hash_discard(&freq->c);\n \n \tfree(freq);\n \t*freq_p = NULL;\ndiff --git a/http.h b/http.h\nindex 6b0639150f..729c51904d 100644\n--- a/http.h\n+++ b/http.h\n@@ -255,7 +255,6 @@ struct http_object_request {\n \tstruct object_id oid;\n \tstruct object_id real_oid;\n \tstruct git_hash_ctx c;\n-\tint hash_ctx_valid;\n \tgit_zstream stream;\n \tint zret;\n \tint rename;\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547292","messageId":"20260707050952.GG1288294@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH 7/7] hash: check ctx->active flag in all wrapper functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T05:09:52Z","receivedAt":"2026-07-07T05:09:53Z","isPatch":true,"body":"It only makes sense to call git_hash_update(), etc, on a hash context\nthat has been initialized but not yet finalized or discarded. This is an\nunlikely error to make, but it's easy for us to catch it and complain.\n\nIt's especially important because it would quietly \"work\" for many hash\nbackends (like sha1dc, which is just manipulating some bytes) but would\ncause undefined behavior with others (like OpenSSL, which puts the\ncontext onto the heap). Checking the flag lets us catch problems\nconsistently on every build.\n\nNote that we can't do the same for git_init_hash(). Even though it would\ncause a leak to call it twice (without an intervening final/discard),\nthe point of the function is that the contents of the struct are\nundefined before the call. But calling it twice is an even less likely\nerror to make, so not covering it is OK.\n\nSigned-off-by: Jeff King \n---\n hash.c | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/hash.c b/hash.c\nindex b1296f0018..82f7e24404 100644\n--- a/hash.c\n+++ b/hash.c\n@@ -290,22 +290,32 @@ void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)\n \n void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)\n {\n+\tif (!src->active)\n+\t\tBUG(\"attempt to copy from an inactive hash context\");\n+\tif (!dst->active)\n+\t\tBUG(\"attempt to copy to an inactive hash context\");\n \tsrc->algop->clone_fn(dst, src);\n }\n \n void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)\n {\n+\tif (!ctx->active)\n+\t\tBUG(\"attempt to update an inactive hash context\");\n \tctx->algop->update_fn(ctx, in, len);\n }\n \n void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)\n {\n+\tif (!ctx->active)\n+\t\tBUG(\"attempt to finalize an inactive hash context\");\n \tctx->algop->final_fn(hash, ctx);\n \tctx->active = false;\n }\n \n void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)\n {\n+\tif (!ctx->active)\n+\t\tBUG(\"attempt to finalize an inactive hash context\");\n \tctx->algop->final_oid_fn(oid, ctx);\n \tctx->active = false;\n }\n-- \n2.55.0.459.g1b256877c9\n"},{"id":"547330","messageId":"ak0MnN6sUtFimvYe@pks.im","threadId":"65938","inReplyTo":"20260707050557.GC1288294@coredump.intra.peff.net","subject":"Re: [PATCH 3/7] hash: document function pointers and wrappers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-07T14:26:36Z","receivedAt":"2026-07-07T14:26:43Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 01:05:57AM -0400, Jeff King wrote:\n> diff --git a/hash.h b/hash.h\n> index 0a23ef4dfd..5686914b71 100644\n> --- a/hash.h\n> +++ b/hash.h\n> @@ -341,12 +334,40 @@ struct git_hash_algo {\n> };\n> extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];\n> \n> +/*\n> + * Prepare an uninitialized hash context for use. You must eventually release\n> + * the context with with git_hash_final() (or final_oid()) or by calling\n\ns/with with/with/\n\nPatrick\n"},{"id":"547331","messageId":"ak0MoazdNNj1_7OQ@pks.im","threadId":"65938","inReplyTo":"20260707050952.GG1288294@coredump.intra.peff.net","subject":"Re: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-07T14:26:41Z","receivedAt":"2026-07-07T14:26:47Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 01:09:52AM -0400, Jeff King wrote:\n> It only makes sense to call git_hash_update(), etc, on a hash context\n> that has been initialized but not yet finalized or discarded. This is an\n> unlikely error to make, but it's easy for us to catch it and complain.\n> \n> It's especially important because it would quietly \"work\" for many hash\n> backends (like sha1dc, which is just manipulating some bytes) but would\n> cause undefined behavior with others (like OpenSSL, which puts the\n> context onto the heap). Checking the flag lets us catch problems\n> consistently on every build.\n> \n> Note that we can't do the same for git_init_hash(). Even though it would\n\nYou probably mean `git_hash_init()`?\n\n> cause a leak to call it twice (without an intervening final/discard),\n> the point of the function is that the contents of the struct are\n> undefined before the call. But calling it twice is an even less likely\n> error to make, so not covering it is OK.\n\nRight. We could of course enforce that the structure must be zeroed\nbefore calling this function. But I agree that this would become quite\nawkward.\n\nPatrick\n"},{"id":"547332","messageId":"ak0MphY52YkJqs9c@pks.im","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"Re: [PATCH 0/7] git_hash_*() quality-of-life improvements","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-07T14:26:46Z","receivedAt":"2026-07-07T14:26:50Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 12:55:56AM -0400, Jeff King wrote:\n> This implements the \"idempotent git_hash_discard()\" discussed in this\n> subthread:\n> \n> https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/\n> \n> with associated cleanups.\n> \n> It should be applied on top of jk/hash-algo-leak-fixes.\n\nThanks, this was a pleasant read. I have two minor nits, but other than\nthat I'm happy with this series!\n\nPatrick\n"},{"id":"547333","messageId":"xmqq5x2q984j.fsf@gitster.g","threadId":"65938","inReplyTo":"20260707050141.GA1288294@coredump.intra.peff.net","subject":"Re: [PATCH 1/7] hash: use git_hash_init() consistently","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T14:39:24Z","receivedAt":"2026-07-07T14:39:27Z","isPatch":true,"body":"Jeff King writes:\n\n> We'd like to add more logic to git_hash_init(), but many callers skip it\n> and call algop->init_fn() directly. Let's make sure we're consistently\n> using the wrapper by adding a coccinelle rule.\n>\n> Besides the coccinelle file itself, this is a purely mechanical\n> conversion based on the patch it generates. There should be no bare\n> init_fn() calls left (except for the one in the wrapper).\n>\n> Signed-off-by: Jeff King \n> ---\n> It feels like the \"expression ALGO\" in the rule should be a\n> \"git_hash_algo\", but I had trouble getting coccinelle to recognize all\n> cases when I did that. Probably not worth digging too far into, as\n> the presence of the git_hash_ctx type means we should never hit any\n> false positives.\n\nThanks. May conversions do look simple and straight-forward, but\nsome look a bit curious.\n\n> diff --git a/object-file.c b/object-file.c\n> index e3c68cfb66..f292683c2d 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> ...\n> -\talgo->init_fn(c);\n> -\tif (compat && compat_c)\n> -\t\tcompat->init_fn(compat_c);\n> +\tgit_hash_init(c, algo);\n> +\tif (compat && compat_c) {\n> +\t\tgit_hash_init(compat_c, compat);\n> +\t}\n\nFor example, it is a mystery how Coccinelle decided to add a pair of\nbraces around this single statement. It should be obvious that the\ncorresponding single statement in the original did not need one.\n\n> diff --git a/rerere.c b/rerere.c\n> index 8232542585..2e932439a4 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -438,8 +438,9 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz\n> \tstruct git_hash_ctx ctx;\n> \tstruct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;\n> \tint has_conflicts = 0;\n> -\tif (hash)\n> -\t\tthe_hash_algo->init_fn(&ctx);\n> +\tif (hash) {\n> +\t\tgit_hash_init(&ctx, the_hash_algo);\n> +\t}\n\nDitto.\n\n"},{"id":"547359","messageId":"xmqq1pde93p4.fsf@gitster.g","threadId":"65938","inReplyTo":"20260707050417.GB1288294@coredump.intra.peff.net","subject":"Re: [PATCH 2/7] hash: convert remaining direct function calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T16:15:03Z","receivedAt":"2026-07-07T16:15:07Z","isPatch":true,"body":"Jeff King writes:\n\n> The previous patch added a coccinelle rule to make sure callers always\n> use git_hash_init() rather than direct function pointers from the algo\n> struct.\n>\n> Let's do the same for the rest of the git_hash_*() wrappers. I split\n> these out because they're a bit different: they implicitly use the algop\n> pointer in the git_hash_ctx. So when we convert:\n>\n> -algo->update_fn(&ctx, buf, len);\n> +git_hash_update(&ctx, buf, len);\n>\n> we drop the reference to algo entirely! But this is always going to be\n> the right thing. If \"algo\" does not match what is in ctx.algop, then\n> we'd already be invoking undefined behavior.\n>\n> So in addition to making it possible to add more logic to the\n> git_hash_*() functions, we're avoiding the need to pass around the extra\n> algo pointer and make sure that it matches what's in \"ctx\".\n>\n> The rest of the patch is the mechanical application of that coccinelle\n> patch, plus a minor cleanup in test-synthesize.c to drop a now-unused\n> function parameter (since we don't have to pass around the algo\n> separately anymore).\n>\n> Signed-off-by: Jeff King \n> ---\n> builtin/submodule--helper.c | 8 +++---\n> t/helper/test-synthesize.c | 29 ++++++++++----------\n> tools/coccinelle/hash.cocci | 53 +++++++++++++++++++++++++++++++++++++\n> 3 files changed, 71 insertions(+), 19 deletions(-)\n\nLooks very straight-forward.\n"},{"id":"547360","messageId":"xmqqqzle7osz.fsf@gitster.g","threadId":"65938","inReplyTo":"20260707050700.GD1288294@coredump.intra.peff.net","subject":"Re: [PATCH 4/7] hash: make git_hash_discard() idempotent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T16:22:04Z","receivedAt":"2026-07-07T16:22:06Z","isPatch":true,"body":"Jeff King writes:\n\n> You must always either finalize or discard a hash context to release any\n> resources, but you must call only one such function. This creates extra\n> work for some callers, since their cleanup code paths need to know\n> whether they got there via their happy path (and the finalization\n> happened) or due to an error (in which case they need to discard).\n>\n> Let's add an \"active\" flag that turns a redundant discard into a noop.\n> That lets you safely do this:\n>\n> git_hash_init(&ctx, algo);\n> ...\n> if (some_error)\n> goto out;\n> ...\n> git_hash_final(result, &ctx);\n>\n> out:\n> git_hash_discard(&ctx);\n>\n> This should avoid future errors, and will also let us simplify a few\n> existing callers (in future patches).\n\nHmph, so is the point of this change to allow _discard() to be\ncalled even after _final() was already called that we do not need an\nearly return or something before the out: label?\n\nUnlike commit_*() and rollback_*() used in lockfile API, where the\nnames clearly say which one is for happy and which one is for error\ncase, the _final() and _discard() pair does not exactly tell me\nwhich is which, but I guess I will get used to it, perhaps.\n\nBut the change nevertheless looks mostly good except for one \"hmph\".\nWhen _init() is called, active gets turned on automatically, and\neither _discard() or _final() turns it off. Only _discard() is\nprotected from getting called multiple times. Is this because\nit is already a no-op to call _final() multiple times?\n\nThanks.\n\n> Signed-off-by: Jeff King \n> ---\n> hash.c | 6 ++++++\n> hash.h | 1 +\n> 2 files changed, 7 insertions(+)\n>\n> diff --git a/hash.c b/hash.c\n> index 55d1d41770..b1296f0018 100644\n> --- a/hash.c\n> +++ b/hash.c\n> @@ -285,6 +285,7 @@ void git_hash_free(struct git_hash_ctx *ctx)\n> void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)\n> {\n> \talgop->init_fn(ctx);\n> +\tctx->active = true;\n> }\n> \n> void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)\n> @@ -300,16 +301,21 @@ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)\n> void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)\n> {\n> \tctx->algop->final_fn(hash, ctx);\n> +\tctx->active = false;\n> }\n> \n> void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)\n> {\n> \tctx->algop->final_oid_fn(oid, ctx);\n> +\tctx->active = false;\n> }\n> \n> void git_hash_discard(struct git_hash_ctx *ctx)\n> {\n> +\tif (!ctx->active)\n> +\t\treturn;\n> \tctx->algop->discard_fn(ctx);\n> +\tctx->active = false;\n> }\n> \n> uint32_t hash_algo_by_name(const char *name)\n> diff --git a/hash.h b/hash.h\n> index 5686914b71..f97f7b9ff4 100644\n> --- a/hash.h\n> +++ b/hash.h\n> @@ -281,6 +281,7 @@ struct git_hash_ctx {\n> \t\tgit_SHA_CTX_unsafe sha1_unsafe;\n> \t\tgit_SHA256_CTX sha256;\n> \t} state;\n> +\tbool active;\n> };\n> \n> typedef void (*git_hash_init_fn)(struct git_hash_ctx *ctx);\n"},{"id":"547362","messageId":"xmqqldbm7oni.fsf@gitster.g","threadId":"65938","inReplyTo":"20260707050814.GF1288294@coredump.intra.peff.net","subject":"Re: [PATCH 6/7] http: use idempotent git_hash_discard()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T16:25:21Z","receivedAt":"2026-07-07T16:25:24Z","isPatch":true,"body":"Jeff King writes:\n\n> Now that it is OK to call git_hash_discard() even after finalizing the\n> hash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http:\n> discard hash in dumb-http http_object_request, 2026-07-02).\n>\n> Signed-off-by: Jeff King \n> ---\n> http.c | 5 +----\n> http.h | 1 -\n> 2 files changed, 1 insertion(+), 5 deletions(-)\n\nOK, because calling _discard() on an already discarded or finished\nhash context is a no-op, we do not have to remember if we finialized\nor discarded anymore, allowing us to be extra lazy and safe. Nice.\n\n> diff --git a/http.c b/http.c\n> index 0341de5031..caccf2108e 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -2880,7 +2880,6 @@ struct http_object_request *new_http_object_request(const char *base_url,\n> \tgit_inflate_init(&freq->stream);\n> \n> \tgit_hash_init(&freq->c, the_hash_algo);\n> -\tfreq->hash_ctx_valid = 1;\n> \n> \tfreq->url = get_remote_object_url(base_url, hex, 0);\n> \n> @@ -2989,7 +2988,6 @@ int finish_http_object_request(struct http_object_request *freq)\n> \t}\n> \n> \tgit_hash_final_oid(&freq->real_oid, &freq->c);\n> -\tfreq->hash_ctx_valid = 0;\n> \tif (freq->zret != Z_STREAM_END) {\n> \t\tunlink_or_warn(freq->tmpfile.buf);\n> \t\treturn -1;\n> @@ -3030,8 +3028,7 @@ void release_http_object_request(struct http_object_request **freq_p)\n> \tcurl_slist_free_all(freq->headers);\n> \tstrbuf_release(&freq->tmpfile);\n> \tgit_inflate_end(&freq->stream);\n> -\tif (freq->hash_ctx_valid)\n> -\t\tgit_hash_discard(&freq->c);\n> +\tgit_hash_discard(&freq->c);\n> \n> \tfree(freq);\n> \t*freq_p = NULL;\n> diff --git a/http.h b/http.h\n> index 6b0639150f..729c51904d 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -255,7 +255,6 @@ struct http_object_request {\n> \tstruct object_id oid;\n> \tstruct object_id real_oid;\n> \tstruct git_hash_ctx c;\n> -\tint hash_ctx_valid;\n> \tgit_zstream stream;\n> \tint zret;\n> \tint rename;\n"},{"id":"547363","messageId":"xmqqcxwy7oal.fsf@gitster.g","threadId":"65938","inReplyTo":"20260707050952.GG1288294@coredump.intra.peff.net","subject":"Re: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T16:33:06Z","receivedAt":"2026-07-07T16:33:08Z","isPatch":true,"body":"Jeff King writes:\n\n> It only makes sense to call git_hash_update(), etc, on a hash context\n> that has been initialized but not yet finalized or discarded. This is an\n> unlikely error to make, but it's easy for us to catch it and complain.\n>\n> It's especially important because it would quietly \"work\" for many hash\n> backends (like sha1dc, which is just manipulating some bytes) but would\n> cause undefined behavior with others (like OpenSSL, which puts the\n> context onto the heap). Checking the flag lets us catch problems\n> consistently on every build.\n>\n> Note that we can't do the same for git_init_hash(). Even though it would\n> cause a leak to call it twice (without an intervening final/discard),\n> the point of the function is that the contents of the struct are\n> undefined before the call. But calling it twice is an even less likely\n> error to make, so not covering it is OK.\n>\n> Signed-off-by: Jeff King \n> ---\n> hash.c | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n\nAmong the four we see here, I agree that calling _clone and _update\non an already discarded or finalized context should be caught as an\nerror. As I alluded to earlier, though, I am not sure about\n_final. The asymmetry in a design that allows _discard after _final\nbut not _final after _final disturbs me slightly, but perhaps that\nis only because my morning caffeine has not yet kicked in. \n\n>\n> diff --git a/hash.c b/hash.c\n> index b1296f0018..82f7e24404 100644\n> --- a/hash.c\n> +++ b/hash.c\n> @@ -290,22 +290,32 @@ void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)\n> \n> void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)\n> {\n> +\tif (!src->active)\n> +\t\tBUG(\"attempt to copy from an inactive hash context\");\n> +\tif (!dst->active)\n> +\t\tBUG(\"attempt to copy to an inactive hash context\");\n> \tsrc->algop->clone_fn(dst, src);\n> }\n> \n> void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)\n> {\n> +\tif (!ctx->active)\n> +\t\tBUG(\"attempt to update an inactive hash context\");\n> \tctx->algop->update_fn(ctx, in, len);\n> }\n> \n> void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)\n> {\n> +\tif (!ctx->active)\n> +\t\tBUG(\"attempt to finalize an inactive hash context\");\n> \tctx->algop->final_fn(hash, ctx);\n> \tctx->active = false;\n> }\n> \n> void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)\n> {\n> +\tif (!ctx->active)\n> +\t\tBUG(\"attempt to finalize an inactive hash context\");\n> \tctx->algop->final_oid_fn(oid, ctx);\n> \tctx->active = false;\n> }\n"},{"id":"547394","messageId":"20260707200556.GA11780@coredump.intra.peff.net","threadId":"65938","inReplyTo":"ak0MnN6sUtFimvYe@pks.im","subject":"Re: [PATCH 3/7] hash: document function pointers and wrappers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T20:05:56Z","receivedAt":"2026-07-07T20:05:58Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 04:26:36PM +0200, Patrick Steinhardt wrote:\n\n> On Tue, Jul 07, 2026 at 01:05:57AM -0400, Jeff King wrote:\n> > diff --git a/hash.h b/hash.h\n> > index 0a23ef4dfd..5686914b71 100644\n> > --- a/hash.h\n> > +++ b/hash.h\n> > @@ -341,12 +334,40 @@ struct git_hash_algo {\n> > };\n> > extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];\n> > \n> > +/*\n> > + * Prepare an uninitialized hash context for use. You must eventually release\n> > + * the context with with git_hash_final() (or final_oid()) or by calling\n> \n> s/with with/with/\n\nThanks, looks like there are a few minor formatting nits, so I'll fix\nthis in a v2.\n\n-Peff\n"},{"id":"547395","messageId":"20260707201046.GB11780@coredump.intra.peff.net","threadId":"65938","inReplyTo":"xmqqcxwy7oal.fsf@gitster.g","subject":"Re: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T20:10:46Z","receivedAt":"2026-07-07T20:10:47Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 09:33:06AM -0700, Junio C Hamano wrote:\n\n> Among the four we see here, I agree that calling _clone and _update\n> on an already discarded or finalized context should be caught as an\n> error. As I alluded to earlier, though, I am not sure about\n> _final. The asymmetry in a design that allows _discard after _final\n> but not _final after _final disturbs me slightly, but perhaps that\n> is only because my morning caffeine has not yet kicked in. \n\nThere was more discussion in the earlier thread:\n\n https://lore.kernel.org/git/20260706000105.GA2301945@coredump.intra.peff.net/\n\nBut basically the asymmetry comes from the fact that the finalize is\ntrying to _do_ something, whereas discard is just, well, discarding.\n\nSo what should:\n\n git_hash_discard(&ctx);\n git_hash_finalize(result, &ctx);\n\nput into result? It is probably one of:\n\n 1. the null hash\n\n 2. the hash you get from init() + no updates + final()\n\n 3. nothing, BUG() instead\n\nIt seems nice at first that (1) or (2) won't cause the program to crash,\nbut ultimately they are probably the sign of a bug in the program. So\ncomplaining loudly via BUG() is probably our best bet. We could always\nloosen it later if somebody actually adds code where another behavior\nmakes sense (we know there are not such paths now, as they'd segfault\nunder openssl's heap-based backend).\n\n-Peff\n"},{"id":"547396","messageId":"20260707201315.GC11780@coredump.intra.peff.net","threadId":"65938","inReplyTo":"xmqq5x2q984j.fsf@gitster.g","subject":"Re: [PATCH 1/7] hash: use git_hash_init() consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T20:13:15Z","receivedAt":"2026-07-07T20:13:16Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 07:39:24AM -0700, Junio C Hamano wrote:\n\n> > diff --git a/object-file.c b/object-file.c\n> > index e3c68cfb66..f292683c2d 100644\n> > --- a/object-file.c\n> > +++ b/object-file.c\n> > ...\n> > -\talgo->init_fn(c);\n> > -\tif (compat && compat_c)\n> > -\t\tcompat->init_fn(compat_c);\n> > +\tgit_hash_init(c, algo);\n> > +\tif (compat && compat_c) {\n> > +\t\tgit_hash_init(compat_c, compat);\n> > +\t}\n> \n> For example, it is a mystery how Coccinelle decided to add a pair of\n> braces around this single statement. It should be obvious that the\n> corresponding single statement in the original did not need one.\n\nYeah, I noticed that coccinelle was eager to add braces in a few cases,\nbut I'm not sure why.\n\nI had actually removed them, but either I missed these two, or more\nlikely I ended up re-applying the semantic patch a final time before\ncommitting (I did a lot of \"reset --hard; make hash.cocci.patch && git\napply hash.cocci.patch\" while testing various refactors of the patch\nitself).\n\nI'll drop them in v2. Thanks for reading carefully.\n\n-Peff\n"},{"id":"547398","messageId":"xmqqfr1u1rma.fsf@gitster.g","threadId":"65938","inReplyTo":"20260707201315.GC11780@coredump.intra.peff.net","subject":"Re: [PATCH 1/7] hash: use git_hash_init() consistently","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T20:17:49Z","receivedAt":"2026-07-07T20:17:51Z","isPatch":true,"body":"Jeff King writes:\n\n> On Tue, Jul 07, 2026 at 07:39:24AM -0700, Junio C Hamano wrote:\n>\n>> > diff --git a/object-file.c b/object-file.c\n>> > index e3c68cfb66..f292683c2d 100644\n>> > --- a/object-file.c\n>> > +++ b/object-file.c\n>> > ...\n>> > -\talgo->init_fn(c);\n>> > -\tif (compat && compat_c)\n>> > -\t\tcompat->init_fn(compat_c);\n>> > +\tgit_hash_init(c, algo);\n>> > +\tif (compat && compat_c) {\n>> > +\t\tgit_hash_init(compat_c, compat);\n>> > +\t}\n>> \n>> For example, it is a mystery how Coccinelle decided to add a pair of\n>> braces around this single statement. It should be obvious that the\n>> corresponding single statement in the original did not need one.\n>\n> Yeah, I noticed that coccinelle was eager to add braces in a few cases,\n> but I'm not sure why.\n>\n> I had actually removed them, but either I missed these two, or more\n> likely I ended up re-applying the semantic patch a final time before\n> committing (I did a lot of \"reset --hard; make hash.cocci.patch && git\n> apply hash.cocci.patch\" while testing various refactors of the patch\n> itself).\n>\n> I'll drop them in v2. Thanks for reading carefully.\n\nThanks.\n\nIf we run cocci twice, the second time it should be idempotent,\nright? So running it once, fixing these braces and then running it\nagain would not make us see the extra braces in the result, I guess.\n\n"},{"id":"547399","messageId":"20260707201808.GD11780@coredump.intra.peff.net","threadId":"65938","inReplyTo":"xmqqqzle7osz.fsf@gitster.g","subject":"Re: [PATCH 4/7] hash: make git_hash_discard() idempotent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T20:18:08Z","receivedAt":"2026-07-07T20:18:10Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 09:22:04AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > You must always either finalize or discard a hash context to release any\n> > resources, but you must call only one such function. This creates extra\n> > work for some callers, since their cleanup code paths need to know\n> > whether they got there via their happy path (and the finalization\n> > happened) or due to an error (in which case they need to discard).\n> >\n> > Let's add an \"active\" flag that turns a redundant discard into a noop.\n> > That lets you safely do this:\n> >\n> > git_hash_init(&ctx, algo);\n> > ...\n> > if (some_error)\n> > goto out;\n> > ...\n> > git_hash_final(result, &ctx);\n> >\n> > out:\n> > git_hash_discard(&ctx);\n> >\n> > This should avoid future errors, and will also let us simplify a few\n> > existing callers (in future patches).\n> \n> Hmph, so is the point of this change to allow _discard() to be\n> called even after _final() was already called that we do not need an\n> early return or something before the out: label?\n\nRight. Maybe fleshing out this example was not a good idea, as yeah, you\ncould fix it with an early return. If there were more cleanup in the\n\"out\" label it would be harder. In practice neither of the spots we're\nable to clean up look exactly like this. They are split across multiple\nfunctions. So maybe:\n\n /* foo contains a git_hash_ctx and initializes it here */\n foo_init(&foo);\n\n if (some_error)\n\tfoo_release(&foo);\n\n git_hash_final(&foo.ctx);\n foo_release(&foo);\n\nwould be more realistic. The problem is that foo_release() doesn't know\nif the hash was finalized or not.\n\n> Unlike commit_*() and rollback_*() used in lockfile API, where the\n> names clearly say which one is for happy and which one is for error\n> case, the _final() and _discard() pair does not exactly tell me\n> which is which, but I guess I will get used to it, perhaps.\n\nHmm, I had hoped that \"discard\" versus just \"release\" would communicate\nthat. \"final\" is a bit funny, but that is the long-standing name for\nthat hash operation (both in our code and in libraries).\n\n> But the change nevertheless looks mostly good except for one \"hmph\".\n> When _init() is called, active gets turned on automatically, and\n> either _discard() or _final() turns it off. Only _discard() is\n> protected from getting called multiple times. Is this because\n> it is already a no-op to call _final() multiple times?\n\nNo, it's a bug to call _final() multiple times. See my response\nelsewhere in the thread.\n\n-Peff\n"},{"id":"547400","messageId":"20260707202541.GE11780@coredump.intra.peff.net","threadId":"65938","inReplyTo":"xmqqfr1u1rma.fsf@gitster.g","subject":"Re: [PATCH 1/7] hash: use git_hash_init() consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-07T20:25:41Z","receivedAt":"2026-07-07T20:25:43Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 01:17:49PM -0700, Junio C Hamano wrote:\n\n> > I had actually removed them, but either I missed these two, or more\n> > likely I ended up re-applying the semantic patch a final time before\n> > committing (I did a lot of \"reset --hard; make hash.cocci.patch && git\n> > apply hash.cocci.patch\" while testing various refactors of the patch\n> > itself).\n> >\n> > I'll drop them in v2. Thanks for reading carefully.\n> \n> Thanks.\n> \n> If we run cocci twice, the second time it should be idempotent,\n> right? So running it once, fixing these braces and then running it\n> again would not make us see the extra braces in the result, I guess.\n\nYep, exactly.\n\nIf my \"re-applying\" theory above is correct, that is different because I\nwas calling \"reset --hard\" in the middle to test that the patch still\ndid what it claimed. ;)\n\nI assume this is coccinelle having some kind of \"add braces to be\ncareful in some situations\" logic, but I didn't dig into it further.\n\n-Peff\n"},{"id":"547405","messageId":"ak1u25b2pmRAQIxD@fruit.crustytoothpaste.net","threadId":"65938","inReplyTo":"20260707050141.GA1288294@coredump.intra.peff.net","subject":"Re: [PATCH 1/7] hash: use git_hash_init() consistently","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-07-07T21:25:48Z","receivedAt":"2026-07-07T21:25:56Z","isPatch":true,"body":"On 2026-07-07 at 05:01:41, Jeff King wrote:\n> We'd like to add more logic to git_hash_init(), but many callers skip it\n> and call algop->init_fn() directly. Let's make sure we're consistently\n> using the wrapper by adding a coccinelle rule.\n> \n> Besides the coccinelle file itself, this is a purely mechanical\n> conversion based on the patch it generates. There should be no bare\n> init_fn() calls left (except for the one in the wrapper).\n\nFor context, the reason `git_hash_init` exists is that our Rust code\nneeds to initialize a hash context but it treats `const struct\ngit_hash_algo *` as `const void *` and doesn't have any access to the\ncontents of the structure. We could fix this with `cbindgen` and\n`bindgen`, but haven't done so yet.\n\nSo that's why everybody has been using `init_fn` instead of\n`git_hash_init`. Anyway, I have no objections to making this the\nstandard interface going forward.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"547407","messageId":"ak1yazHtP_OazDaO@fruit.crustytoothpaste.net","threadId":"65938","inReplyTo":"20260707201808.GD11780@coredump.intra.peff.net","subject":"Re: [PATCH 4/7] hash: make git_hash_discard() idempotent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-07-07T21:41:00Z","receivedAt":"2026-07-07T21:41:02Z","isPatch":true,"body":"On 2026-07-07 at 20:18:08, Jeff King wrote:\n> On Tue, Jul 07, 2026 at 09:22:04AM -0700, Junio C Hamano wrote:\n> > But the change nevertheless looks mostly good except for one \"hmph\".\n> > When _init() is called, active gets turned on automatically, and\n> > either _discard() or _final() turns it off. Only _discard() is\n> > protected from getting called multiple times. Is this because\n> > it is already a no-op to call _final() multiple times?\n> \n> No, it's a bug to call _final() multiple times. See my response\n> elsewhere in the thread.\n\nThis is almost always the case in hash function libraries. Let me\nexplain why.\n\nA context for SHA-256 contains the 8 32-bit words in the state, a bit or\nbyte counter (as a 64-bit quantity or two 32-bit quantities), and a\n64-byte buffer for unprocessed bytes—and that's it. When finalizing a\nhash, you must always pad with a 0x80 byte and then optionally some zero\nbytes, plus a 64-bit counter of bits in the message. That may result in\none or two iterations of the hash to process the remaining bytes and the\npadding, and that almost always updates the state words in the context\nin place. (SHA-1 functions identically but for the state size.)\n\nSo if you call the final function multiple times, you're not computing\nthe final value the second time, but instead trying to re-pad and\nre-compute the final hash value, which results in a _different_,\nincorrect value. In SHA-256, this is a valid hash value for a different\nmessage (which is the original message with the padding and length\ntacked on and is effectively a length-extension attack), but in hashes\nthat don't allow length-extension attacks, such as SHA-3 and BLAKE2,\nwhat you get is simply corrupt data.\n\nSo most hash function libraries that allocate memory are going to free\nit in the final function because you can't really call final multiple\ntimes and get a sensible response. If you want to do that, then you\nneed to clone the context and call final on each context once.\n\nOur Rust code makes calling final a second time impossible because\nfinalization takes `self`, not `&mut self`, so the object is _moved_\ninto the final method and you no longer have access to it after that.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"547408","messageId":"xmqqqzlezbcz.fsf@gitster.g","threadId":"65938","inReplyTo":"ak1yazHtP_OazDaO@fruit.crustytoothpaste.net","subject":"Re: [PATCH 4/7] hash: make git_hash_discard() idempotent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T22:25:00Z","receivedAt":"2026-07-07T22:25:06Z","isPatch":true,"body":"\"brian m. carlson\" writes:\n\n> Our Rust code makes calling final a second time impossible because\n> finalization takes `self`, not `&mut self`, so the object is _moved_\n> into the final method and you no longer have access to it after that.\n\nThat is a cute trick available to Rust but not many other languages,\nI guess ;-).\n"},{"id":"547435","messageId":"20260708035235.GA41491@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260707045556.GA1288172@coredump.intra.peff.net","subject":"[PATCH v2 0/7] git_hash_*() quality-of-life improvements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:52:35Z","receivedAt":"2026-07-08T03:52:36Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 12:55:57AM -0400, Jeff King wrote:\n\n> This implements the \"idempotent git_hash_discard()\" discussed in this\n> subthread:\n> \n> https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/\n> \n> with associated cleanups.\n\nHere's a v2 addressing the comments so far. Mostly minor changes:\n\n - fixed typos noticed by Patrick\n\n - dropped extra braces added by coccinelle\n\n - dropped a trailing blank line from patch 1 (this gets fixed in a\n later patch as we add more content after the blank line, but I\n noticed \"git apply\" complaining)\n\n - a bit more explanation in patch 7 about why we don't support\n idempotent final() calls\n\nPatch list below, followed by range diff.\n\n [1/7]: hash: use git_hash_init() consistently\n [2/7]: hash: convert remaining direct function calls\n [3/7]: hash: document function pointers and wrappers\n [4/7]: hash: make git_hash_discard() idempotent\n [5/7]: csum-file: use idempotent git_hash_discard()\n [6/7]: http: use idempotent git_hash_discard()\n [7/7]: hash: check ctx->active flag in all wrapper functions\n\n builtin/fast-import.c | 4 +--\n builtin/index-pack.c | 6 ++--\n builtin/patch-id.c | 2 +-\n builtin/receive-pack.c | 6 ++--\n builtin/submodule--helper.c | 10 +++---\n builtin/unpack-objects.c | 4 +--\n csum-file.c | 23 +++++---------\n diff.c | 4 +--\n hash.c | 16 ++++++++++\n hash.h | 44 +++++++++++++++++++-------\n http-push.c | 2 +-\n http.c | 9 ++----\n http.h | 1 -\n object-file.c | 14 ++++-----\n pack-check.c | 2 +-\n pack-write.c | 6 ++--\n read-cache.c | 6 ++--\n rerere.c | 2 +-\n t/helper/test-hash-speed.c | 2 +-\n t/helper/test-hash.c | 2 +-\n t/helper/test-synthesize.c | 33 ++++++++++---------\n t/unit-tests/u-hash.c | 2 +-\n tools/coccinelle/hash.cocci | 63 +++++++++++++++++++++++++++++++++++++\n trace2/tr2_sid.c | 2 +-\n 24 files changed, 177 insertions(+), 88 deletions(-)\n create mode 100644 tools/coccinelle/hash.cocci\n\n\n1: 2f1c8cbc98 ! 1: 911cf0dfcd hash: use git_hash_init() consistently\n @@ object-file.c: static int start_loose_object_common(struct odb_source_loose *loo\n \tstream->next_out = buf;\n \tstream->avail_out = buflen;\n -\talgo->init_fn(c);\n --\tif (compat && compat_c)\n --\t\tcompat->init_fn(compat_c);\n +\tgit_hash_init(c, algo);\n -+\tif (compat && compat_c) {\n + \tif (compat && compat_c)\n +-\t\tcompat->init_fn(compat_c);\n +\t\tgit_hash_init(compat_c, compat);\n -+\t}\n \n \t/* Start to feed header to zlib stream */\n \tstream->next_in = (unsigned char *)hdr;\n @@ read-cache.c: static size_t read_eoie_extension(const char *mmap, size_t mmap_si\n \n ## rerere.c ##\n @@ rerere.c: static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz\n - \tstruct git_hash_ctx ctx;\n \tstruct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;\n \tint has_conflicts = 0;\n --\tif (hash)\n + \tif (hash)\n -\t\tthe_hash_algo->init_fn(&ctx);\n -+\tif (hash) {\n +\t\tgit_hash_init(&ctx, the_hash_algo);\n -+\t}\n \n \twhile (!io->getline(&buf, io)) {\n \t\tif (is_cmarker(buf.buf, '<', marker_size)) {\n @@ tools/coccinelle/hash.cocci (new)\n +- ALGO->init_fn(CTX);\n ++ git_hash_init(CTX, ALGO);\n + ...>}\n -+\n \n ## trace2/tr2_sid.c ##\n @@ trace2/tr2_sid.c: static void tr2_sid_append_my_sid_component(void)\n2: cf88edda3f ! 2: 879962bf47 hash: convert remaining direct function calls\n @@ t/helper/test-synthesize.c: static int generate_pack_with_large_object(const cha\n \n ## tools/coccinelle/hash.cocci ##\n @@ tools/coccinelle/hash.cocci: struct git_hash_ctx *CTX;\n + - ALGO->init_fn(CTX);\n + git_hash_init(CTX, ALGO);\n ...>}\n - \n ++\n +@@\n +identifier f != git_hash_clone;\n +expression ALGO;\n3: 3c302bbe74 ! 3: f06387a467 hash: document function pointers and wrappers\n @@ hash.h: struct git_hash_algo {\n \n +/*\n + * Prepare an uninitialized hash context for use. You must eventually release\n -+ * the context with with git_hash_final() (or final_oid()) or by calling\n ++ * the context with git_hash_final() (or final_oid()) or by calling\n + * git_hash_discard().\n + */\n void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop);\n4: e8b50b164a = 4: 2d876c17b1 hash: make git_hash_discard() idempotent\n5: 5488debdae = 5: 3d3d5d2c63 csum-file: use idempotent git_hash_discard()\n6: 15fd04f519 = 6: 91bda10e58 http: use idempotent git_hash_discard()\n7: 5370cd31a8 ! 7: 2b366bc79b hash: check ctx->active flag in all wrapper functions\n @@ Commit message\n context onto the heap). Checking the flag lets us catch problems\n consistently on every build.\n \n - Note that we can't do the same for git_init_hash(). Even though it would\n + Note that we can't do the same for git_hash_init(). Even though it would\n cause a leak to call it twice (without an intervening final/discard),\n the point of the function is that the contents of the struct are\n undefined before the call. But calling it twice is an even less likely\n error to make, so not covering it is OK.\n \n + We leave git_hash_discard() alone, as its idempotent behavior is\n + convenient for callers. We _could_ try to do something similar for\n + git_hash_final(), allowing:\n +\n + git_hash_final(result, &ctx);\n + git_hash_final(other_result, &ctx);\n +\n + but it does not make much sense. After the first final() call we have\n + thrown away the state, so we cannot produce the same output. We could\n + come up with some sensible output (the null hash, or the empty hash),\n + but double-calls like this are more likely a bug, so our best bet is to\n + complain loudly (whereas the current code produces either nonsense\n + output or undefined behavior, depending on the backend).\n +\n Signed-off-by: Jeff King \n \n ## hash.c ##\n"},{"id":"547436","messageId":"20260708035249.GA41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 1/7] hash: use git_hash_init() consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:52:49Z","receivedAt":"2026-07-08T03:52:50Z","isPatch":true,"body":"We'd like to add more logic to git_hash_init(), but many callers skip it\nand call algop->init_fn() directly. Let's make sure we're consistently\nusing the wrapper by adding a coccinelle rule.\n\nBesides the coccinelle file itself, this is a purely mechanical\nconversion based on the patch it generates. There should be no bare\ninit_fn() calls left (except for the one in the wrapper).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-import.c | 4 ++--\n builtin/index-pack.c | 6 +++---\n builtin/patch-id.c | 2 +-\n builtin/receive-pack.c | 6 +++---\n builtin/submodule--helper.c | 2 +-\n builtin/unpack-objects.c | 4 ++--\n csum-file.c | 6 +++---\n diff.c | 4 ++--\n http-push.c | 2 +-\n http.c | 4 ++--\n object-file.c | 14 +++++++-------\n pack-check.c | 2 +-\n pack-write.c | 6 +++---\n read-cache.c | 6 +++---\n rerere.c | 2 +-\n t/helper/test-hash-speed.c | 2 +-\n t/helper/test-hash.c | 2 +-\n t/helper/test-synthesize.c | 4 ++--\n t/unit-tests/u-hash.c | 2 +-\n tools/coccinelle/hash.cocci | 9 +++++++++\n trace2/tr2_sid.c | 2 +-\n 21 files changed, 50 insertions(+), 41 deletions(-)\n create mode 100644 tools/coccinelle/hash.cocci\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex f6473dcc8e..6692f7cd81 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -969,7 +969,7 @@ static int store_object(\n \n \thdrlen = format_object_header((char *)hdr, sizeof(hdr), type,\n \t\t\t\t dat->len);\n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \tgit_hash_update(&c, hdr, hdrlen);\n \tgit_hash_update(&c, dat->buf, dat->len);\n \tgit_hash_final_oid(&oid, &c);\n@@ -1131,7 +1131,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \n \thdrlen = format_object_header((char *)out_buf, out_sz, OBJ_BLOB, len);\n \n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \tgit_hash_update(&c, out_buf, hdrlen);\n \n \tcrc32_begin(pack_file);\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex f396658468..53a8cb9dd7 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -374,7 +374,7 @@ static const char *open_pack_file(const char *pack_name)\n \t\toutput_fd = -1;\n \t\tnothread_data.pack_fd = input_fd;\n \t}\n-\tthe_hash_algo->init_fn(&input_ctx);\n+\tgit_hash_init(&input_ctx, the_hash_algo);\n \treturn pack_name;\n }\n \n@@ -481,7 +481,7 @@ static void *unpack_entry_data(off_t offset, size_t size,\n \n \tif (!is_delta_type(type)) {\n \t\thdrlen = format_object_header(hdr, sizeof(hdr), type, size);\n-\t\tthe_hash_algo->init_fn(&c);\n+\t\tgit_hash_init(&c, the_hash_algo);\n \t\tgit_hash_update(&c, hdr, hdrlen);\n \t} else\n \t\toid = NULL;\n@@ -1291,7 +1291,7 @@ static void parse_pack_objects(unsigned char *hash)\n \n \t/* Check pack integrity */\n \tflush();\n-\tthe_hash_algo->init_fn(&tmp_ctx);\n+\tgit_hash_init(&tmp_ctx, the_hash_algo);\n \tgit_hash_clone(&tmp_ctx, &input_ctx);\n \tgit_hash_final(hash, &tmp_ctx);\n \tif (!hasheq(fill(the_hash_algo->rawsz), hash, the_repository->hash_algo))\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 57d9bd4a65..22f36ecf80 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -73,7 +73,7 @@ static size_t get_one_patchid(struct object_id *next_oid, struct object_id *resu\n \tchar pre_oid_str[GIT_MAX_HEXSZ + 1], post_oid_str[GIT_MAX_HEXSZ + 1];\n \tstruct git_hash_ctx ctx;\n \n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \toidclr(result, the_repository->hash_algo);\n \n \twhile (strbuf_getwholeline(line_buf, stdin, '\\n') != EOF) {\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 19eb6a1b61..faf0f120ac 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -615,7 +615,7 @@ static void hmac_hash(unsigned char *out,\n \t/* RFC 2104 2. (1) */\n \tmemset(key, '\\0', GIT_MAX_BLKSZ);\n \tif (the_hash_algo->blksz < key_len) {\n-\t\tthe_hash_algo->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, the_hash_algo);\n \t\tgit_hash_update(&ctx, key_in, key_len);\n \t\tgit_hash_final(key, &ctx);\n \t} else {\n@@ -629,13 +629,13 @@ static void hmac_hash(unsigned char *out,\n \t}\n \n \t/* RFC 2104 2. (3) & (4) */\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tgit_hash_update(&ctx, k_ipad, sizeof(k_ipad));\n \tgit_hash_update(&ctx, text, text_len);\n \tgit_hash_final(out, &ctx);\n \n \t/* RFC 2104 2. (6) & (7) */\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tgit_hash_update(&ctx, k_opad, sizeof(k_opad));\n \tgit_hash_update(&ctx, out, the_hash_algo->rawsz);\n \tgit_hash_final(out, &ctx);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 1cc82a134d..bf114a7856 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -550,7 +550,7 @@ static void create_default_gitdir_config(const char *submodule_name)\n \n \t/* Case 2.4: If all the above failed, try a hash of the name as a last resort */\n \theader_len = snprintf(header, sizeof(header), \"blob %zu\", strlen(submodule_name));\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tthe_hash_algo->update_fn(&ctx, header, header_len);\n \tthe_hash_algo->update_fn(&ctx, \"\\0\", 1);\n \tthe_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name));\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex f3849bb654..93a9caa582 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -670,10 +670,10 @@ int cmd_unpack_objects(int argc,\n \t\t/* We don't take any non-flag arguments now.. Maybe some day */\n \t\tusage(unpack_usage);\n \t}\n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tunpack_all();\n \tgit_hash_update(&ctx, buffer, offset);\n-\tthe_hash_algo->init_fn(&tmp_ctx);\n+\tgit_hash_init(&tmp_ctx, the_hash_algo);\n \tgit_hash_clone(&tmp_ctx, &ctx);\n \tgit_hash_final_oid(&oid, &tmp_ctx);\n \tif (strict) {\ndiff --git a/csum-file.c b/csum-file.c\nindex b166f89624..7e81391524 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -175,7 +175,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,\n \tf->skip_hash = 0;\n \n \tf->algop = unsafe_hash_algo(algop);\n-\tf->algop->init_fn(&f->ctx);\n+\tgit_hash_init(&f->ctx, f->algop);\n \n \tf->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE;\n \tf->buffer = xmalloc(f->buffer_len);\n@@ -200,7 +200,7 @@ void hashfile_checkpoint_init(struct hashfile *f,\n \t\t\t struct hashfile_checkpoint *checkpoint)\n {\n \tmemset(checkpoint, 0, sizeof(*checkpoint));\n-\tf->algop->init_fn(&checkpoint->ctx);\n+\tgit_hash_init(&checkpoint->ctx, f->algop);\n }\n \n void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpoint)\n@@ -252,7 +252,7 @@ int hashfile_checksum_valid(const struct git_hash_algo *algop,\n \tif (total_len < algop->rawsz)\n \t\treturn 0; /* say \"too short\"? */\n \n-\talgop->init_fn(&ctx);\n+\tgit_hash_init(&ctx, algop);\n \tgit_hash_update(&ctx, data, data_len);\n \tgit_hash_final(got, &ctx);\n \ndiff --git a/diff.c b/diff.c\nindex 1568f0ed9c..589c1969e4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -6855,7 +6855,7 @@ void flush_one_hunk(struct object_id *result, struct git_hash_ctx *ctx)\n \tint i;\n \n \tgit_hash_final(hash, ctx);\n-\tthe_hash_algo->init_fn(ctx);\n+\tgit_hash_init(ctx, the_hash_algo);\n \t/* 20-byte sum, with carry */\n \tfor (i = 0; i < the_hash_algo->rawsz; ++i) {\n \t\tcarry += result->hash[i] + hash[i];\n@@ -6899,7 +6899,7 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid\n \tstruct git_hash_ctx ctx;\n \tstruct patch_id_t data;\n \n-\tthe_hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, the_hash_algo);\n \tmemset(&data, 0, sizeof(struct patch_id_t));\n \tdata.ctx = &ctx;\n \toidclr(oid, the_repository->hash_algo);\ndiff --git a/http-push.c b/http-push.c\nindex 3c23cbba27..60f6f8f054 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -776,7 +776,7 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {\n \t\t\tlock->token = xstrdup(ctx->cdata);\n \n-\t\t\tthe_hash_algo->init_fn(&hash_ctx);\n+\t\t\tgit_hash_init(&hash_ctx, the_hash_algo);\n \t\t\tgit_hash_update(&hash_ctx, lock->token, strlen(lock->token));\n \t\t\tgit_hash_final(lock_token_hash, &hash_ctx);\n \ndiff --git a/http.c b/http.c\nindex 63abbaae8a..0341de5031 100644\n--- a/http.c\n+++ b/http.c\n@@ -2879,7 +2879,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \n \tgit_inflate_init(&freq->stream);\n \n-\tthe_hash_algo->init_fn(&freq->c);\n+\tgit_hash_init(&freq->c, the_hash_algo);\n \tfreq->hash_ctx_valid = 1;\n \n \tfreq->url = get_remote_object_url(base_url, hex, 0);\n@@ -2916,7 +2916,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \t\tgit_inflate_end(&freq->stream);\n \t\tmemset(&freq->stream, 0, sizeof(freq->stream));\n \t\tgit_inflate_init(&freq->stream);\n-\t\tthe_hash_algo->init_fn(&freq->c);\n+\t\tgit_hash_init(&freq->c, the_hash_algo);\n \t\tif (prev_posn>0) {\n \t\t\tprev_posn = 0;\n \t\t\tlseek(freq->localfile, 0, SEEK_SET);\ndiff --git a/object-file.c b/object-file.c\nindex e3c68cfb66..93602f8c50 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -124,7 +124,7 @@ int stream_object_signature(struct repository *r,\n \thdrlen = format_object_header(hdr, sizeof(hdr), st->type, st->size);\n \n \t/* Sha1.. */\n-\tr->hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, r->hash_algo);\n \tgit_hash_update(&c, hdr, hdrlen);\n \tfor (;;) {\n \t\tchar buf[1024 * 16];\n@@ -320,7 +320,7 @@ static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_c\n \t\t\t struct object_id *oid,\n \t\t\t char *hdr, size_t *hdrlen)\n {\n-\talgo->init_fn(c);\n+\tgit_hash_init(c, algo);\n \tgit_hash_update(c, hdr, *hdrlen);\n \tgit_hash_update(c, buf, len);\n \tgit_hash_final_oid(oid, c);\n@@ -681,9 +681,9 @@ static int start_loose_object_common(struct odb_source_loose *loose,\n \tgit_deflate_init(stream, cfg->zlib_compression_level);\n \tstream->next_out = buf;\n \tstream->avail_out = buflen;\n-\talgo->init_fn(c);\n+\tgit_hash_init(c, algo);\n \tif (compat && compat_c)\n-\t\tcompat->init_fn(compat_c);\n+\t\tgit_hash_init(compat_c, compat);\n \n \t/* Start to feed header to zlib stream */\n \tstream->next_in = (unsigned char *)hdr;\n@@ -1141,7 +1141,7 @@ static int hash_blob_stream(struct odb_write_stream *stream,\n \n \theader_len = format_object_header((char *)buf, sizeof(buf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\thash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, hash_algo);\n \tgit_hash_update(&ctx, buf, header_len);\n \n \twhile (!stream->is_finished) {\n@@ -1313,7 +1313,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas\n \n \theader_len = format_object_header((char *)obuf, sizeof(obuf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\ttransaction->base.source->odb->repo->hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo);\n \tgit_hash_update(&ctx, obuf, header_len);\n \n \t/*\n@@ -1560,7 +1560,7 @@ static int check_stream_oid(git_zstream *stream,\n \tunsigned long total_read;\n \tint status = Z_OK;\n \n-\talgop->init_fn(&c);\n+\tgit_hash_init(&c, algop);\n \tgit_hash_update(&c, hdr, stream->total_out);\n \n \t/*\ndiff --git a/pack-check.c b/pack-check.c\nindex 5adfb3f272..c3b8db7c5c 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -69,7 +69,7 @@ static int verify_packfile(struct repository *r,\n \tif (!is_pack_valid(p))\n \t\treturn error(\"packfile %s cannot be accessed\", p->pack_name);\n \n-\tr->hash_algo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, r->hash_algo);\n \tdo {\n \t\tunsigned long remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\ndiff --git a/pack-write.c b/pack-write.c\nindex 83eaf88541..24033a9101 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -402,8 +402,8 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \tchar *buf;\n \tssize_t read_result;\n \n-\thash_algo->init_fn(&old_hash_ctx);\n-\thash_algo->init_fn(&new_hash_ctx);\n+\tgit_hash_init(&old_hash_ctx, hash_algo);\n+\tgit_hash_init(&new_hash_ctx, hash_algo);\n \n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n@@ -455,7 +455,7 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \t\t\t * pack, which also means making partial_pack_offset\n \t\t\t * big enough not to matter anymore.\n \t\t\t */\n-\t\t\thash_algo->init_fn(&old_hash_ctx);\n+\t\t\tgit_hash_init(&old_hash_ctx, hash_algo);\n \t\t\tpartial_pack_offset = ~partial_pack_offset;\n \t\t\tpartial_pack_offset -= MSB(partial_pack_offset, 1);\n \t\t}\ndiff --git a/read-cache.c b/read-cache.c\nindex 7c1cdcf696..5fa747e6fc 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1722,7 +1722,7 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)\n \tif (oideq(&oid, null_oid(the_hash_algo)))\n \t\treturn 0;\n \n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \tgit_hash_update(&c, hdr, size - the_hash_algo->rawsz);\n \tgit_hash_final(hash, &c);\n \tif (!hasheq(hash, start, the_repository->hash_algo))\n@@ -2957,7 +2957,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t */\n \tif (offset && record_eoie()) {\n \t\tCALLOC_ARRAY(eoie_c, 1);\n-\t\tthe_hash_algo->init_fn(eoie_c);\n+\t\tgit_hash_init(eoie_c, the_hash_algo);\n \t}\n \n \t/*\n@@ -3598,7 +3598,7 @@ static size_t read_eoie_extension(const char *mmap, size_t mmap_size)\n \t *\t \"REUC\" + )\n \t */\n \tsrc_offset = offset;\n-\tthe_hash_algo->init_fn(&c);\n+\tgit_hash_init(&c, the_hash_algo);\n \twhile (src_offset < mmap_size - the_hash_algo->rawsz - EOIE_SIZE_WITH_HEADER) {\n \t\t/* After an array of active_nr index entries,\n \t\t * there can be arbitrary number of extended\ndiff --git a/rerere.c b/rerere.c\nindex 8232542585..216100925a 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -439,7 +439,7 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz\n \tstruct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;\n \tint has_conflicts = 0;\n \tif (hash)\n-\t\tthe_hash_algo->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, the_hash_algo);\n \n \twhile (!io->getline(&buf, io)) {\n \t\tif (is_cmarker(buf.buf, '<', marker_size)) {\ndiff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c\nindex fbf67fe6bd..89b0268011 100644\n--- a/t/helper/test-hash-speed.c\n+++ b/t/helper/test-hash-speed.c\n@@ -5,7 +5,7 @@\n \n static inline void compute_hash(const struct git_hash_algo *algo, struct git_hash_ctx *ctx, uint8_t *final, const void *p, size_t len)\n {\n-\talgo->init_fn(ctx);\n+\tgit_hash_init(ctx, algo);\n \tgit_hash_update(ctx, p, len);\n \tgit_hash_final(final, ctx);\n }\ndiff --git a/t/helper/test-hash.c b/t/helper/test-hash.c\nindex f0ee61c8b4..1f7163695f 100644\n--- a/t/helper/test-hash.c\n+++ b/t/helper/test-hash.c\n@@ -29,7 +29,7 @@ int cmd_hash_impl(int ac, const char **av, int algo, int unsafe)\n \t\t\tdie(\"OOPS\");\n \t}\n \n-\talgop->init_fn(&ctx);\n+\tgit_hash_init(&ctx, algop);\n \n \twhile (1) {\n \t\tssize_t sz, this_sz;\ndiff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c\nindex 3fa534fbdf..7719fb3a76 100644\n--- a/t/helper/test-synthesize.c\n+++ b/t/helper/test-synthesize.c\n@@ -97,7 +97,7 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,\n \t/* Write the data as uncompressed zlib */\n \twrite_uncompressed_zlib(f, pack_ctx, data, len, algo);\n \n-\talgo->init_fn(&ctx);\n+\tgit_hash_init(&ctx, algo);\n \tobject_header_len = format_object_header(object_header,\n \t\t\t\t\t\t sizeof(object_header),\n \t\t\t\t\t\t type, len);\n@@ -430,7 +430,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,\n \n \tf = xfopen(path, \"wb\");\n \n-\talgo->init_fn(&pack_ctx);\n+\tgit_hash_init(&pack_ctx, algo);\n \n \t/* Write pack header */\n \tfwrite_or_die(f, &pack_header, sizeof(pack_header));\ndiff --git a/t/unit-tests/u-hash.c b/t/unit-tests/u-hash.c\nindex bd4ac6a6e1..19f4efd410 100644\n--- a/t/unit-tests/u-hash.c\n+++ b/t/unit-tests/u-hash.c\n@@ -12,7 +12,7 @@ static void check_hash_data(const void *data, size_t data_length,\n \t\tunsigned char hash[GIT_MAX_HEXSZ];\n \t\tconst struct git_hash_algo *algop = &hash_algos[i];\n \n-\t\talgop->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, algop);\n \t\tgit_hash_update(&ctx, data, data_length);\n \t\tgit_hash_final(hash, &ctx);\n \ndiff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci\nnew file mode 100644\nindex 0000000000..04270ee043\n--- /dev/null\n+++ b/tools/coccinelle/hash.cocci\n@@ -0,0 +1,9 @@\n+@@\n+identifier f != git_hash_init;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+@@\n+ f(...) {<...\n+- ALGO->init_fn(CTX);\n++ git_hash_init(CTX, ALGO);\n+ ...>}\ndiff --git a/trace2/tr2_sid.c b/trace2/tr2_sid.c\nindex 1c1d27b0ee..131b4f5a62 100644\n--- a/trace2/tr2_sid.c\n+++ b/trace2/tr2_sid.c\n@@ -45,7 +45,7 @@ static void tr2_sid_append_my_sid_component(void)\n \tif (xgethostname(hostname, sizeof(hostname)))\n \t\tstrbuf_add(&tr2sid_buf, \"Localhost\", 9);\n \telse {\n-\t\talgo->init_fn(&ctx);\n+\t\tgit_hash_init(&ctx, algo);\n \t\tgit_hash_update(&ctx, hostname, strlen(hostname));\n \t\tgit_hash_final(hash, &ctx);\n \t\thash_to_hex_algop_r(hex, hash, algo);\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547437","messageId":"20260708035253.GB41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 2/7] hash: convert remaining direct function calls","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:52:53Z","receivedAt":"2026-07-08T03:52:54Z","isPatch":true,"body":"The previous patch added a coccinelle rule to make sure callers always\nuse git_hash_init() rather than direct function pointers from the algo\nstruct.\n\nLet's do the same for the rest of the git_hash_*() wrappers. I split\nthese out because they're a bit different: they implicitly use the algop\npointer in the git_hash_ctx. So when we convert:\n\n -algo->update_fn(&ctx, buf, len);\n +git_hash_update(&ctx, buf, len);\n\nwe drop the reference to algo entirely! But this is always going to be\nthe right thing. If \"algo\" does not match what is in ctx.algop, then\nwe'd already be invoking undefined behavior.\n\nSo in addition to making it possible to add more logic to the\ngit_hash_*() functions, we're avoiding the need to pass around the extra\nalgo pointer and make sure that it matches what's in \"ctx\".\n\nThe rest of the patch is the mechanical application of that coccinelle\npatch, plus a minor cleanup in test-synthesize.c to drop a now-unused\nfunction parameter (since we don't have to pass around the algo\nseparately anymore).\n\nSigned-off-by: Jeff King \n---\n builtin/submodule--helper.c | 8 +++---\n t/helper/test-synthesize.c | 29 ++++++++++----------\n tools/coccinelle/hash.cocci | 54 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex bf114a7856..510f193a15 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -551,10 +551,10 @@ static void create_default_gitdir_config(const char *submodule_name)\n \t/* Case 2.4: If all the above failed, try a hash of the name as a last resort */\n \theader_len = snprintf(header, sizeof(header), \"blob %zu\", strlen(submodule_name));\n \tgit_hash_init(&ctx, the_hash_algo);\n-\tthe_hash_algo->update_fn(&ctx, header, header_len);\n-\tthe_hash_algo->update_fn(&ctx, \"\\0\", 1);\n-\tthe_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name));\n-\tthe_hash_algo->final_fn(raw_name_hash, &ctx);\n+\tgit_hash_update(&ctx, header, header_len);\n+\tgit_hash_update(&ctx, \"\\0\", 1);\n+\tgit_hash_update(&ctx, submodule_name, strlen(submodule_name));\n+\tgit_hash_final(raw_name_hash, &ctx);\n \thash_to_hex_algop_r(hex_name_hash, raw_name_hash, the_hash_algo);\n \tstrbuf_reset(&gitdir_path);\n \trepo_git_path_append(the_repository, &gitdir_path, \"modules/%s\", hex_name_hash);\ndiff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c\nindex 7719fb3a76..fd116c87ba 100644\n--- a/t/helper/test-synthesize.c\n+++ b/t/helper/test-synthesize.c\n@@ -25,8 +25,7 @@ static const unsigned char zeros[BLOCK_SIZE];\n * Updates the pack checksum context.\n */\n static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n-\t\t\t\t const void *data, size_t len,\n-\t\t\t\t const struct git_hash_algo *algo)\n+\t\t\t\t const void *data, size_t len)\n {\n \tunsigned char zlib_header[2] = { 0x78, 0x01 }; /* CMF, FLG */\n \tunsigned char block_header[5];\n@@ -37,7 +36,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n \n \t/* Write zlib header */\n \tfwrite_or_die(f, zlib_header, sizeof(zlib_header));\n-\talgo->update_fn(pack_ctx, zlib_header, 2);\n+\tgit_hash_update(pack_ctx, zlib_header, 2);\n \n \t/* Write uncompressed blocks (max 64KB each) */\n \tdo {\n@@ -52,11 +51,11 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n \t\tblock_header[4] = block_header[2] ^ 0xff;\n \n \t\tfwrite_or_die(f, block_header, sizeof(block_header));\n-\t\talgo->update_fn(pack_ctx, block_header, 5);\n+\t\tgit_hash_update(pack_ctx, block_header, 5);\n \n \t\tif (block_len) {\n \t\t\tfwrite_or_die(f, block_data, block_len);\n-\t\t\talgo->update_fn(pack_ctx, block_data, block_len);\n+\t\t\tgit_hash_update(pack_ctx, block_data, block_len);\n \t\t\tadler = adler32(adler, block_data, block_len);\n \t\t}\n \n@@ -68,7 +67,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,\n \t/* Write adler32 checksum */\n \tput_be32(adler_buf, adler);\n \tfwrite_or_die(f, adler_buf, sizeof(adler_buf));\n-\talgo->update_fn(pack_ctx, adler_buf, 4);\n+\tgit_hash_update(pack_ctx, adler_buf, 4);\n }\n \n /*\n@@ -92,24 +91,24 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,\n \t\t\t\t\t\t sizeof(pack_header),\n \t\t\t\t\t\t type, len);\n \tfwrite_or_die(f, pack_header, pack_header_len);\n-\talgo->update_fn(pack_ctx, pack_header, pack_header_len);\n+\tgit_hash_update(pack_ctx, pack_header, pack_header_len);\n \n \t/* Write the data as uncompressed zlib */\n-\twrite_uncompressed_zlib(f, pack_ctx, data, len, algo);\n+\twrite_uncompressed_zlib(f, pack_ctx, data, len);\n \n \tgit_hash_init(&ctx, algo);\n \tobject_header_len = format_object_header(object_header,\n \t\t\t\t\t\t sizeof(object_header),\n \t\t\t\t\t\t type, len);\n-\talgo->update_fn(&ctx, object_header, object_header_len);\n+\tgit_hash_update(&ctx, object_header, object_header_len);\n \tif (data)\n-\t\talgo->update_fn(&ctx, data, len);\n+\t\tgit_hash_update(&ctx, data, len);\n \telse {\n \t\tfor (size_t i = len / BLOCK_SIZE; i; i--)\n-\t\t\talgo->update_fn(&ctx, zeros, BLOCK_SIZE);\n-\t\talgo->update_fn(&ctx, zeros, len % BLOCK_SIZE);\n+\t\t\tgit_hash_update(&ctx, zeros, BLOCK_SIZE);\n+\t\tgit_hash_update(&ctx, zeros, len % BLOCK_SIZE);\n \t}\n-\talgo->final_oid_fn(oid, &ctx);\n+\tgit_hash_final_oid(oid, &ctx);\n }\n \n /*\n@@ -434,7 +433,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,\n \n \t/* Write pack header */\n \tfwrite_or_die(f, &pack_header, sizeof(pack_header));\n-\talgo->update_fn(&pack_ctx, &pack_header, sizeof(pack_header));\n+\tgit_hash_update(&pack_ctx, &pack_header, sizeof(pack_header));\n \n \t/* 1. Write the large blob */\n \twrite_pack_object(f, &pack_ctx, OBJ_BLOB, NULL, blob_size, &blob_oid, algo);\n@@ -472,7 +471,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,\n \twrite_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &final_commit_oid, algo);\n \n \t/* Write pack trailer (checksum) */\n-\talgo->final_fn(pack_hash, &pack_ctx);\n+\tgit_hash_final(pack_hash, &pack_ctx);\n \tfwrite_or_die(f, pack_hash, algo->rawsz);\n \tif (fclose(f))\n \t\tdie_errno(_(\"could not close '%s'\"), path);\ndiff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci\nindex 04270ee043..d0e2e5f4b1 100644\n--- a/tools/coccinelle/hash.cocci\n+++ b/tools/coccinelle/hash.cocci\n@@ -7,3 +7,57 @@ struct git_hash_ctx *CTX;\n - ALGO->init_fn(CTX);\n + git_hash_init(CTX, ALGO);\n ...>}\n+\n+@@\n+identifier f != git_hash_clone;\n+expression ALGO;\n+struct git_hash_ctx *SRC;\n+struct git_hash_ctx *DST;\n+@@\n+ f(...) {<...\n+- ALGO->clone_fn(DST, SRC);\n++ git_hash_clone(DST, SRC);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_update;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+expression list ARGS;\n+@@\n+ f(...) {<...\n+- ALGO->update_fn(CTX, ARGS);\n++ git_hash_update(CTX, ARGS);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_final;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+expression list ARGS;\n+@@\n+ f(...) {<...\n+- ALGO->final_fn(ARGS, CTX);\n++ git_hash_final(ARGS, CTX);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_final_oid;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+expression list ARGS;\n+@@\n+ f(...) {<...\n+- ALGO->final_oid_fn(ARGS, CTX);\n++ git_hash_final_oid(ARGS, CTX);\n+ ...>}\n+\n+@@\n+identifier f != git_hash_discard;\n+expression ALGO;\n+struct git_hash_ctx *CTX;\n+@@\n+ f(...) {<...\n+- ALGO->discard_fn(CTX);\n++ git_hash_discard(CTX);\n+ ...>}\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547438","messageId":"20260708035255.GC41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 3/7] hash: document function pointers and wrappers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:52:55Z","receivedAt":"2026-07-08T03:52:56Z","isPatch":true,"body":"We want people to use the git_hash_*() wrappers rather than the bare\nfunction pointers in the git_hash_algo struct. Let's document them\nrather than the bare pointers, and warn people away from the pointers.\nCoccinelle will eventually force the use of the wrappers, but it's\nhelpful to lead readers in the right direction from the start.\n\nWhile we're here we can document a few other bits of wisdom I've turned\nup while working in this area:\n\n - You have to initialize the destination of a git_hash_clone(). This\n is something we may eventually change for efficiency, but we should\n definitely document the requirement for now.\n\n - You must eventually finalize or discard a hash, since some backends\n may allocate resources during initialization.\n\nSigned-off-by: Jeff King \n---\n hash.h | 43 ++++++++++++++++++++++++++++++++-----------\n 1 file changed, 32 insertions(+), 11 deletions(-)\n\ndiff --git a/hash.h b/hash.h\nindex 0a23ef4dfd..121ecf13aa 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -309,22 +309,15 @@ struct git_hash_algo {\n \t/* The block size of the hash. */\n \tsize_t blksz;\n \n-\t/* The hash initialization function. */\n+\t/*\n+\t * Low-level implementation hooks. Callers should use the git_hash_*\n+\t * wrappers below rather than invoking these directly.\n+\t */\n \tgit_hash_init_fn init_fn;\n-\n-\t/* The hash context cloning function. */\n \tgit_hash_clone_fn clone_fn;\n-\n-\t/* The hash update function. */\n \tgit_hash_update_fn update_fn;\n-\n-\t/* The hash finalization function. */\n \tgit_hash_final_fn final_fn;\n-\n-\t/* The hash finalization function for object IDs. */\n \tgit_hash_final_oid_fn final_oid_fn;\n-\n-\t/* Discard an initialized hash without finalizing. */\n \tgit_hash_discard_fn discard_fn;\n \n \t/* The OID of the empty tree. */\n@@ -341,12 +334,40 @@ struct git_hash_algo {\n };\n extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];\n \n+/*\n+ * Prepare an uninitialized hash context for use. You must eventually release\n+ * the context with git_hash_final() (or final_oid()) or by calling\n+ * git_hash_discard().\n+ */\n void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop);\n+\n+/*\n+ * Clone the state of a hash. Both src and dst must have been initialized with\n+ * git_hash_init().\n+ */\n void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src);\n+\n+/*\n+ * Add more data to an initialized hash context.\n+ */\n void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len);\n+\n+/*\n+ * Retrieve the final hash value from a context, releasing any resources.\n+ */\n void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx);\n+\n+/*\n+ * Like git_hash_final(), but write the result into an object_id.\n+ */\n void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx);\n+\n+/*\n+ * Discard a hash context without computing the final value, but still\n+ * releasing any resources.\n+ */\n void git_hash_discard(struct git_hash_ctx *ctx);\n+\n const struct git_hash_algo *hash_algo_ptr_by_number(uint32_t algo);\n struct git_hash_ctx *git_hash_alloc(void);\n void git_hash_free(struct git_hash_ctx *ctx);\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547439","messageId":"20260708035257.GD41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 4/7] hash: make git_hash_discard() idempotent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:52:57Z","receivedAt":"2026-07-08T03:52:59Z","isPatch":true,"body":"You must always either finalize or discard a hash context to release any\nresources, but you must call only one such function. This creates extra\nwork for some callers, since their cleanup code paths need to know\nwhether they got there via their happy path (and the finalization\nhappened) or due to an error (in which case they need to discard).\n\nLet's add an \"active\" flag that turns a redundant discard into a noop.\nThat lets you safely do this:\n\n git_hash_init(&ctx, algo);\n ...\n if (some_error)\n goto out;\n ...\n git_hash_final(result, &ctx);\n\n out:\n git_hash_discard(&ctx);\n\nThis should avoid future errors, and will also let us simplify a few\nexisting callers (in future patches).\n\nSigned-off-by: Jeff King \n---\n hash.c | 6 ++++++\n hash.h | 1 +\n 2 files changed, 7 insertions(+)\n\ndiff --git a/hash.c b/hash.c\nindex 55d1d41770..b1296f0018 100644\n--- a/hash.c\n+++ b/hash.c\n@@ -285,6 +285,7 @@ void git_hash_free(struct git_hash_ctx *ctx)\n void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)\n {\n \talgop->init_fn(ctx);\n+\tctx->active = true;\n }\n \n void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)\n@@ -300,16 +301,21 @@ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)\n void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)\n {\n \tctx->algop->final_fn(hash, ctx);\n+\tctx->active = false;\n }\n \n void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)\n {\n \tctx->algop->final_oid_fn(oid, ctx);\n+\tctx->active = false;\n }\n \n void git_hash_discard(struct git_hash_ctx *ctx)\n {\n+\tif (!ctx->active)\n+\t\treturn;\n \tctx->algop->discard_fn(ctx);\n+\tctx->active = false;\n }\n \n uint32_t hash_algo_by_name(const char *name)\ndiff --git a/hash.h b/hash.h\nindex 121ecf13aa..cf94ad5700 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -281,6 +281,7 @@ struct git_hash_ctx {\n \t\tgit_SHA_CTX_unsafe sha1_unsafe;\n \t\tgit_SHA256_CTX sha256;\n \t} state;\n+\tbool active;\n };\n \n typedef void (*git_hash_init_fn)(struct git_hash_ctx *ctx);\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547440","messageId":"20260708035300.GE41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 5/7] csum-file: use idempotent git_hash_discard()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:53:00Z","receivedAt":"2026-07-08T03:53:01Z","isPatch":true,"body":"Now that it is safe to call git_hash_discard() even after finalizing it,\nwe can simplify our cleanup logic a bit. This is mostly undoing a few\nbits of 64337aecde (csum-file: always finalize or discard hash,\n2026-07-02):\n\n - We no longer need a separate free_hashfile_memory() function for\n finalize_hashfile(). It can just call free_hashfile(), which will\n now discard (or not) the hash as appropriate.\n\n - When f->skip_hash is set, we don't need to discard; we can rely on\n free_hashfile() to do it.\n\nSigned-off-by: Jeff King \n---\n csum-file.c | 17 +++++------------\n 1 file changed, 5 insertions(+), 12 deletions(-)\n\ndiff --git a/csum-file.c b/csum-file.c\nindex 7e81391524..fe18ee1de3 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -55,32 +55,25 @@ void hashflush(struct hashfile *f)\n \t}\n }\n \n-static void free_hashfile_memory(struct hashfile *f)\n+void free_hashfile(struct hashfile *f)\n {\n+\tgit_hash_discard(&f->ctx);\n \tfree(f->buffer);\n \tfree(f->check_buffer);\n \tfree(f);\n }\n \n-void free_hashfile(struct hashfile *f)\n-{\n-\tgit_hash_discard(&f->ctx);\n-\tfree_hashfile_memory(f);\n-}\n-\n int finalize_hashfile(struct hashfile *f, unsigned char *result,\n \t\t enum fsync_component component, unsigned int flags)\n {\n \tint fd;\n \n \thashflush(f);\n \n-\tif (f->skip_hash) {\n-\t\tgit_hash_discard(&f->ctx);\n+\tif (f->skip_hash)\n \t\thashclr(f->buffer, f->algop);\n-\t} else {\n+\telse\n \t\tgit_hash_final(f->buffer, &f->ctx);\n-\t}\n \n \tif (result)\n \t\thashcpy(result, f->buffer, f->algop);\n@@ -105,7 +98,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result,\n \t\tif (close(f->check_fd))\n \t\t\tdie_errno(\"%s: sha1 file error on close\", f->name);\n \t}\n-\tfree_hashfile_memory(f);\n+\tfree_hashfile(f);\n \treturn fd;\n }\n \n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547441","messageId":"20260708035302.GF41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 6/7] http: use idempotent git_hash_discard()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:53:02Z","receivedAt":"2026-07-08T03:53:03Z","isPatch":true,"body":"Now that it is OK to call git_hash_discard() even after finalizing the\nhash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http:\ndiscard hash in dumb-http http_object_request, 2026-07-02).\n\nSigned-off-by: Jeff King \n---\n http.c | 5 +----\n http.h | 1 -\n 2 files changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 0341de5031..caccf2108e 100644\n--- a/http.c\n+++ b/http.c\n@@ -2880,7 +2880,6 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \tgit_inflate_init(&freq->stream);\n \n \tgit_hash_init(&freq->c, the_hash_algo);\n-\tfreq->hash_ctx_valid = 1;\n \n \tfreq->url = get_remote_object_url(base_url, hex, 0);\n \n@@ -2989,7 +2988,6 @@ int finish_http_object_request(struct http_object_request *freq)\n \t}\n \n \tgit_hash_final_oid(&freq->real_oid, &freq->c);\n-\tfreq->hash_ctx_valid = 0;\n \tif (freq->zret != Z_STREAM_END) {\n \t\tunlink_or_warn(freq->tmpfile.buf);\n \t\treturn -1;\n@@ -3030,8 +3028,7 @@ void release_http_object_request(struct http_object_request **freq_p)\n \tcurl_slist_free_all(freq->headers);\n \tstrbuf_release(&freq->tmpfile);\n \tgit_inflate_end(&freq->stream);\n-\tif (freq->hash_ctx_valid)\n-\t\tgit_hash_discard(&freq->c);\n+\tgit_hash_discard(&freq->c);\n \n \tfree(freq);\n \t*freq_p = NULL;\ndiff --git a/http.h b/http.h\nindex 6b0639150f..729c51904d 100644\n--- a/http.h\n+++ b/http.h\n@@ -255,7 +255,6 @@ struct http_object_request {\n \tstruct object_id oid;\n \tstruct object_id real_oid;\n \tstruct git_hash_ctx c;\n-\tint hash_ctx_valid;\n \tgit_zstream stream;\n \tint zret;\n \tint rename;\n-- \n2.55.0.459.g1b256877c9\n\n"},{"id":"547442","messageId":"20260708035305.GG41620@coredump.intra.peff.net","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"[PATCH v2 7/7] hash: check ctx->active flag in all wrapper functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:53:05Z","receivedAt":"2026-07-08T03:53:06Z","isPatch":true,"body":"It only makes sense to call git_hash_update(), etc, on a hash context\nthat has been initialized but not yet finalized or discarded. This is an\nunlikely error to make, but it's easy for us to catch it and complain.\n\nIt's especially important because it would quietly \"work\" for many hash\nbackends (like sha1dc, which is just manipulating some bytes) but would\ncause undefined behavior with others (like OpenSSL, which puts the\ncontext onto the heap). Checking the flag lets us catch problems\nconsistently on every build.\n\nNote that we can't do the same for git_hash_init(). Even though it would\ncause a leak to call it twice (without an intervening final/discard),\nthe point of the function is that the contents of the struct are\nundefined before the call. But calling it twice is an even less likely\nerror to make, so not covering it is OK.\n\nWe leave git_hash_discard() alone, as its idempotent behavior is\nconvenient for callers. We _could_ try to do something similar for\ngit_hash_final(), allowing:\n\n git_hash_final(result, &ctx);\n git_hash_final(other_result, &ctx);\n\nbut it does not make much sense. After the first final() call we have\nthrown away the state, so we cannot produce the same output. We could\ncome up with some sensible output (the null hash, or the empty hash),\nbut double-calls like this are more likely a bug, so our best bet is to\ncomplain loudly (whereas the current code produces either nonsense\noutput or undefined behavior, depending on the backend).\n\nSigned-off-by: Jeff King \n---\n hash.c | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/hash.c b/hash.c\nindex b1296f0018..82f7e24404 100644\n--- a/hash.c\n+++ b/hash.c\n@@ -290,22 +290,32 @@ void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)\n \n void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)\n {\n+\tif (!src->active)\n+\t\tBUG(\"attempt to copy from an inactive hash context\");\n+\tif (!dst->active)\n+\t\tBUG(\"attempt to copy to an inactive hash context\");\n \tsrc->algop->clone_fn(dst, src);\n }\n \n void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)\n {\n+\tif (!ctx->active)\n+\t\tBUG(\"attempt to update an inactive hash context\");\n \tctx->algop->update_fn(ctx, in, len);\n }\n \n void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)\n {\n+\tif (!ctx->active)\n+\t\tBUG(\"attempt to finalize an inactive hash context\");\n \tctx->algop->final_fn(hash, ctx);\n \tctx->active = false;\n }\n \n void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)\n {\n+\tif (!ctx->active)\n+\t\tBUG(\"attempt to finalize an inactive hash context\");\n \tctx->algop->final_oid_fn(oid, ctx);\n \tctx->active = false;\n }\n-- \n2.55.0.459.g1b256877c9\n"},{"id":"547443","messageId":"20260708035439.GA41684@coredump.intra.peff.net","threadId":"65938","inReplyTo":"ak1u25b2pmRAQIxD@fruit.crustytoothpaste.net","subject":"Re: [PATCH 1/7] hash: use git_hash_init() consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-08T03:54:39Z","receivedAt":"2026-07-08T03:54:40Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 09:25:48PM +0000, brian m. carlson wrote:\n\n> On 2026-07-07 at 05:01:41, Jeff King wrote:\n> > We'd like to add more logic to git_hash_init(), but many callers skip it\n> > and call algop->init_fn() directly. Let's make sure we're consistently\n> > using the wrapper by adding a coccinelle rule.\n> > \n> > Besides the coccinelle file itself, this is a purely mechanical\n> > conversion based on the patch it generates. There should be no bare\n> > init_fn() calls left (except for the one in the wrapper).\n> \n> For context, the reason `git_hash_init` exists is that our Rust code\n> needs to initialize a hash context but it treats `const struct\n> git_hash_algo *` as `const void *` and doesn't have any access to the\n> contents of the structure. We could fix this with `cbindgen` and\n> `bindgen`, but haven't done so yet.\n> \n> So that's why everybody has been using `init_fn` instead of\n> `git_hash_init`. Anyway, I have no objections to making this the\n> standard interface going forward.\n\nThanks, I remember there being some actual reason but couldn't recall\nexactly what it was. The use of bare algo->update_fn(), etc, in two\nspots was what really puzzled me. It's not wrong, but just harder to\nwrite than the usual way. ;)\n\n-Peff\n"},{"id":"547477","messageId":"ak4E4-jmgYFSI75O@pks.im","threadId":"65938","inReplyTo":"20260708035235.GA41491@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/7] git_hash_*() quality-of-life improvements","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-08T08:05:55Z","receivedAt":"2026-07-08T08:06:06Z","isPatch":true,"body":"On Tue, Jul 07, 2026 at 11:52:35PM -0400, Jeff King wrote:\n> On Tue, Jul 07, 2026 at 12:55:57AM -0400, Jeff King wrote:\n> \n> > This implements the \"idempotent git_hash_discard()\" discussed in this\n> > subthread:\n> > \n> > https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/\n> > \n> > with associated cleanups.\n> \n> Here's a v2 addressing the comments so far. Mostly minor changes:\n> \n> - fixed typos noticed by Patrick\n> \n> - dropped extra braces added by coccinelle\n> \n> - dropped a trailing blank line from patch 1 (this gets fixed in a\n> later patch as we add more content after the blank line, but I\n> noticed \"git apply\" complaining)\n> \n> - a bit more explanation in patch 7 about why we don't support\n> idempotent final() calls\n\nAll of these changes look good to me, and the range-diff matches what\nyou describe here. So this series looks good to me, thanks!\n\nPatrick\n"}]}