From 9b204b825bcaf656ea6a2cb0657ef907db21a0e6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:52:49 -0400 Subject: [PATCH 1/7] hash: use git_hash_init() consistently We'd like to add more logic to git_hash_init(), but many callers skip it and call algop->init_fn() directly. Let's make sure we're consistently using the wrapper by adding a coccinelle rule. Besides the coccinelle file itself, this is a purely mechanical conversion based on the patch it generates. There should be no bare init_fn() calls left (except for the one in the wrapper). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 4 ++-- builtin/index-pack.c | 6 +++--- builtin/patch-id.c | 2 +- builtin/receive-pack.c | 6 +++--- builtin/submodule--helper.c | 2 +- builtin/unpack-objects.c | 4 ++-- csum-file.c | 6 +++--- diff.c | 4 ++-- http-push.c | 2 +- http.c | 4 ++-- object-file.c | 14 +++++++------- pack-check.c | 2 +- pack-write.c | 6 +++--- read-cache.c | 6 +++--- rerere.c | 2 +- t/helper/test-hash-speed.c | 2 +- t/helper/test-hash.c | 2 +- t/helper/test-synthesize.c | 4 ++-- t/unit-tests/u-hash.c | 2 +- tools/coccinelle/hash.cocci | 9 +++++++++ trace2/tr2_sid.c | 2 +- 21 files changed, 50 insertions(+), 41 deletions(-) create mode 100644 tools/coccinelle/hash.cocci diff --git a/builtin/fast-import.c b/builtin/fast-import.c index f6473dcc8e..6692f7cd81 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -969,7 +969,7 @@ static int store_object( hdrlen = format_object_header((char *)hdr, sizeof(hdr), type, dat->len); - the_hash_algo->init_fn(&c); + git_hash_init(&c, the_hash_algo); git_hash_update(&c, hdr, hdrlen); git_hash_update(&c, dat->buf, dat->len); git_hash_final_oid(&oid, &c); @@ -1131,7 +1131,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark) hdrlen = format_object_header((char *)out_buf, out_sz, OBJ_BLOB, len); - the_hash_algo->init_fn(&c); + git_hash_init(&c, the_hash_algo); git_hash_update(&c, out_buf, hdrlen); crc32_begin(pack_file); diff --git a/builtin/index-pack.c b/builtin/index-pack.c index f396658468..53a8cb9dd7 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -374,7 +374,7 @@ static const char *open_pack_file(const char *pack_name) output_fd = -1; nothread_data.pack_fd = input_fd; } - the_hash_algo->init_fn(&input_ctx); + git_hash_init(&input_ctx, the_hash_algo); return pack_name; } @@ -481,7 +481,7 @@ static void *unpack_entry_data(off_t offset, size_t size, if (!is_delta_type(type)) { hdrlen = format_object_header(hdr, sizeof(hdr), type, size); - the_hash_algo->init_fn(&c); + git_hash_init(&c, the_hash_algo); git_hash_update(&c, hdr, hdrlen); } else oid = NULL; @@ -1291,7 +1291,7 @@ static void parse_pack_objects(unsigned char *hash) /* Check pack integrity */ flush(); - the_hash_algo->init_fn(&tmp_ctx); + git_hash_init(&tmp_ctx, the_hash_algo); git_hash_clone(&tmp_ctx, &input_ctx); git_hash_final(hash, &tmp_ctx); if (!hasheq(fill(the_hash_algo->rawsz), hash, the_repository->hash_algo)) diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 57d9bd4a65..22f36ecf80 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -73,7 +73,7 @@ static size_t get_one_patchid(struct object_id *next_oid, struct object_id *resu char pre_oid_str[GIT_MAX_HEXSZ + 1], post_oid_str[GIT_MAX_HEXSZ + 1]; struct git_hash_ctx ctx; - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); oidclr(result, the_repository->hash_algo); while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) { diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 19eb6a1b61..faf0f120ac 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -615,7 +615,7 @@ static void hmac_hash(unsigned char *out, /* RFC 2104 2. (1) */ memset(key, '\0', GIT_MAX_BLKSZ); if (the_hash_algo->blksz < key_len) { - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); git_hash_update(&ctx, key_in, key_len); git_hash_final(key, &ctx); } else { @@ -629,13 +629,13 @@ static void hmac_hash(unsigned char *out, } /* RFC 2104 2. (3) & (4) */ - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); git_hash_update(&ctx, k_ipad, sizeof(k_ipad)); git_hash_update(&ctx, text, text_len); git_hash_final(out, &ctx); /* RFC 2104 2. (6) & (7) */ - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); git_hash_update(&ctx, k_opad, sizeof(k_opad)); git_hash_update(&ctx, out, the_hash_algo->rawsz); git_hash_final(out, &ctx); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1cc82a134d..bf114a7856 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -550,7 +550,7 @@ static void create_default_gitdir_config(const char *submodule_name) /* Case 2.4: If all the above failed, try a hash of the name as a last resort */ header_len = snprintf(header, sizeof(header), "blob %zu", strlen(submodule_name)); - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); the_hash_algo->update_fn(&ctx, header, header_len); the_hash_algo->update_fn(&ctx, "\0", 1); the_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name)); diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index f3849bb654..93a9caa582 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -670,10 +670,10 @@ int cmd_unpack_objects(int argc, /* We don't take any non-flag arguments now.. Maybe some day */ usage(unpack_usage); } - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); unpack_all(); git_hash_update(&ctx, buffer, offset); - the_hash_algo->init_fn(&tmp_ctx); + git_hash_init(&tmp_ctx, the_hash_algo); git_hash_clone(&tmp_ctx, &ctx); git_hash_final_oid(&oid, &tmp_ctx); if (strict) { diff --git a/csum-file.c b/csum-file.c index b166f89624..7e81391524 100644 --- a/csum-file.c +++ b/csum-file.c @@ -175,7 +175,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop, f->skip_hash = 0; f->algop = unsafe_hash_algo(algop); - f->algop->init_fn(&f->ctx); + git_hash_init(&f->ctx, f->algop); f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE; f->buffer = xmalloc(f->buffer_len); @@ -200,7 +200,7 @@ void hashfile_checkpoint_init(struct hashfile *f, struct hashfile_checkpoint *checkpoint) { memset(checkpoint, 0, sizeof(*checkpoint)); - f->algop->init_fn(&checkpoint->ctx); + git_hash_init(&checkpoint->ctx, f->algop); } void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpoint) @@ -252,7 +252,7 @@ int hashfile_checksum_valid(const struct git_hash_algo *algop, if (total_len < algop->rawsz) return 0; /* say "too short"? */ - algop->init_fn(&ctx); + git_hash_init(&ctx, algop); git_hash_update(&ctx, data, data_len); git_hash_final(got, &ctx); diff --git a/diff.c b/diff.c index 1568f0ed9c..589c1969e4 100644 --- a/diff.c +++ b/diff.c @@ -6855,7 +6855,7 @@ void flush_one_hunk(struct object_id *result, struct git_hash_ctx *ctx) int i; git_hash_final(hash, ctx); - the_hash_algo->init_fn(ctx); + git_hash_init(ctx, the_hash_algo); /* 20-byte sum, with carry */ for (i = 0; i < the_hash_algo->rawsz; ++i) { carry += result->hash[i] + hash[i]; @@ -6899,7 +6899,7 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid struct git_hash_ctx ctx; struct patch_id_t data; - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); memset(&data, 0, sizeof(struct patch_id_t)); data.ctx = &ctx; oidclr(oid, the_repository->hash_algo); diff --git a/http-push.c b/http-push.c index 3c23cbba27..60f6f8f054 100644 --- a/http-push.c +++ b/http-push.c @@ -776,7 +776,7 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed) } else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) { lock->token = xstrdup(ctx->cdata); - the_hash_algo->init_fn(&hash_ctx); + git_hash_init(&hash_ctx, the_hash_algo); git_hash_update(&hash_ctx, lock->token, strlen(lock->token)); git_hash_final(lock_token_hash, &hash_ctx); diff --git a/http.c b/http.c index 63abbaae8a..0341de5031 100644 --- a/http.c +++ b/http.c @@ -2879,7 +2879,7 @@ struct http_object_request *new_http_object_request(const char *base_url, git_inflate_init(&freq->stream); - the_hash_algo->init_fn(&freq->c); + git_hash_init(&freq->c, the_hash_algo); freq->hash_ctx_valid = 1; freq->url = get_remote_object_url(base_url, hex, 0); @@ -2916,7 +2916,7 @@ struct http_object_request *new_http_object_request(const char *base_url, git_inflate_end(&freq->stream); memset(&freq->stream, 0, sizeof(freq->stream)); git_inflate_init(&freq->stream); - the_hash_algo->init_fn(&freq->c); + git_hash_init(&freq->c, the_hash_algo); if (prev_posn>0) { prev_posn = 0; lseek(freq->localfile, 0, SEEK_SET); diff --git a/object-file.c b/object-file.c index 035d005279..304b83f859 100644 --- a/object-file.c +++ b/object-file.c @@ -124,7 +124,7 @@ int stream_object_signature(struct repository *r, hdrlen = format_object_header(hdr, sizeof(hdr), st->type, st->size); /* Sha1.. */ - r->hash_algo->init_fn(&c); + git_hash_init(&c, r->hash_algo); git_hash_update(&c, hdr, hdrlen); for (;;) { char buf[1024 * 16]; @@ -320,7 +320,7 @@ static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_c struct object_id *oid, char *hdr, int *hdrlen) { - algo->init_fn(c); + git_hash_init(c, algo); git_hash_update(c, hdr, *hdrlen); git_hash_update(c, buf, len); git_hash_final_oid(oid, c); @@ -681,9 +681,9 @@ static int start_loose_object_common(struct odb_source_loose *loose, git_deflate_init(stream, cfg->zlib_compression_level); stream->next_out = buf; stream->avail_out = buflen; - algo->init_fn(c); + git_hash_init(c, algo); if (compat && compat_c) - compat->init_fn(compat_c); + git_hash_init(compat_c, compat); /* Start to feed header to zlib stream */ stream->next_in = (unsigned char *)hdr; @@ -1141,7 +1141,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, header_len = format_object_header((char *)buf, sizeof(buf), OBJ_BLOB, size); - hash_algo->init_fn(&ctx); + git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); while (!stream->is_finished) { @@ -1313,7 +1313,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas header_len = format_object_header((char *)obuf, sizeof(obuf), OBJ_BLOB, size); - transaction->base.source->odb->repo->hash_algo->init_fn(&ctx); + git_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo); git_hash_update(&ctx, obuf, header_len); /* @@ -1560,7 +1560,7 @@ static int check_stream_oid(git_zstream *stream, unsigned long total_read; int status = Z_OK; - algop->init_fn(&c); + git_hash_init(&c, algop); git_hash_update(&c, hdr, stream->total_out); /* diff --git a/pack-check.c b/pack-check.c index 5adfb3f272..c3b8db7c5c 100644 --- a/pack-check.c +++ b/pack-check.c @@ -69,7 +69,7 @@ static int verify_packfile(struct repository *r, if (!is_pack_valid(p)) return error("packfile %s cannot be accessed", p->pack_name); - r->hash_algo->init_fn(&ctx); + git_hash_init(&ctx, r->hash_algo); do { unsigned long remaining; unsigned char *in = use_pack(p, w_curs, offset, &remaining); diff --git a/pack-write.c b/pack-write.c index 83eaf88541..24033a9101 100644 --- a/pack-write.c +++ b/pack-write.c @@ -402,8 +402,8 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo, char *buf; ssize_t read_result; - hash_algo->init_fn(&old_hash_ctx); - hash_algo->init_fn(&new_hash_ctx); + git_hash_init(&old_hash_ctx, hash_algo); + git_hash_init(&new_hash_ctx, hash_algo); if (lseek(pack_fd, 0, SEEK_SET) != 0) die_errno("Failed seeking to start of '%s'", pack_name); @@ -455,7 +455,7 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo, * pack, which also means making partial_pack_offset * big enough not to matter anymore. */ - hash_algo->init_fn(&old_hash_ctx); + git_hash_init(&old_hash_ctx, hash_algo); partial_pack_offset = ~partial_pack_offset; partial_pack_offset -= MSB(partial_pack_offset, 1); } diff --git a/read-cache.c b/read-cache.c index 21ca58beea..1d3d9c119c 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1721,7 +1721,7 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size) if (oideq(&oid, null_oid(the_hash_algo))) return 0; - the_hash_algo->init_fn(&c); + git_hash_init(&c, the_hash_algo); git_hash_update(&c, hdr, size - the_hash_algo->rawsz); git_hash_final(hash, &c); if (!hasheq(hash, start, the_repository->hash_algo)) @@ -2956,7 +2956,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, */ if (offset && record_eoie()) { CALLOC_ARRAY(eoie_c, 1); - the_hash_algo->init_fn(eoie_c); + git_hash_init(eoie_c, the_hash_algo); } /* @@ -3597,7 +3597,7 @@ static size_t read_eoie_extension(const char *mmap, size_t mmap_size) * "REUC" + ) */ src_offset = offset; - the_hash_algo->init_fn(&c); + git_hash_init(&c, the_hash_algo); while (src_offset < mmap_size - the_hash_algo->rawsz - EOIE_SIZE_WITH_HEADER) { /* After an array of active_nr index entries, * there can be arbitrary number of extended diff --git a/rerere.c b/rerere.c index 8232542585..216100925a 100644 --- a/rerere.c +++ b/rerere.c @@ -439,7 +439,7 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz struct strbuf buf = STRBUF_INIT, out = STRBUF_INIT; int has_conflicts = 0; if (hash) - the_hash_algo->init_fn(&ctx); + git_hash_init(&ctx, the_hash_algo); while (!io->getline(&buf, io)) { if (is_cmarker(buf.buf, '<', marker_size)) { diff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c index fbf67fe6bd..89b0268011 100644 --- a/t/helper/test-hash-speed.c +++ b/t/helper/test-hash-speed.c @@ -5,7 +5,7 @@ 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) { - algo->init_fn(ctx); + git_hash_init(ctx, algo); git_hash_update(ctx, p, len); git_hash_final(final, ctx); } diff --git a/t/helper/test-hash.c b/t/helper/test-hash.c index f0ee61c8b4..1f7163695f 100644 --- a/t/helper/test-hash.c +++ b/t/helper/test-hash.c @@ -29,7 +29,7 @@ int cmd_hash_impl(int ac, const char **av, int algo, int unsafe) die("OOPS"); } - algop->init_fn(&ctx); + git_hash_init(&ctx, algop); while (1) { ssize_t sz, this_sz; diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c index 3fa534fbdf..7719fb3a76 100644 --- a/t/helper/test-synthesize.c +++ b/t/helper/test-synthesize.c @@ -97,7 +97,7 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx, /* Write the data as uncompressed zlib */ write_uncompressed_zlib(f, pack_ctx, data, len, algo); - algo->init_fn(&ctx); + git_hash_init(&ctx, algo); object_header_len = format_object_header(object_header, sizeof(object_header), type, len); @@ -430,7 +430,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size, f = xfopen(path, "wb"); - algo->init_fn(&pack_ctx); + git_hash_init(&pack_ctx, algo); /* Write pack header */ fwrite_or_die(f, &pack_header, sizeof(pack_header)); diff --git a/t/unit-tests/u-hash.c b/t/unit-tests/u-hash.c index bd4ac6a6e1..19f4efd410 100644 --- a/t/unit-tests/u-hash.c +++ b/t/unit-tests/u-hash.c @@ -12,7 +12,7 @@ static void check_hash_data(const void *data, size_t data_length, unsigned char hash[GIT_MAX_HEXSZ]; const struct git_hash_algo *algop = &hash_algos[i]; - algop->init_fn(&ctx); + git_hash_init(&ctx, algop); git_hash_update(&ctx, data, data_length); git_hash_final(hash, &ctx); diff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci new file mode 100644 index 0000000000..04270ee043 --- /dev/null +++ b/tools/coccinelle/hash.cocci @@ -0,0 +1,9 @@ +@@ +identifier f != git_hash_init; +expression ALGO; +struct git_hash_ctx *CTX; +@@ + f(...) {<... +- ALGO->init_fn(CTX); ++ git_hash_init(CTX, ALGO); + ...>} diff --git a/trace2/tr2_sid.c b/trace2/tr2_sid.c index 1c1d27b0ee..131b4f5a62 100644 --- a/trace2/tr2_sid.c +++ b/trace2/tr2_sid.c @@ -45,7 +45,7 @@ static void tr2_sid_append_my_sid_component(void) if (xgethostname(hostname, sizeof(hostname))) strbuf_add(&tr2sid_buf, "Localhost", 9); else { - algo->init_fn(&ctx); + git_hash_init(&ctx, algo); git_hash_update(&ctx, hostname, strlen(hostname)); git_hash_final(hash, &ctx); hash_to_hex_algop_r(hex, hash, algo); From b87af5aa77c07f014e14c91ac5f942fdef132df9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:52:53 -0400 Subject: [PATCH 2/7] hash: convert remaining direct function calls The previous patch added a coccinelle rule to make sure callers always use git_hash_init() rather than direct function pointers from the algo struct. Let's do the same for the rest of the git_hash_*() wrappers. I split these out because they're a bit different: they implicitly use the algop pointer in the git_hash_ctx. So when we convert: -algo->update_fn(&ctx, buf, len); +git_hash_update(&ctx, buf, len); we drop the reference to algo entirely! But this is always going to be the right thing. If "algo" does not match what is in ctx.algop, then we'd already be invoking undefined behavior. So in addition to making it possible to add more logic to the git_hash_*() functions, we're avoiding the need to pass around the extra algo pointer and make sure that it matches what's in "ctx". The rest of the patch is the mechanical application of that coccinelle patch, plus a minor cleanup in test-synthesize.c to drop a now-unused function parameter (since we don't have to pass around the algo separately anymore). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 8 +++--- t/helper/test-synthesize.c | 29 ++++++++++---------- tools/coccinelle/hash.cocci | 54 +++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index bf114a7856..510f193a15 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -551,10 +551,10 @@ static void create_default_gitdir_config(const char *submodule_name) /* Case 2.4: If all the above failed, try a hash of the name as a last resort */ header_len = snprintf(header, sizeof(header), "blob %zu", strlen(submodule_name)); git_hash_init(&ctx, the_hash_algo); - the_hash_algo->update_fn(&ctx, header, header_len); - the_hash_algo->update_fn(&ctx, "\0", 1); - the_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name)); - the_hash_algo->final_fn(raw_name_hash, &ctx); + git_hash_update(&ctx, header, header_len); + git_hash_update(&ctx, "\0", 1); + git_hash_update(&ctx, submodule_name, strlen(submodule_name)); + git_hash_final(raw_name_hash, &ctx); hash_to_hex_algop_r(hex_name_hash, raw_name_hash, the_hash_algo); strbuf_reset(&gitdir_path); repo_git_path_append(the_repository, &gitdir_path, "modules/%s", hex_name_hash); diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c index 7719fb3a76..fd116c87ba 100644 --- a/t/helper/test-synthesize.c +++ b/t/helper/test-synthesize.c @@ -25,8 +25,7 @@ static const unsigned char zeros[BLOCK_SIZE]; * Updates the pack checksum context. */ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx, - const void *data, size_t len, - const struct git_hash_algo *algo) + const void *data, size_t len) { unsigned char zlib_header[2] = { 0x78, 0x01 }; /* CMF, FLG */ unsigned char block_header[5]; @@ -37,7 +36,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx, /* Write zlib header */ fwrite_or_die(f, zlib_header, sizeof(zlib_header)); - algo->update_fn(pack_ctx, zlib_header, 2); + git_hash_update(pack_ctx, zlib_header, 2); /* Write uncompressed blocks (max 64KB each) */ do { @@ -52,11 +51,11 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx, block_header[4] = block_header[2] ^ 0xff; fwrite_or_die(f, block_header, sizeof(block_header)); - algo->update_fn(pack_ctx, block_header, 5); + git_hash_update(pack_ctx, block_header, 5); if (block_len) { fwrite_or_die(f, block_data, block_len); - algo->update_fn(pack_ctx, block_data, block_len); + git_hash_update(pack_ctx, block_data, block_len); adler = adler32(adler, block_data, block_len); } @@ -68,7 +67,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx, /* Write adler32 checksum */ put_be32(adler_buf, adler); fwrite_or_die(f, adler_buf, sizeof(adler_buf)); - algo->update_fn(pack_ctx, adler_buf, 4); + git_hash_update(pack_ctx, adler_buf, 4); } /* @@ -92,24 +91,24 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx, sizeof(pack_header), type, len); fwrite_or_die(f, pack_header, pack_header_len); - algo->update_fn(pack_ctx, pack_header, pack_header_len); + git_hash_update(pack_ctx, pack_header, pack_header_len); /* Write the data as uncompressed zlib */ - write_uncompressed_zlib(f, pack_ctx, data, len, algo); + write_uncompressed_zlib(f, pack_ctx, data, len); git_hash_init(&ctx, algo); object_header_len = format_object_header(object_header, sizeof(object_header), type, len); - algo->update_fn(&ctx, object_header, object_header_len); + git_hash_update(&ctx, object_header, object_header_len); if (data) - algo->update_fn(&ctx, data, len); + git_hash_update(&ctx, data, len); else { for (size_t i = len / BLOCK_SIZE; i; i--) - algo->update_fn(&ctx, zeros, BLOCK_SIZE); - algo->update_fn(&ctx, zeros, len % BLOCK_SIZE); + git_hash_update(&ctx, zeros, BLOCK_SIZE); + git_hash_update(&ctx, zeros, len % BLOCK_SIZE); } - algo->final_oid_fn(oid, &ctx); + git_hash_final_oid(oid, &ctx); } /* @@ -434,7 +433,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size, /* Write pack header */ fwrite_or_die(f, &pack_header, sizeof(pack_header)); - algo->update_fn(&pack_ctx, &pack_header, sizeof(pack_header)); + git_hash_update(&pack_ctx, &pack_header, sizeof(pack_header)); /* 1. Write the large blob */ write_pack_object(f, &pack_ctx, OBJ_BLOB, NULL, blob_size, &blob_oid, algo); @@ -472,7 +471,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size, write_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &final_commit_oid, algo); /* Write pack trailer (checksum) */ - algo->final_fn(pack_hash, &pack_ctx); + git_hash_final(pack_hash, &pack_ctx); fwrite_or_die(f, pack_hash, algo->rawsz); if (fclose(f)) die_errno(_("could not close '%s'"), path); diff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci index 04270ee043..d0e2e5f4b1 100644 --- a/tools/coccinelle/hash.cocci +++ b/tools/coccinelle/hash.cocci @@ -7,3 +7,57 @@ struct git_hash_ctx *CTX; - ALGO->init_fn(CTX); + git_hash_init(CTX, ALGO); ...>} + +@@ +identifier f != git_hash_clone; +expression ALGO; +struct git_hash_ctx *SRC; +struct git_hash_ctx *DST; +@@ + f(...) {<... +- ALGO->clone_fn(DST, SRC); ++ git_hash_clone(DST, SRC); + ...>} + +@@ +identifier f != git_hash_update; +expression ALGO; +struct git_hash_ctx *CTX; +expression list ARGS; +@@ + f(...) {<... +- ALGO->update_fn(CTX, ARGS); ++ git_hash_update(CTX, ARGS); + ...>} + +@@ +identifier f != git_hash_final; +expression ALGO; +struct git_hash_ctx *CTX; +expression list ARGS; +@@ + f(...) {<... +- ALGO->final_fn(ARGS, CTX); ++ git_hash_final(ARGS, CTX); + ...>} + +@@ +identifier f != git_hash_final_oid; +expression ALGO; +struct git_hash_ctx *CTX; +expression list ARGS; +@@ + f(...) {<... +- ALGO->final_oid_fn(ARGS, CTX); ++ git_hash_final_oid(ARGS, CTX); + ...>} + +@@ +identifier f != git_hash_discard; +expression ALGO; +struct git_hash_ctx *CTX; +@@ + f(...) {<... +- ALGO->discard_fn(CTX); ++ git_hash_discard(CTX); + ...>} From 90a55e3a51525370798d9b484a74a51ad2b3f047 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:52:55 -0400 Subject: [PATCH 3/7] hash: document function pointers and wrappers We want people to use the git_hash_*() wrappers rather than the bare function pointers in the git_hash_algo struct. Let's document them rather than the bare pointers, and warn people away from the pointers. Coccinelle will eventually force the use of the wrappers, but it's helpful to lead readers in the right direction from the start. While we're here we can document a few other bits of wisdom I've turned up while working in this area: - You have to initialize the destination of a git_hash_clone(). This is something we may eventually change for efficiency, but we should definitely document the requirement for now. - You must eventually finalize or discard a hash, since some backends may allocate resources during initialization. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- hash.h | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/hash.h b/hash.h index 0a23ef4dfd..121ecf13aa 100644 --- a/hash.h +++ b/hash.h @@ -309,22 +309,15 @@ struct git_hash_algo { /* The block size of the hash. */ size_t blksz; - /* The hash initialization function. */ + /* + * Low-level implementation hooks. Callers should use the git_hash_* + * wrappers below rather than invoking these directly. + */ git_hash_init_fn init_fn; - - /* The hash context cloning function. */ git_hash_clone_fn clone_fn; - - /* The hash update function. */ git_hash_update_fn update_fn; - - /* The hash finalization function. */ git_hash_final_fn final_fn; - - /* The hash finalization function for object IDs. */ git_hash_final_oid_fn final_oid_fn; - - /* Discard an initialized hash without finalizing. */ git_hash_discard_fn discard_fn; /* The OID of the empty tree. */ @@ -341,12 +334,40 @@ struct git_hash_algo { }; extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS]; +/* + * Prepare an uninitialized hash context for use. You must eventually release + * the context with git_hash_final() (or final_oid()) or by calling + * git_hash_discard(). + */ void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop); + +/* + * Clone the state of a hash. Both src and dst must have been initialized with + * git_hash_init(). + */ void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src); + +/* + * Add more data to an initialized hash context. + */ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len); + +/* + * Retrieve the final hash value from a context, releasing any resources. + */ void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx); + +/* + * Like git_hash_final(), but write the result into an object_id. + */ void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx); + +/* + * Discard a hash context without computing the final value, but still + * releasing any resources. + */ void git_hash_discard(struct git_hash_ctx *ctx); + const struct git_hash_algo *hash_algo_ptr_by_number(uint32_t algo); struct git_hash_ctx *git_hash_alloc(void); void git_hash_free(struct git_hash_ctx *ctx); From 2c51615d3f57e116c60e825b4a0d587a6f0da12a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:52:57 -0400 Subject: [PATCH 4/7] hash: make git_hash_discard() idempotent You must always either finalize or discard a hash context to release any resources, but you must call only one such function. This creates extra work for some callers, since their cleanup code paths need to know whether they got there via their happy path (and the finalization happened) or due to an error (in which case they need to discard). Let's add an "active" flag that turns a redundant discard into a noop. That lets you safely do this: git_hash_init(&ctx, algo); ... if (some_error) goto out; ... git_hash_final(result, &ctx); out: git_hash_discard(&ctx); This should avoid future errors, and will also let us simplify a few existing callers (in future patches). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- hash.c | 6 ++++++ hash.h | 1 + 2 files changed, 7 insertions(+) diff --git a/hash.c b/hash.c index 55d1d41770..b1296f0018 100644 --- a/hash.c +++ b/hash.c @@ -285,6 +285,7 @@ void git_hash_free(struct git_hash_ctx *ctx) void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop) { algop->init_fn(ctx); + ctx->active = true; } void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src) @@ -300,16 +301,21 @@ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len) void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx) { ctx->algop->final_fn(hash, ctx); + ctx->active = false; } void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx) { ctx->algop->final_oid_fn(oid, ctx); + ctx->active = false; } void git_hash_discard(struct git_hash_ctx *ctx) { + if (!ctx->active) + return; ctx->algop->discard_fn(ctx); + ctx->active = false; } uint32_t hash_algo_by_name(const char *name) diff --git a/hash.h b/hash.h index 121ecf13aa..cf94ad5700 100644 --- a/hash.h +++ b/hash.h @@ -281,6 +281,7 @@ struct git_hash_ctx { git_SHA_CTX_unsafe sha1_unsafe; git_SHA256_CTX sha256; } state; + bool active; }; typedef void (*git_hash_init_fn)(struct git_hash_ctx *ctx); From 6728cfba89aa77b38d08154404eda65c1461bcd3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:53:00 -0400 Subject: [PATCH 5/7] csum-file: use idempotent git_hash_discard() Now that it is safe to call git_hash_discard() even after finalizing it, we can simplify our cleanup logic a bit. This is mostly undoing a few bits of 64337aecde (csum-file: always finalize or discard hash, 2026-07-02): - We no longer need a separate free_hashfile_memory() function for finalize_hashfile(). It can just call free_hashfile(), which will now discard (or not) the hash as appropriate. - When f->skip_hash is set, we don't need to discard; we can rely on free_hashfile() to do it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- csum-file.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/csum-file.c b/csum-file.c index 7e81391524..fe18ee1de3 100644 --- a/csum-file.c +++ b/csum-file.c @@ -55,17 +55,12 @@ void hashflush(struct hashfile *f) } } -static void free_hashfile_memory(struct hashfile *f) -{ - free(f->buffer); - free(f->check_buffer); - free(f); -} - void free_hashfile(struct hashfile *f) { git_hash_discard(&f->ctx); - free_hashfile_memory(f); + free(f->buffer); + free(f->check_buffer); + free(f); } int finalize_hashfile(struct hashfile *f, unsigned char *result, @@ -75,12 +70,10 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, hashflush(f); - if (f->skip_hash) { - git_hash_discard(&f->ctx); + if (f->skip_hash) hashclr(f->buffer, f->algop); - } else { + else git_hash_final(f->buffer, &f->ctx); - } if (result) hashcpy(result, f->buffer, f->algop); @@ -105,7 +98,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, if (close(f->check_fd)) die_errno("%s: sha1 file error on close", f->name); } - free_hashfile_memory(f); + free_hashfile(f); return fd; } From f1bf9772f85c8522e09d16e662858f8de1636bda Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:53:02 -0400 Subject: [PATCH 6/7] http: use idempotent git_hash_discard() Now that it is OK to call git_hash_discard() even after finalizing the hash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http: discard hash in dumb-http http_object_request, 2026-07-02). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 5 +---- http.h | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/http.c b/http.c index 0341de5031..caccf2108e 100644 --- a/http.c +++ b/http.c @@ -2880,7 +2880,6 @@ struct http_object_request *new_http_object_request(const char *base_url, git_inflate_init(&freq->stream); git_hash_init(&freq->c, the_hash_algo); - freq->hash_ctx_valid = 1; freq->url = get_remote_object_url(base_url, hex, 0); @@ -2989,7 +2988,6 @@ int finish_http_object_request(struct http_object_request *freq) } git_hash_final_oid(&freq->real_oid, &freq->c); - freq->hash_ctx_valid = 0; if (freq->zret != Z_STREAM_END) { unlink_or_warn(freq->tmpfile.buf); return -1; @@ -3030,8 +3028,7 @@ void release_http_object_request(struct http_object_request **freq_p) curl_slist_free_all(freq->headers); strbuf_release(&freq->tmpfile); git_inflate_end(&freq->stream); - if (freq->hash_ctx_valid) - git_hash_discard(&freq->c); + git_hash_discard(&freq->c); free(freq); *freq_p = NULL; diff --git a/http.h b/http.h index 6b0639150f..729c51904d 100644 --- a/http.h +++ b/http.h @@ -255,7 +255,6 @@ struct http_object_request { struct object_id oid; struct object_id real_oid; struct git_hash_ctx c; - int hash_ctx_valid; git_zstream stream; int zret; int rename; From 9e396aa553028e708c6fc842d72d6305b761bb5d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:53:05 -0400 Subject: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions It only makes sense to call git_hash_update(), etc, on a hash context that has been initialized but not yet finalized or discarded. This is an unlikely error to make, but it's easy for us to catch it and complain. It's especially important because it would quietly "work" for many hash backends (like sha1dc, which is just manipulating some bytes) but would cause undefined behavior with others (like OpenSSL, which puts the context onto the heap). Checking the flag lets us catch problems consistently on every build. Note that we can't do the same for git_hash_init(). Even though it would cause a leak to call it twice (without an intervening final/discard), the point of the function is that the contents of the struct are undefined before the call. But calling it twice is an even less likely error to make, so not covering it is OK. We leave git_hash_discard() alone, as its idempotent behavior is convenient for callers. We _could_ try to do something similar for git_hash_final(), allowing: git_hash_final(result, &ctx); git_hash_final(other_result, &ctx); but it does not make much sense. After the first final() call we have thrown away the state, so we cannot produce the same output. We could come up with some sensible output (the null hash, or the empty hash), but double-calls like this are more likely a bug, so our best bet is to complain loudly (whereas the current code produces either nonsense output or undefined behavior, depending on the backend). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- hash.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hash.c b/hash.c index b1296f0018..82f7e24404 100644 --- a/hash.c +++ b/hash.c @@ -290,22 +290,32 @@ void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop) void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src) { + if (!src->active) + BUG("attempt to copy from an inactive hash context"); + if (!dst->active) + BUG("attempt to copy to an inactive hash context"); src->algop->clone_fn(dst, src); } void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len) { + if (!ctx->active) + BUG("attempt to update an inactive hash context"); ctx->algop->update_fn(ctx, in, len); } void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx) { + if (!ctx->active) + BUG("attempt to finalize an inactive hash context"); ctx->algop->final_fn(hash, ctx); ctx->active = false; } void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx) { + if (!ctx->active) + BUG("attempt to finalize an inactive hash context"); ctx->algop->final_oid_fn(oid, ctx); ctx->active = false; }