Re: [PATCH 7/7] hash: check ctx->active flag in all wrapper functions
From: Junio C Hamano <hidden>
Date: 2026-07-07 16:33:08
Jeff King [off-list ref] writes:
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_init_hash(). 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. Signed-off-by: Jeff King <redacted> --- hash.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Among the four we see here, I agree that calling _clone and _update on an already discarded or finalized context should be caught as an error. As I alluded to earlier, though, I am not sure about _final. The asymmetry in a design that allows _discard after _final but not _final after _final disturbs me slightly, but perhaps that is only because my morning caffeine has not yet kicked in.
quoted hunk ↗ jump to hunk
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; }