From 926a750a702bc47416facd026690cbb5e25cad09 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 03:57:44 -0400 Subject: [PATCH 01/43] csum-file: drop discard_hashfile() Commit c3d034df16 (csum-file: introduce discard_hashfile(), 2024-07-25) added a cleanup function that no longer has any callers. In that commit we adjusted do_write_index() to use the new function. But a similar fix occurred on a parallel branch, making free_hashfile() public, and the merge resolution in 1b6b2bfae5 (Merge branch 'ps/leakfixes-part-4', 2024-08-23) took the free_hashfile() version. So now we have two functions, discard_hashfile() and free_hashfile(), and we only need one. Which one do we want to keep? The only difference between them is that the discard variant also closes the descriptors held in the struct. Let's look at the three callers: 1. In finalize_hashfile() we've either already closed the descriptors (if the CSUM_CLOSE flag is passed) or the caller didn't want them closed (if it didn't pass that flag). So we want the more limited free_hashfile(). 2. In object-file.c:flush_packfile_transaction() we close the descriptor ourselves. So discard_hashfile() could save us a line of code. 3. In do_write_index() we don't close the descriptor. This was the spot for which c3d034df16 added the discard function in the first place, but I'm skeptical that closing the descriptor here is the right thing. It is true that we are done with the descriptor at this point and closing it would be ideal. But we don't really own it! The descriptor comes from a tempfile struct (as part of a lock) and that tempfile will hold on to the descriptor and try to close it when it is deleted. This might happen at the end of the program, in which case the double-close is mostly harmless (we might accidentally close some other open descriptor, but at that point we're just closing and unlinking everything we can). But in theory it could also cause subtle bugs. If do_write_index() fails, we return the error up the stack and would eventually end up in write_locked_index(). There we roll back the lock file on error, which will close the descriptor. So now we get our double close, and we might actually close something else that was opened in the interim. This is probably unlikely in practice (as soon as we see the error we'd mostly be unwinding the stack, not opening new files). But it highlights a potential problem with the discard_hashfile() interface: the hashfile doesn't necessarily own that descriptor. Note that I said "descriptors" plural above. Those callers all care about the "fd" member of the struct. But discard_hashfile() also closes check_fd. That is only used if the struct is initialized with hashfd_check(), and neither of its two callers call either discard or free (they always "finalize" instead). So closing it is irrelevant for the current callers. I think we're better off sticking with the simpler free_hashfile() interface, and the handful of callers can decide how to handle the descriptors themselves. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- csum-file.c | 9 --------- csum-file.h | 1 - 2 files changed, 10 deletions(-) diff --git a/csum-file.c b/csum-file.c index 9558177a11b49a..2bbedfa36e41fb 100644 --- a/csum-file.c +++ b/csum-file.c @@ -101,15 +101,6 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, return fd; } -void discard_hashfile(struct hashfile *f) -{ - if (0 <= f->check_fd) - close(f->check_fd); - if (0 <= f->fd) - close(f->fd); - free_hashfile(f); -} - void hashwrite(struct hashfile *f, const void *buf, uint32_t count) { while (count) { diff --git a/csum-file.h b/csum-file.h index a9b390d3366875..1d762a64b0d67a 100644 --- a/csum-file.h +++ b/csum-file.h @@ -74,7 +74,6 @@ void free_hashfile(struct hashfile *f); * Finalize the hashfile by flushing data to disk and free'ing it. */ int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int); -void discard_hashfile(struct hashfile *); void hashwrite(struct hashfile *, const void *, uint32_t); void hashflush(struct hashfile *f); void crc32_begin(struct hashfile *); From cdb20e97d8beb5492db4bf308fc04403b0a75d1d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 03:59:53 -0400 Subject: [PATCH 02/43] hash: add discard primitive The usual life-cycle for a git_hash_ctx is calling git_hash_init(), adding some data, and then using git_hash_final() to get the output digest and free any resources. Sometimes we decide to abort the operation without the final() call (e.g., due to errors or other reasons). In that case we just abandon the hash_ctx completely and let it go out of scope. For most hash implementations this is fine; they were just holding values directly in the struct. But some implementations do allocate memory, and in these cases we leak the memory. Notably OpenSSL >= 3.0 requires us to allocate the digest context on the heap with EVP_MD_CTX_new(). Let's provide a git_hash_discard() function that can be used in these code paths to free any resources. For now we'll implement it by just calling git_hash_final() into a dummy output, relying on its side effect of freeing the resources. Our view of the underlying hash implementation is abstracted behind the platform_SHA_* macros, so that's the best we can do without widening that interface. It's a little inefficient, but probably not noticeably so in practice, especially as we'd usually hit this on an error code path. And by abstracting it in this function, we can later swap it out when the platform_SHA interface lets us do so. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- hash.c | 12 ++++++++++++ hash.h | 1 + 2 files changed, 13 insertions(+) diff --git a/hash.c b/hash.c index e925b9754e04fc..63672a3d2222d5 100644 --- a/hash.c +++ b/hash.c @@ -283,6 +283,18 @@ void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx) ctx->algop->final_oid_fn(oid, ctx); } +void git_hash_discard(struct git_hash_ctx *ctx) +{ + /* + * XXX Many implementations do not need to do anything here, + * and a dummy final() call is wasteful. But we can't fix + * that unless our implementation API exposes a discard + * primitive. + */ + unsigned char dummy[GIT_MAX_RAWSZ]; + git_hash_final(dummy, ctx); +} + uint32_t hash_algo_by_name(const char *name) { if (!name) diff --git a/hash.h b/hash.h index c082a53c9ac93d..6b2f04e2a4a116 100644 --- a/hash.h +++ b/hash.h @@ -325,6 +325,7 @@ void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src); 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); void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx); +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 64337aecded9e91a766b585b86bcf5f0342e0b87 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:01:30 -0400 Subject: [PATCH 03/43] csum-file: always finalize or discard hash When a hashfile struct is created, we always initialize the git_hash_ctx inside it. We usually end up in hashfile_finalize(), which passes that ctx to git_hash_final(), cleaning it up. But a few code paths don't do so: 1. If we bail on the hashfile and call free_hashfile() directly rather than finalizing. 2. If the skip_hash flag is set, the hashfile_finalize() call will never call git_hash_final(). (You might think that we should just avoid git_hash_init() entirely in this case, but the skip_hash flag is set by the caller after the hashfile is initialized). For most hash implementations this is OK, but for ones that allocate on initialization it causes a memory leak. You can see many failures by running: make SANITIZE=leak OPENSSL_SHA1_UNSAFE=1 test since OpenSSL >= 3.0 is such an allocating hash implementation (and csum-file uses the "unsafe" algorithm variant). We can solve this by calling git_hash_discard() as appropriate. Note that free_hashfile() is used both directly by callers to abort without finalizing, and by hashfile_finalize() to free memory. In the latter case we _don't_ want to call git_hash_discard(), because we'll already have either finalized or discarded it. So we'll push that to an internal "free_memory" function, and keep free_hashfile() as the public interface to abort a hashfile without finalizing. This fix makes several scripts leak-free with the command above: t1600, t1601, t2107, t7008, t9210, t9211. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- csum-file.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/csum-file.c b/csum-file.c index 2bbedfa36e41fb..69e3b9925490dc 100644 --- a/csum-file.c +++ b/csum-file.c @@ -55,13 +55,19 @@ void hashflush(struct hashfile *f) } } -void free_hashfile(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); +} + int finalize_hashfile(struct hashfile *f, unsigned char *result, enum fsync_component component, unsigned int flags) { @@ -69,10 +75,12 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, hashflush(f); - if (f->skip_hash) + if (f->skip_hash) { + git_hash_discard(&f->ctx); hashclr(f->buffer, f->algop); - else + } else { git_hash_final(f->buffer, &f->ctx); + } if (result) hashcpy(result, f->buffer, f->algop); @@ -97,7 +105,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(f); + free_hashfile_memory(f); return fd; } From 46ba44e1fd6b1a3d71d529f6713821efc26072c9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:03:19 -0400 Subject: [PATCH 04/43] csum-file: provide a function to release checkpoints A hashfile_checkpoint struct is basically just a copy of the hash_ctx state at a given point in the file. As such, it contains its own git_hash_ctx which may (depending on the underlying hash implementation) need to be discarded when we're done with it. Let's add a "release" function which cleans up the hash context it holds. I chose "release" here and not "discard" because you'd use this to clean up every checkpoint, whether you used it or not. As opposed to git_hash_discard(), which is needed only if you didn't call git_hash_final(). There are only two callers which use hashfile_checkpoints, and we can add release calls to both. When built with "SANITIZE=leak OPENSSL_SHA1_UNSAFE=1", this makes both t1050 and t9300 leak-free. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 1 + csum-file.c | 5 +++++ csum-file.h | 1 + object-file.c | 2 ++ 4 files changed, 9 insertions(+) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 82bc6dcc003723..a596b0ee2ad4d4 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -1214,6 +1214,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark) out: free(in_buf); free(out_buf); + hashfile_checkpoint_release(&checkpoint); } /* All calls must be guarded by find_object() or find_mark() to diff --git a/csum-file.c b/csum-file.c index 69e3b9925490dc..19805e580acb72 100644 --- a/csum-file.c +++ b/csum-file.c @@ -223,6 +223,11 @@ int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint return 0; } +void hashfile_checkpoint_release(struct hashfile_checkpoint *checkpoint) +{ + git_hash_discard(&checkpoint->ctx); +} + void crc32_begin(struct hashfile *f) { f->crc32 = crc32(0, NULL, 0); diff --git a/csum-file.h b/csum-file.h index 1d762a64b0d67a..b98bf570606c82 100644 --- a/csum-file.h +++ b/csum-file.h @@ -39,6 +39,7 @@ struct hashfile_checkpoint { void hashfile_checkpoint_init(struct hashfile *, struct hashfile_checkpoint *); void hashfile_checkpoint(struct hashfile *, struct hashfile_checkpoint *); int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *); +void hashfile_checkpoint_release(struct hashfile_checkpoint *); /* finalize_hashfile flags */ #define CSUM_CLOSE 1 diff --git a/object-file.c b/object-file.c index 2acc9522df2daa..aeb0d893687ffc 100644 --- a/object-file.c +++ b/object-file.c @@ -1639,6 +1639,8 @@ static int index_blob_packfile_transaction(struct odb_transaction_files *transac state->alloc_written); state->written[state->nr_written++] = idx; } + + hashfile_checkpoint_release(&checkpoint); return 0; } From 64b69225620a4119e1ee6e02620c56a58dc442fb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:04:11 -0400 Subject: [PATCH 05/43] patch-id: discard hash when done When computing a patch-id, we have a flush_one_hunk() helper that calls git_hash_final() on our running hunk git_hash_ctx, and then reinitializes that context for the next hunk. When we run out of hunks to look at, we return, discarding the git_hash_ctx. This can cause a leak if the hash implementation we are using allocates any memory during its initialization. This includes OpenSSL >= 3.0, for both SHA-1 and SHA-256. Normally we would not use SHA-1 here at all, as we only recommend using non-DC implementations for the "unsafe" variant (and patch-id, though they probably _could_ use the unsafe variant, were never taught to do so). But it is certainly a problem for SHA-256, which you can see with: make SANITIZE=leak \ OPENSSL_SHA256=1 \ GIT_TEST_DEFAULT_HASH=sha256 \ test That results in leak failures of 60 scripts, 57 of which are fixed by this patch (basically anything which runs rebase will hit this case). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/patch-id.c | 1 + diff.c | 1 + 2 files changed, 2 insertions(+) diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 2781598ede6ea0..57d9bd4a656d1a 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -173,6 +173,7 @@ static size_t get_one_patchid(struct object_id *next_oid, struct object_id *resu oidclr(next_oid, the_repository->hash_algo); flush_one_hunk(result, &ctx); + git_hash_discard(&ctx); return patchlen; } diff --git a/diff.c b/diff.c index 397e38b41cc6fa..f631bf1e2aa47d 100644 --- a/diff.c +++ b/diff.c @@ -6958,6 +6958,7 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid flush_one_hunk(oid, &ctx); } + git_hash_discard(&ctx); return 0; } From 77f78b802559f19167a1d004d5566da9fbff9e85 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:05:03 -0400 Subject: [PATCH 06/43] check_stream_oid(): discard hash on read error The happy path of check_stream_oid() is to initialize a hash, feed the loose object zlib stream into it, and then get the final result. But if we hit a zlib error or see extra cruft we'll bail early with an error. Since we never call git_hash_final() in this cases, any resources held by the git_hash_ctx may be leaked. Our default hash algorithms don't allocate anything in the hash_ctx, but some implementations do. For example, running: make SANITIZE=leak \ OPENSSL_SHA256=1 \ GIT_TEST_DEFAULT_HASH=sha256 \ test will fail t1450, since it feeds corrupted objects that cause us to bail from check_stream_oid(). This patch fixes it by discarding the hash in those early return paths. Trying to jump to a common "out:" label is not worth it here, as we must _not_ discard a hash that was already fed to git_hash_final(). And the hash_ctx itself does not carry any information (so we cannot check for a NULL pointer, etc). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- object-file.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/object-file.c b/object-file.c index aeb0d893687ffc..d593e160155c12 100644 --- a/object-file.c +++ b/object-file.c @@ -2122,11 +2122,13 @@ static int check_stream_oid(git_zstream *stream, if (status != Z_STREAM_END) { error(_("corrupt loose object '%s'"), oid_to_hex(expected_oid)); + git_hash_discard(&c); return -1; } if (stream->avail_in) { error(_("garbage at end of loose object '%s'"), oid_to_hex(expected_oid)); + git_hash_discard(&c); return -1; } From a2d8ea5a766c7f16afd4294acb7a618b67de75f2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:07:07 -0400 Subject: [PATCH 07/43] http: discard hash in dumb-http http_object_request Usually an object request results in finish_http_object_request() calling git_hash_final_oid(), after we've received all of the data. But if we hit an error, we'll bail early and free the http_object_request, dropping the git_hash_ctx entirely. This can cause a leak for hash implementations that allocate memory in their context, like OpenSSL >= 3.0. The obvious fix is for abort_http_object_request() to call git_hash_discard(), under the assumption that every request is either finished or aborted. But that's not quite true: 1. Not everybody calls the abort function. Sometimes they jump straight to release_http_object_request(). So we'd have to put it there. 2. After the finish function finalizes the hash, we can still encounter errors! In that case we end up aborting or releasing, and they must not discard that hash (since that would be a double-free). So we'll keep a flag marking the validity of the hash_ctx field of the request. The lifetime is simple: it is valid immediately after creation, up until we call finalize. And then our release function can just conditionally discard the hash based on that flag. This fixes test failures in t5550 and t5619 when run with: make SANITIZE=leak \ OPENSSL_SHA256=1 \ GIT_TEST_DEFAULT_HASH=sha256 \ test The flag handling could be removed if the hash-discard function were idempotent. This could be done easily-ish by having the underlying hash functions (like the ones in sha256/openssl.h) set the context pointer to NULL after free-ing. But it's something that every platform implementation would have to remember to do, and the benefit for the callers is not that huge (it would let us shave a few lines here and probably in a few other spots). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 4 ++++ http.h | 1 + 2 files changed, 5 insertions(+) diff --git a/http.c b/http.c index 67c9c6fc60673d..8b0de519840661 100644 --- a/http.c +++ b/http.c @@ -2806,6 +2806,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); + freq->hash_ctx_valid = 1; freq->url = get_remote_object_url(base_url, hex, 0); @@ -2913,6 +2914,7 @@ 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; @@ -2953,6 +2955,8 @@ 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); free(freq); *freq_p = NULL; diff --git a/http.h b/http.h index f9ee888c3ed67e..dc9700c1131117 100644 --- a/http.h +++ b/http.h @@ -249,6 +249,7 @@ 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 a51b54530e1f38dccf77afdc49e0ea16fdc69b16 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:09:07 -0400 Subject: [PATCH 08/43] hash: fix memory leak copying sha256 gcrypt handles Our abstracted hash-algorithm API allows for cloning a hash context. By default this just memcpy()s the bytes, but specific implementations can provide a custom clone function. Our API is based around the way that OpenSSL works, which is that you first initialize the destination context, then copy into it. In our code that is this: algo->init_fn(&dst); git_hash_clone(&dst, src); and that translates into OpenSSL calls like: /* init_fn */ dst->ectx = EVP_MD_CTX_new(); EVP_DigestInit_ex(dst->ectx, EVP_sha256()); /* clone */ EVP_MD_CTX_copy_ex(dst->ectx, src->ectx); So the allocation happens in the first step, and then the clone is just copying values (the DigestInit is initializing values that just get overwritten, but that's not wrong, just a little inefficient). But libgcrypt doesn't work like that! Its copy function initializes dst from scratch. So when using the sha256 gcrypt backend, that becomes: /* init_fn; this allocates */ gcry_md_open(&dst, GCRY_MD_SHA256); /* clone; this also allocates, leaking the previous value! */ gcry_md_copy(&dst, src); You can see the leaks in the test suite by running: make \ SANITIZE=leak \ GCRYPT_SHA256=1 \ GIT_TEST_DEFAULT_SHA=256 \ test which has many failures, as opposed to building with OPENSSL_SHA256, which is leak-free. The easy fix here is for the clone function to close the open context we're about to overwrite. It's a little inefficient (we did a pointless open in the init function), but probably not a big deal in practice. If our API went the other way, assuming that we're always cloning into garbage bytes, then we could be more efficient. We'd teach OpenSSL's clone function to do its own new(), skip the DigestInit, and then copy into it. And gcrypt could stick with just the copy() call. But look again at the asymmetry in the very first code example. We call the init function straight from the git_hash_algo struct, and then subsequent calls are dispatched through our git_hash_* wrappers. If you wanted to clone into an uninitialized destination, you'd do something like: algo->clone_fn(&dst, src); instead. That would require changing all of the callers. There's not that many of them, but I don't know that it's worth changing our calling conventions to try to reclaim this tiny bit of efficiency. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha256/gcrypt.h | 1 + 1 file changed, 1 insertion(+) diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h index 17a90f1052526c..694a2b70a15bb1 100644 --- a/sha256/gcrypt.h +++ b/sha256/gcrypt.h @@ -27,6 +27,7 @@ static inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src) { + gcry_md_close(*dst); gcry_md_copy(dst, *src); } From 600588d2aa4e3311ee476f89cd57a622ed8bf0ec Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Jul 2026 04:13:49 -0400 Subject: [PATCH 09/43] hash: add platform-specific discard functions Our git_hash_discard() is a bit hacky: it just calls git_hash_final() into a dummy result buffer, using the side effect that each implementation's Final() function will also free any resources. This is probably not too terrible, since generating the final hash is not that expensive and we'd mostly call discard on unusual or error code paths. But we can do better by widening the platform API a bit to add an explicit discard function. This requires an annoying amount of boilerplate: - Each algorithm needs a git_$ALGO_discard() wrapper that dereferences the union'd git_hash_ctx into the type-safe field. So sha1 + sha256 + sha1-unsafe, plus a BUG() for the unknown algo. And then these all need to be referenced in the git_hash_algo structs. - Platforms which don't do anything special to discard now need a fallback function which does nothing. And we need this for each algo (sha1, sha256, and sha1-unsafe). - Platforms which do need to discard must define their discard functions. This includes sha1/openssl, sha256/openssl, and sha256/gcrypt (no sha1-unsafe here as it sits atop the sha1/openssl functions). - Algo selection needs to point platform_*_Discard to the appropriate underlying macro, or indicate that the fallback should be used. We have a similar situation for the Clone function (where a straight memcpy() of the context struct is not enough for some platforms). I've tied Discard to the same flag used by Clone here, since they are basically the same problem: is the hash context a sequence of bytes, or does it need smart copying/discarding? It's easy to miss a case here since we don't even compile the implementations we aren't using. I've tested with each of: - no flags, which uses our internal sha1/sha256 implementations, both of which exercise the noop fallback function - OPENSSL_SHA1_UNSAFE=1, which checks that our unsafe macro redirections work - OPENSSL_SHA1=1, though you should not do that in real life! - OPENSSL_SHA256=1, passes tests with GIT_TEST_DEFAULT_HASH=sha256 - GCRYPT_SHA256=1, which likewise passes The other implementations do not set the CLONE_HELPER flag, so they treat the context as bytes and should be fine with the fallback. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- hash.c | 33 +++++++++++++++++++++++++-------- hash.h | 21 +++++++++++++++++++++ sha1/openssl.h | 6 ++++++ sha256/gcrypt.h | 6 ++++++ sha256/openssl.h | 6 ++++++ 5 files changed, 64 insertions(+), 8 deletions(-) diff --git a/hash.c b/hash.c index 63672a3d2222d5..55d1d41770366a 100644 --- a/hash.c +++ b/hash.c @@ -72,6 +72,11 @@ static void git_hash_sha1_final_oid(struct object_id *oid, struct git_hash_ctx * oid->algo = GIT_HASH_SHA1; } +static void git_hash_sha1_discard(struct git_hash_ctx *ctx) +{ + git_SHA1_Discard(&ctx->state.sha1); +} + static void git_hash_sha1_init_unsafe(struct git_hash_ctx *ctx) { ctx->algop = unsafe_hash_algo(&hash_algos[GIT_HASH_SHA1]); @@ -102,6 +107,11 @@ static void git_hash_sha1_final_oid_unsafe(struct object_id *oid, struct git_has oid->algo = GIT_HASH_SHA1; } +static void git_hash_sha1_discard_unsafe(struct git_hash_ctx *ctx) +{ + git_SHA1_Discard_unsafe(&ctx->state.sha1_unsafe); +} + static void git_hash_sha256_init(struct git_hash_ctx *ctx) { ctx->algop = unsafe_hash_algo(&hash_algos[GIT_HASH_SHA256]); @@ -135,6 +145,11 @@ static void git_hash_sha256_final_oid(struct object_id *oid, struct git_hash_ctx oid->algo = GIT_HASH_SHA256; } +static void git_hash_sha256_discard(struct git_hash_ctx *ctx) +{ + git_SHA256_Discard(&ctx->state.sha256); +} + static void git_hash_unknown_init(struct git_hash_ctx *ctx UNUSED) { BUG("trying to init unknown hash"); @@ -165,6 +180,11 @@ static void git_hash_unknown_final_oid(struct object_id *oid UNUSED, BUG("trying to finalize unknown hash"); } +static void git_hash_unknown_discard(struct git_hash_ctx *ctx UNUSED) +{ + BUG("trying to discard unknown hash"); +} + static const struct git_hash_algo sha1_unsafe_algo = { .name = "sha1", .format_id = GIT_SHA1_FORMAT_ID, @@ -176,6 +196,7 @@ static const struct git_hash_algo sha1_unsafe_algo = { .update_fn = git_hash_sha1_update_unsafe, .final_fn = git_hash_sha1_final_unsafe, .final_oid_fn = git_hash_sha1_final_oid_unsafe, + .discard_fn = git_hash_sha1_discard_unsafe, .empty_tree = &empty_tree_oid, .empty_blob = &empty_blob_oid, .null_oid = &null_oid_sha1, @@ -193,6 +214,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = { .update_fn = git_hash_unknown_update, .final_fn = git_hash_unknown_final, .final_oid_fn = git_hash_unknown_final_oid, + .discard_fn = git_hash_unknown_discard, .empty_tree = NULL, .empty_blob = NULL, .null_oid = NULL, @@ -208,6 +230,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = { .update_fn = git_hash_sha1_update, .final_fn = git_hash_sha1_final, .final_oid_fn = git_hash_sha1_final_oid, + .discard_fn = git_hash_sha1_discard, .unsafe = &sha1_unsafe_algo, .empty_tree = &empty_tree_oid, .empty_blob = &empty_blob_oid, @@ -224,6 +247,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = { .update_fn = git_hash_sha256_update, .final_fn = git_hash_sha256_final, .final_oid_fn = git_hash_sha256_final_oid, + .discard_fn = git_hash_sha256_discard, .empty_tree = &empty_tree_oid_sha256, .empty_blob = &empty_blob_oid_sha256, .null_oid = &null_oid_sha256, @@ -285,14 +309,7 @@ void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx) void git_hash_discard(struct git_hash_ctx *ctx) { - /* - * XXX Many implementations do not need to do anything here, - * and a dummy final() call is wasteful. But we can't fix - * that unless our implementation API exposes a discard - * primitive. - */ - unsigned char dummy[GIT_MAX_RAWSZ]; - git_hash_final(dummy, ctx); + ctx->algop->discard_fn(ctx); } uint32_t hash_algo_by_name(const char *name) diff --git a/hash.h b/hash.h index 6b2f04e2a4a116..0a23ef4dfd4e67 100644 --- a/hash.h +++ b/hash.h @@ -37,6 +37,7 @@ # define platform_SHA1_Clone_unsafe openssl_SHA1_Clone # define platform_SHA1_Update_unsafe openssl_SHA1_Update # define platform_SHA1_Final_unsafe openssl_SHA1_Final +# define platform_SHA1_Discard_unsafe openssl_SHA1_Discard # else # define platform_SHA_CTX_unsafe SHA_CTX # define platform_SHA1_Init_unsafe SHA1_Init @@ -92,6 +93,7 @@ # define platform_SHA1_Final_unsafe platform_SHA1_Final # ifdef platform_SHA1_Clone # define platform_SHA1_Clone_unsafe platform_SHA1_Clone +# define platform_SHA1_Discard_unsafe platform_SHA1_Discard # endif # ifdef SHA1_NEEDS_CLONE_HELPER # define SHA1_NEEDS_CLONE_HELPER_UNSAFE @@ -110,9 +112,11 @@ #ifdef platform_SHA1_Clone #define git_SHA1_Clone platform_SHA1_Clone +#define git_SHA1_Discard platform_SHA1_Discard #endif #ifdef platform_SHA1_Clone_unsafe # define git_SHA1_Clone_unsafe platform_SHA1_Clone_unsafe +# define git_SHA1_Discard_unsafe platform_SHA1_Discard_unsafe #endif #ifndef platform_SHA256_CTX @@ -129,6 +133,7 @@ #ifdef platform_SHA256_Clone #define git_SHA256_Clone platform_SHA256_Clone +#define git_SHA256_Discard platform_SHA256_Discard #endif #ifdef SHA1_MAX_BLOCK_SIZE @@ -142,6 +147,10 @@ static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src) { memcpy(dst, src, sizeof(*dst)); } +static inline void git_SHA1_Discard(git_SHA_CTX *ctx UNUSED) +{ + /* noop */ +} #endif #ifndef SHA1_NEEDS_CLONE_HELPER_UNSAFE static inline void git_SHA1_Clone_unsafe(git_SHA_CTX_unsafe *dst, @@ -149,6 +158,10 @@ static inline void git_SHA1_Clone_unsafe(git_SHA_CTX_unsafe *dst, { memcpy(dst, src, sizeof(*dst)); } +static inline void git_SHA1_Discard_unsafe(git_SHA_CTX_unsafe *ctx UNUSED) +{ + /* noop */ +} #endif #ifndef SHA256_NEEDS_CLONE_HELPER @@ -156,6 +169,10 @@ static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *s { memcpy(dst, src, sizeof(*dst)); } +static inline void git_SHA256_Discard(git_SHA256_CTX *ctx UNUSED) +{ + /* noop */ +} #endif /* @@ -271,6 +288,7 @@ typedef void (*git_hash_clone_fn)(struct git_hash_ctx *dst, const struct git_has typedef void (*git_hash_update_fn)(struct git_hash_ctx *ctx, const void *in, size_t len); typedef void (*git_hash_final_fn)(unsigned char *hash, struct git_hash_ctx *ctx); typedef void (*git_hash_final_oid_fn)(struct object_id *oid, struct git_hash_ctx *ctx); +typedef void (*git_hash_discard_fn)(struct git_hash_ctx *ctx); struct git_hash_algo { /* @@ -306,6 +324,9 @@ struct git_hash_algo { /* 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. */ const struct object_id *empty_tree; diff --git a/sha1/openssl.h b/sha1/openssl.h index 1038af47dafa79..48deeb724ad511 100644 --- a/sha1/openssl.h +++ b/sha1/openssl.h @@ -40,12 +40,18 @@ static inline void openssl_SHA1_Clone(struct openssl_SHA1_CTX *dst, EVP_MD_CTX_copy_ex(dst->ectx, src->ectx); } +static inline void openssl_SHA1_Discard(struct openssl_SHA1_CTX *ctx) +{ + EVP_MD_CTX_free(ctx->ectx); +} + #ifndef platform_SHA_CTX #define platform_SHA_CTX openssl_SHA1_CTX #define platform_SHA1_Init openssl_SHA1_Init #define platform_SHA1_Clone openssl_SHA1_Clone #define platform_SHA1_Update openssl_SHA1_Update #define platform_SHA1_Final openssl_SHA1_Final +#define platform_SHA1_Discard openssl_SHA1_Discard #endif #endif /* SHA1_OPENSSL_H */ diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h index 694a2b70a15bb1..d91ffe73d31aec 100644 --- a/sha256/gcrypt.h +++ b/sha256/gcrypt.h @@ -31,10 +31,16 @@ static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA2 gcry_md_copy(dst, *src); } +static inline void gcrypt_SHA256_Discard(gcrypt_SHA256_CTX *ctx) +{ + gcry_md_close(*ctx); +} + #define platform_SHA256_CTX gcrypt_SHA256_CTX #define platform_SHA256_Init gcrypt_SHA256_Init #define platform_SHA256_Clone gcrypt_SHA256_Clone #define platform_SHA256_Update gcrypt_SHA256_Update #define platform_SHA256_Final gcrypt_SHA256_Final +#define platform_SHA256_Discard gcrypt_SHA256_Discard #endif diff --git a/sha256/openssl.h b/sha256/openssl.h index c1083d94914621..3d457ca99de9ef 100644 --- a/sha256/openssl.h +++ b/sha256/openssl.h @@ -40,10 +40,16 @@ static inline void openssl_SHA256_Clone(struct openssl_SHA256_CTX *dst, EVP_MD_CTX_copy_ex(dst->ectx, src->ectx); } +static inline void openssl_SHA256_Discard(struct openssl_SHA256_CTX *ctx) +{ + EVP_MD_CTX_free(ctx->ectx); +} + #define platform_SHA256_CTX openssl_SHA256_CTX #define platform_SHA256_Init openssl_SHA256_Init #define platform_SHA256_Clone openssl_SHA256_Clone #define platform_SHA256_Update openssl_SHA256_Update #define platform_SHA256_Final openssl_SHA256_Final +#define platform_SHA256_Discard openssl_SHA256_Discard #endif /* SHA256_OPENSSL_H */ From bad766fbac590e72b5cf9e3f83e4fce820d8b9c6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 4 Jul 2026 08:52:04 +0000 Subject: [PATCH 10/43] ci(dockerized): raise the PID limit for private repositories Every once in a while I need to verify that Microsoft Git's test suite passes for changes that are not yet meant for public consumption, and since it was (made) too difficult to keep up a working Azure Pipeline definition, I have to use GitHub Actions in a private GitHub repository for that purpose. In these tests, basically all Dockerized CI jobs fail consistently. The symptom is something like: error: cannot create async thread: Resource temporarily unavailable in the middle of a test, typically in the t5xxx-t6xxx range. The first such error is immediately followed by plenty more of these errors, and not a single test succeeds afterwards. At first, I thought that maybe the massive parallelism I enjoy there is the problem, and I thought that the cgroups limits might be shared between the many containers that run on essentially the same physical machine. But even reducing the matrix to just a single of those Dockerized jobs runs into the very same problems. The underlying reason seems to be a substantial difference in the hosted runners that execute these Dockerized jobs: forcing the PID limit of the container to a high number lets the jobs pass, even when running the complete matrix of all 13 Dockerized jobs concurrently. But that's not the only difference: The jobs seem to take a lot longer in these containers than, say, in the containers made available to https://github.com/git/git. When forcing a PID limit of 64k in that private repository, the jobs completed successfully, but they also took a lot longer, between 2x to 2.5x longer, i.e. painfully much longer. Reducing the PID limit to 16k, the CI jobs still passed, but took an equally long amount of time. Reducing the PID limit to 8k caused the errors to reappear. Here are the numbers from three example runs, the first one forcing the PID and nproc limit to 65536, the second one to 16384, the third run is from the public git/git repository: Job | 64k | 16k | reference ------------------------------|---------|---------|--------- almalinux-8 | 19m 3s | 16m 0s | 9m 36s debian-11 | 20m 31s | 20m 3s | 8m 5s fedora-breaking-changes-meson | 16m 29s | 19m 19s | 9m 40s linux-asan-ubsan | 1h 10m | 1h 11m | 34m 36s linux-breaking-changes | 25m 39s | 25m 58s | 13m 15s linux-leaks | 1h 9m | 1h 10m | 33m 30s linux-meson | 28m 9s | 27m 4s | 13m 45s linux-musl-meson | 16m 32s | 13m 39s | 8m 6s linux-reftable-leaks | 1h 13m | 1h 13m | 34m 34s linux-reftable | 26m 2s | 25m 48s | 13m 31s linux-sha256 | 26m 12s | 26m 3s | 12m 36s linux-TEST-vars | 26m 5s | 25m 21s | 13m 25s linux32 | 21m 16s | 19m 57s | 10m 44s It does not look as if the PID limit is the reason for the longer runtime, seeing as the 64k vs 16k timings deviate no more than as is usual with GitHub workflows. So let's go for 16k. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- .github/workflows/main.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6f3d94e3a60cdd..96d19581129ec2 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -420,7 +420,9 @@ jobs: CI_JOB_IMAGE: ${{matrix.vector.image}} CUSTOM_PATH: /custom runs-on: ubuntu-latest - container: ${{matrix.vector.image}} + container: + image: ${{ matrix.vector.image }} + options: ${{ github.repository_visibility == 'private' && '--pids-limit 16384 --ulimit nproc=16384:16384 --ulimit nofile=32768:32768' || '' }} steps: - name: prepare libc6 for actions if: matrix.vector.jobname == 'linux32' From 1eb281159f0f75044e9e45a47d1d34162f3b0032 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Sat, 4 Jul 2026 19:37:24 -0400 Subject: [PATCH 11/43] precompose_utf8: use a flex array for d_name On macOS, git status may abort while reading a directory entry whose UTF-8 name grows past NAME_MAX bytes: __chk_fail_overflow __strlcpy_chk precompose_utf8_readdir read_directory_recursive wt_status_collect cmd_status The precompose wrapper already reallocates dirent_prec_psx for long names, but d_name is declared as char[NAME_MAX + 1]. A fortified libc can still see that declared object size and reject a larger strlcpy bound, even though the allocation was grown. Make d_name a FLEX_ARRAY and size allocations from offsetof(). That matches the actual object layout with the dynamic allocation, so the fortified copy sees a destination whose size can grow with max_name_len. Add a regression test that creates an over-NAME_MAX non-ASCII basename and runs status with core.precomposeunicode enabled. Signed-off-by: Ihar Hrachyshka Signed-off-by: Junio C Hamano --- compat/precompose_utf8.c | 12 ++++++++---- compat/precompose_utf8.h | 9 +++++---- t/t3910-mac-os-precompose.sh | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c index 43b3be011439ef..69740c2ad7f2a6 100644 --- a/compat/precompose_utf8.c +++ b/compat/precompose_utf8.c @@ -19,6 +19,11 @@ typedef char *iconv_ibp; static const char *repo_encoding = "UTF-8"; static const char *path_encoding = "UTF-8-MAC"; +static size_t dirent_prec_psx_size(size_t max_name_len) +{ + return st_add(offsetof(dirent_prec_psx, d_name), max_name_len); +} + static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c) { const uint8_t *ptr = (const uint8_t *)s; @@ -110,8 +115,8 @@ const char *precompose_argv_prefix(int argc, const char **argv, const char *pref PREC_DIR *precompose_utf8_opendir(const char *dirname) { PREC_DIR *prec_dir = xmalloc(sizeof(PREC_DIR)); - prec_dir->dirent_nfc = xmalloc(sizeof(dirent_prec_psx)); - prec_dir->dirent_nfc->max_name_len = sizeof(prec_dir->dirent_nfc->d_name); + prec_dir->dirent_nfc = xmalloc(dirent_prec_psx_size(NAME_MAX + 1)); + prec_dir->dirent_nfc->max_name_len = NAME_MAX + 1; prec_dir->dirp = opendir(dirname); if (!prec_dir->dirp) { @@ -139,8 +144,7 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir) int ret_errno = errno; if (new_maxlen > prec_dir->dirent_nfc->max_name_len) { - size_t new_len = sizeof(dirent_prec_psx) + new_maxlen - - sizeof(prec_dir->dirent_nfc->d_name); + size_t new_len = dirent_prec_psx_size(new_maxlen); prec_dir->dirent_nfc = xrealloc(prec_dir->dirent_nfc, new_len); prec_dir->dirent_nfc->max_name_len = new_maxlen; diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h index fea06cf28a52df..c7c3cc211e5031 100644 --- a/compat/precompose_utf8.h +++ b/compat/precompose_utf8.h @@ -14,11 +14,12 @@ typedef struct dirent_prec_psx { /* * See http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/dirent.h.html - * NAME_MAX + 1 should be enough, but some systems have - * NAME_MAX=255 and strlen(d_name) may return 508 or 510 - * Solution: allocate more when needed, see precompose_utf8_readdir() + * Start with room for NAME_MAX + 1 bytes, but keep d_name as a + * flexible array. Some systems have NAME_MAX=255 while strlen(d_name) + * from readdir() may return 508 or 510 bytes. Grow the allocation as + * needed in precompose_utf8_readdir(). */ - char d_name[NAME_MAX+1]; + char d_name[FLEX_ARRAY]; } dirent_prec_psx; diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh index 6d5918c8feaf95..ea75fb490d1a2f 100755 --- a/t/t3910-mac-os-precompose.sh +++ b/t/t3910-mac-os-precompose.sh @@ -207,6 +207,22 @@ test_expect_success "Add long precomposed filename" ' git commit -m "Long filename" ' +test_expect_success "status with long non-ASCII filename" ' + test_when_finished "rm -rf long-utf8-status" && + git init long-utf8-status && + ( + cd long-utf8-status && + test "$(git config --bool core.precomposeunicode)" = true && + long_utf8_name=$( + printf "%253s\342\200\224" "" | + tr " " a + ) && + test "$(printf "%s" "$long_utf8_name" | wc -c | tr -d " ")" = 256 && + printf "content\n" >"$long_utf8_name" && + git status --porcelain=v1 >actual + ) +' + test_expect_failure 'handle existing decomposed filenames' ' echo content >"verbatim.$Adiarnfd" && git -c core.precomposeunicode=false add "verbatim.$Adiarnfd" && From 8b90835161cff95e80bc39e8acb25f0f77592ecf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:18 +0000 Subject: [PATCH 12/43] load_one_loose_object_map(): fix resource leak Pointed out by Coverity. While at it, reduce near-duplicate clean-up code at the end of the function. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- loose.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/loose.c b/loose.c index 0b626c1b854642..940a9e0dfee879 100644 --- a/loose.c +++ b/loose.c @@ -65,6 +65,7 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_ { struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT; FILE *fp; + int ret = -1; if (!loose->map) loose_object_map_init(&loose->map); @@ -84,7 +85,6 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_ return 0; } - errno = 0; if (strbuf_getwholeline(&buf, fp, '\n') || strcmp(buf.buf, loose_object_header)) goto err; while (!strbuf_getline_lf(&buf, fp)) { @@ -98,13 +98,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_ insert_loose_map(loose, &oid, &compat_oid); } - strbuf_release(&buf); - strbuf_release(&path); - return errno ? -1 : 0; + ret = ferror(fp) ? -1 : 0; err: + fclose(fp); strbuf_release(&buf); strbuf_release(&path); - return -1; + return ret; } int repo_read_loose_object_map(struct repository *repo) From bd58327406c6868b92fb1b61d85b7430839042d4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:19 +0000 Subject: [PATCH 13/43] loose: avoid closing invalid fd on error path `write_one_object()` opens a file at line 186 and jumps to the errout label on failure. The errout cleanup unconditionally calls `close(fd)`, but when `open()` itself failed, fd is -1. Calling `close(-1)` is harmless on most platforms (returns EBADF) but is undefined behavior per POSIX and can confuse fd tracking in sanitizer builds. Guard the close with fd >= 0. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- loose.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/loose.c b/loose.c index 940a9e0dfee879..bf01d3e42def34 100644 --- a/loose.c +++ b/loose.c @@ -201,7 +201,8 @@ static int write_one_object(struct odb_source_loose *loose, return 0; errout: error_errno(_("failed to write loose object index %s"), path.buf); - close(fd); + if (fd >= 0) + close(fd); rollback_lock_file(&lock); strbuf_release(&buf); strbuf_release(&path); From a62c2a26fcedb635046f8d072fc9d9629e6e87ba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:20 +0000 Subject: [PATCH 14/43] download_https_uri_to_file(): do not leak fd upon failure When the `git-remote-https` command fails, we do not want to leak `child_out`. Pointed out by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- bundle-uri.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle-uri.c b/bundle-uri.c index 3b2e347288c3b7..34fa452e760905 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -378,7 +378,7 @@ static int download_https_uri_to_file(const char *file, const char *uri) if (child_in) fclose(child_in); if (finish_command(&cp)) - return 1; + result = 1; if (child_out) fclose(child_out); return result; From c3f89beb733a260866ec9c163615bce59e77481b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:21 +0000 Subject: [PATCH 15/43] run-command: avoid `close(-1)` in `start_command()` error paths When `start_command()` fails to set up a pipe partway through, it rolls back by closing the pipe ends it has already opened. For descriptors supplied by the caller rather than allocated locally, that rollback tested `if (cmd->in)` / `if (cmd->out)` before calling close(). The CHILD_PROCESS_INIT default of -1 ("no descriptor") is non-zero and so passes the test, meaning a caller that sets cmd->no_stdin or cmd->no_stdout without supplying a real fd ends up triggering close(-1) on the error path. The stdin-pipe failure branch a few lines above already uses the right idiom, `if (cmd->out > 0)`, which rejects both the -1 sentinel and 0 (the parent's own standard streams). Apply it to the three remaining rollback sites. Reported by Coverity as CID 1049722 ("Argument cannot be negative"). Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- run-command.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/run-command.c b/run-command.c index e70a8a387b9042..ce84db87824262 100644 --- a/run-command.c +++ b/run-command.c @@ -706,7 +706,7 @@ int start_command(struct child_process *cmd) failed_errno = errno; if (need_in) close_pair(fdin); - else if (cmd->in) + else if (cmd->in > 0) close(cmd->in); str = "standard output"; goto fail_pipe; @@ -720,11 +720,11 @@ int start_command(struct child_process *cmd) failed_errno = errno; if (need_in) close_pair(fdin); - else if (cmd->in) + else if (cmd->in > 0) close(cmd->in); if (need_out) close_pair(fdout); - else if (cmd->out) + else if (cmd->out > 0) close(cmd->out); str = "standard error"; fail_pipe: From da82221086eddbf3efa15e3fb7bea14303e16cd9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:22 +0000 Subject: [PATCH 16/43] line-log: avoid redundant copy that leaks in process_ranges When `bloom_filter_check()` indicates that a commit does not touch any of the tracked paths, `line_log_process_ranges_arbitrary_commit()` propagates the current ranges to the parent by calling `line_log_data_copy()` and passing the copy to add_line_range(). However, `add_line_range()` always makes its own copy internally (via line_log_data_copy or line_log_data_merge), so the caller's copy is never freed and leaks every time this path is taken. Pass range directly to `add_line_range()` instead of making a redundant intermediate copy. The callee's internal copy handles ownership correctly. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- line-log.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/line-log.c b/line-log.c index 5fc75ae275e03a..0179f138f70288 100644 --- a/line-log.c +++ b/line-log.c @@ -1141,8 +1141,7 @@ int line_log_process_ranges_arbitrary_commit(struct rev_info *rev, struct commit if (range) { if (commit->parents && !bloom_filter_check(rev, commit, range)) { - struct line_log_data *prange = line_log_data_copy(range); - add_line_range(rev, commit->parents->item, prange); + add_line_range(rev, commit->parents->item, range); clear_commit_line_range(rev, commit); } else if (commit->parents && commit->parents->next) changed = process_ranges_merge_commit(rev, commit, range); From 6eb60059bd6d852d41c5e5c43bf5b033abbfca3b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:23 +0000 Subject: [PATCH 17/43] dir: free allocations on parse-error paths in `read_one_dir()` Two of `read_one_dir()`'s parse-error early returns leak ud.untracked and ud.dirs. Plug them. The other early returns in the same function are fine: they occur after the `xmalloc()`+`memcpy()` that copies ud into `*untracked_`, at which point ownership is transferred to the caller. `read_untracked_extension()` then releases everything via `free_untracked_cache()` on failure. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- dir.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dir.c b/dir.c index 32430090dcdf26..23335b9f7ae12b 100644 --- a/dir.c +++ b/dir.c @@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_, ALLOC_ARRAY(ud.untracked, ud.untracked_nr); ud.dirs_alloc = ud.dirs_nr = decode_varint(&data); - if (data > end) + if (data > end) { + free(ud.untracked); return -1; + } ALLOC_ARRAY(ud.dirs, ud.dirs_nr); eos = memchr(data, '\0', end - data); - if (!eos || eos == end) + if (!eos || eos == end) { + free(ud.untracked); + free(ud.dirs); return -1; + } *untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1)); memcpy(untracked, &ud, sizeof(ud)); From add15d11ac1d882fe1a36e3f7a936fb92943ecca Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:24 +0000 Subject: [PATCH 18/43] submodule: fix cwd leak in `get_superproject_working_tree()` `get_superproject_working_tree()` allocates cwd via `xgetcwd()` at the top of the function, but two early-return paths (when not inside a work tree, and when strbuf_realpath for "../" fails) return 0 without freeing it. Redirect these early returns through a cleanup label that frees cwd before returning. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- submodule.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/submodule.c b/submodule.c index fd91201a92d7b0..92dfb0fc2d341f 100644 --- a/submodule.c +++ b/submodule.c @@ -2627,13 +2627,12 @@ int get_superproject_working_tree(struct strbuf *buf) * We might have a superproject, but it is harder * to determine. */ - return 0; + goto out; if (!strbuf_realpath(&one_up, "../", 0)) - return 0; + goto out; subpath = relative_path(cwd, one_up.buf, &sb); - strbuf_release(&one_up); prepare_submodule_repo_env(&cp.env); strvec_pop(&cp.env); @@ -2678,20 +2677,22 @@ int get_superproject_working_tree(struct strbuf *buf) ret = 1; free(super_wt); } - free(cwd); - strbuf_release(&sb); code = finish_command(&cp); if (code == 128) /* '../' is not a git repository */ - return 0; - if (code == 0 && len == 0) + ret = 0; + else if (code == 0 && len == 0) /* There is an unrelated git repository at '../' */ - return 0; - if (code) + ret = 0; + else if (code) die(_("ls-tree returned unexpected return code %d"), code); +out: + strbuf_release(&sb); + strbuf_release(&one_up); + free(cwd); return ret; } From da2211be7461762a47b67449cc3a518b8942ec84 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:25 +0000 Subject: [PATCH 19/43] worktree: fix resource leaks when branch creation fails In the "add" subcommand, when `run_command()` fails while creating a new branch (line 948), the function returns -1 immediately without freeing the allocations made earlier: path (from prefix_filename at line 858), opt_track, branch_to_free, and new_branch_to_free. Redirect the error return through the existing cleanup block at the end of the function so all four allocations are properly freed. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/worktree.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d21c43fde38b5e..4bc7b4f6e7199a 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -945,14 +945,17 @@ static int add(int ac, const char **av, const char *prefix, strvec_push(&cp.args, branch); if (opt_track) strvec_push(&cp.args, opt_track); - if (run_command(&cp)) - return -1; + if (run_command(&cp)) { + ret = -1; + goto cleanup; + } branch = new_branch; } else if (opt_track) { die(_("--[no-]track can only be used if a new branch is created")); } ret = add_worktree(path, branch, &opts); +cleanup: free(path); free(opt_track); free(branch_to_free); From 22f92aa62a70e740acfbb4e4c2bfa70d31c50f83 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:26 +0000 Subject: [PATCH 20/43] imap-send: avoid leaking the IMAP upload buffer When uploading messages via libcurl, `curl_append_msgs_to_imap()` accumulates each one in a strbuf that grows across loop iterations but is never released before the function returns. Release it alongside the existing libcurl cleanup. Reported by Coverity as CID 1671507 ("Resource leak"). Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- imap-send.c | 1 + 1 file changed, 1 insertion(+) diff --git a/imap-send.c b/imap-send.c index cfd6a5120c50e4..0d16d02029232b 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1750,6 +1750,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server, curl_easy_cleanup(curl); curl_global_cleanup(); + strbuf_release(&msgbuf.buf); if (cred.username) { if (res == CURLE_OK) From b3b1d83f366c6f0db13e7d9679b762334a97d847 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:27 +0000 Subject: [PATCH 21/43] reftable/table: release filter on error path `reftable_table_refs_for_unindexed()` allocates a filtering_ref_iterator and then calls `reftable_buf_add()` to populate its oid buffer. On success ownership is transferred to the output iterator, but if `reftable_buf_add()` fails, the goto-out cleanup only frees the table iterator and walks away from both the filter allocation and the oid buffer that `reftable_buf_add()` may have grown. Release filter->oid and free filter alongside the existing table iterator cleanup. Reported by Coverity as CID 1671512 ("Resource leak"). Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- reftable/table.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/reftable/table.c b/reftable/table.c index 56362df0eda5a6..d604ddebf44dda 100644 --- a/reftable/table.c +++ b/reftable/table.c @@ -709,6 +709,10 @@ static int reftable_table_refs_for_unindexed(struct reftable_table *t, if (ti) table_iter_close(ti); reftable_free(ti); + if (filter) { + reftable_buf_release(&filter->oid); + reftable_free(filter); + } } return err; } From e36cda7d7ab1cbd2a096913777d382182872c8db Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:28 +0000 Subject: [PATCH 22/43] fsmonitor: plug token-data leak on early daemon-startup failures `fsmonitor_run_daemon()` allocates `state.current_token_data` before any subordinate setup step that may fail (alias resolution, listener/health constructors, asynchronous IPC server init). On the successful path the listener thread takes ownership and clears the field during its teardown, so the `done:` cleanup block sees a NULL pointer. On every early-error path, however, control jumps straight to `done:` with the freshly allocated token data still referenced, and it is never freed, as Coverity flagged. Free it at the top of `done:` and clear the pointer. The success path is a no-op (the pointer is already NULL there); the error paths now drop the otherwise-leaked allocation. `fsmonitor_free_token_data()` is NULL-safe and asserts `client_ref_count == 0`, which holds trivially here because the IPC server has not yet begun accepting clients when these failures occur. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index f920cf3a8202f6..4161dd82825b4c 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -1418,6 +1418,8 @@ static int fsmonitor_run_daemon(void) err = fsmonitor_run_daemon_1(&state); done: + fsmonitor_free_token_data(state.current_token_data); + state.current_token_data = NULL; pthread_cond_destroy(&state.cookies_cond); pthread_mutex_destroy(&state.main_lock); { From 9184231173dea25e922a0ee6ced938c57bca37e5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:29 +0000 Subject: [PATCH 23/43] mingw: make `exit_process()` own the process handle on all paths After "mingw: kill child processes in a gentler way", the ownership of the HANDLE passed to `exit_process()` and `terminate_process_tree()` is inconsistent. `terminate_process_tree()` always closes the handle; `exit_process()` closes it on success and on the terminate-tree fallback, but leaks it on the early return where GetExitCodeProcess() fails or reports the process is no longer STILL_ACTIVE. `mingw_kill()` compensated by closing the handle on its own error path, which is a double-close on every error path that does not hit that one leaky branch -- the callee has already closed the handle by then. Coverity flagged the resulting use-after-free as CID 1437238. Pin down the invariant that `exit_process()` and `terminate_process_tree()` own the handle from the call onward and close it on every return path; with that, the bogus close in `mingw_kill()` goes away. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/mingw.c | 4 +--- compat/win32/exit-process.h | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 41e055f7de885e..e2cb92a4144b36 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2269,10 +2269,8 @@ int mingw_kill(pid_t pid, int sig) } ret = terminate_process_tree(h, 128 + sig); } - if (ret) { + if (ret) errno = err_win_to_posix(GetLastError()); - CloseHandle(h); - } return ret; } else if (pid > 0 && sig == 0) { HANDLE h = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid); diff --git a/compat/win32/exit-process.h b/compat/win32/exit-process.h index d53989884cfb0c..26004161bcbdc3 100644 --- a/compat/win32/exit-process.h +++ b/compat/win32/exit-process.h @@ -159,6 +159,7 @@ static int exit_process(HANDLE process, int exit_code) return terminate_process_tree(process, exit_code); } + CloseHandle(process); return 0; } From fc1bac6b57a99cefe2c0febbe57bd19bd3e03ab9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:23:56 +0200 Subject: [PATCH 24/43] README: add GitLab CI badge to make it more discoverable The Git project uses CI systems from both GitHub and GitLab. While both of these systems are extensively used in day-to-day work, we only have a link to the GitHub Workflows in our README, which makes the GitLab CI hard to discover. Improve the situation by adding a second badge for GitLab CI to our README. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d87bca1b8c3ebf..46489b0971d04d 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ -[![Build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) +[![GitHub build status](https://github.com/git/git/workflows/CI/badge.svg)](https://github.com/git/git/actions?query=branch%3Amaster+event%3Apush) +[![GitLab build status](https://gitlab.com/git-scm/git/badges/master/pipeline.svg)](https://gitlab.com/git-scm/git/-/pipelines?ref=master) Git - fast, scalable, distributed revision control system ========================================================= From 9769449fc8c9dc508a3cfd4e0af6d182fc6cf619 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:23:57 +0200 Subject: [PATCH 25/43] t0021: skip EXPENSIVE test that is broken without SIZE_T_IS_64BIT One of the tests in t0021 writes a 2GB file and then roundtrips it through the clean/sumdge filters. This test is broken on 32 bit platforms because they typically don't handle files larger then `SSIZE_MAX` well at all. While our CI has a "linux32" job that should in theory hit this issue, we never noticed it because we didn't use to run EXPENSIVE tests until 7a094d68a2 (ci: run expensive tests on push builds to integration branches, 2026-05-08). And after that commit, the test does not fail but instead hangs completely. Ideally, we'd of course properly detect this situation and then test for it. In practice, this turns out to be hard as the test failure are not reliable as they often (but not always) run into ENOMEM errors. Instead, skip the test altogether. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t0021-conversion.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index 033b00a364ee7a..7b9a0ca877c252 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -296,7 +296,7 @@ test_expect_success 'filter that does not read is fine' ' test_cmp expect actual ' -test_expect_success EXPENSIVE 'filter large file' ' +test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'filter large file' ' test_config filter.largefile.smudge cat && test_config filter.largefile.clean cat && test_seq -f "%1048576d" 1 2048 >2GB && From f0598d079afa3110ef662fd543a83923cf72a5f3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:23:58 +0200 Subject: [PATCH 26/43] t4141: fix inefficient use of dd(1) In t4141 we generate a patch that is roughly 1GB in size to verify that git-apply(1) indeed rejects that patch. We generate that patch by prepending a patch header and then executing `test-tool genzeros` without a limit. This causes us to print infinitely many zeros, and we limit the overall amount of generated bytes via `test_copy_bytes`. This test setup is extremely expensive, as `test_copy_bytes` is implemented via `dd ibs=1 count="$1"`, which copies data one byte at a time. So as we write 1GB of data, we end up doing 1 billion reads and writes. This naturally takes a while: it takes 6 minutes on my system, and around 40 minutes in some CI jobs! We can do much better though, as genzeros already knows to handle an optional limit of how much data it is supposed to write, which allows us to remove the call to `test_copy_bytes`. Furthermore, it has already been optimized to generate the data fast. And indeed, doing this conversion drops the test execution to less than a second on my machine. That means that in theory it becomes feasible to drop the EXPENSIVE prerequisite now. But git-apply(1) still soaks up 1GB of data into memory, which may count as being expensive. Consequently, we keep the prerequisite intact. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t4141-apply-too-large.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t4141-apply-too-large.sh b/t/t4141-apply-too-large.sh index eac6f7e151b562..9dbed940db5eb8 100755 --- a/t/t4141-apply-too-large.sh +++ b/t/t4141-apply-too-large.sh @@ -5,7 +5,6 @@ test_description='git apply with too-large patch' . ./test-lib.sh test_expect_success EXPENSIVE 'git apply rejects patches that are too large' ' - sz=$((1024 * 1024 * 1023)) && { cat <<-\EOF && diff --git a/file b/file @@ -14,8 +13,8 @@ test_expect_success EXPENSIVE 'git apply rejects patches that are too large' ' +++ b/file @@ -0,0 +1 @@ EOF - test-tool genzeros - } | test_copy_bytes $sz | test_must_fail git apply 2>err && + test-tool genzeros $((1024 * 1024 * 1023)) + } | test_must_fail git apply 2>err && grep "patch too large" err ' From bc1854f525ecc07043ccf131d18217adb926c876 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:23:59 +0200 Subject: [PATCH 27/43] t5608: reduce maximum disk usage The tests in t5608 perform a couple of clones of repositories that are somewhat large. Ultimately, we end up creating: - A setup repository that contains 2GB of uncompressed pack data. - A bare clone that contains the same 2GB of data. - A clone with worktree writes a 2GB packfile and a 2GB worktree. - A second setup repository that contains a 4GB packfile. - Two 4GB clone of that repository. Some of these clones ultimately hardlink files, which ensures that we at least don't end up with more than 20GB of data. But at the end of the test we still have around 16GB of data, which is only a tiny bit better. Refactor the test to prune repositories after they have no use anymore. This reduced the peak disk usage of this test to 8GB. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t5608-clone-2gb.sh | 66 +++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh index 4f8a95ddda411c..5d56debf1c7181 100755 --- a/t/t5608-clone-2gb.sh +++ b/t/t5608-clone-2gb.sh @@ -10,45 +10,47 @@ then fi test_expect_success 'setup' ' - - git config pack.compression 0 && - git config pack.depth 0 && - blobsize=$((100*1024*1024)) && - blobcount=$((2*1024*1024*1024/$blobsize+1)) && - i=1 && - (while test $i -le $blobcount - do - printf "Generating blob $i/$blobcount\r" >&2 && - printf "blob\nmark :$i\ndata $blobsize\n" && - #test-tool genrandom $i $blobsize && - printf "%-${blobsize}s" $i && - echo "M 100644 :$i $i" >> commit && - i=$(($i+1)) || - echo $? > exit-status - done && - echo "commit refs/heads/main" && - echo "author A U Thor 123456789 +0000" && - echo "committer C O Mitter 123456789 +0000" && - echo "data 5" && - echo ">2gb" && - cat commit) | - git fast-import --big-file-threshold=2 && - test ! -f exit-status - + git init 2gb-repo && + ( + cd 2gb-repo && + git config pack.compression 0 && + git config pack.depth 0 && + blobsize=$((100*1024*1024)) && + blobcount=$((2*1024*1024*1024/$blobsize+1)) && + i=1 && + (while test $i -le $blobcount + do + printf "Generating blob $i/$blobcount\r" >&2 && + printf "blob\nmark :$i\ndata $blobsize\n" && + #test-tool genrandom $i $blobsize && + printf "%-${blobsize}s" $i && + echo "M 100644 :$i $i" >> commit && + i=$(($i+1)) || + echo $? > exit-status + done && + echo "commit refs/heads/main" && + echo "author A U Thor 123456789 +0000" && + echo "committer C O Mitter 123456789 +0000" && + echo "data 5" && + echo ">2gb" && + cat commit) | + git fast-import --big-file-threshold=2 && + test ! -f exit-status + ) ' test_expect_success 'clone - bare' ' - - git clone --bare --no-hardlinks . clone-bare - + test_when_finished rm -rf clone-bare && + git clone --bare --no-hardlinks 2gb-repo clone-bare ' test_expect_success 'clone - with worktree, file:// protocol' ' - - git clone "file://$(pwd)" clone-wt - + test_when_finished rm -rf clone-wt && + git clone "file://$(pwd)/2gb-repo" clone-wt ' +rm -rf 2gb-repo 2>/dev/null + test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'set up repo with >4GB object' ' large_blob_size=$((4*1024*1024*1024+1)) && git init --bare 4gb-repo && @@ -61,6 +63,7 @@ test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'set up repo with >4GB object' ' ' test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone >4GB object via unpack-objects' ' + test_when_finished rm -rf 4gb-clone-unpack && # The synthesized pack has five objects, so a large unpack limit keeps # fetch-pack on the unpack-objects path. git -c fetch.unpackLimit=100 clone --bare \ @@ -77,6 +80,7 @@ test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone >4GB object via unpack-obje ' test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone with >4GB object via index-pack' ' + test_when_finished rm -rf 4gb-clone-index && # Force fetch-pack to hand the pack to index-pack instead. git -c fetch.unpackLimit=1 clone --bare \ "file://$(pwd)/4gb-repo" 4gb-clone-index && From 8f276d146c55247eea4f5304d6b5e9ef293cefaf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:24:00 +0200 Subject: [PATCH 28/43] t7508: skip EXPENSIVE test that is broken without SIZE_T_IS_64BIT One of the tests in t7508 is marked as EXPENSIVE because it ends up creating and adding files that are multiple gigabytes in size. This takes a while to complete, hence the EXPENSIVE prerequisite. Besides being expensive though the test can only work on systems where `size_t` is at least 64 bit. This is because one of the created files is larger than 4GB, and because Git tracks object size via `size_t` it will eventually blow up. This test has also been blowing up in the "linux32" CI job in GitHub Workflows since 7a094d68a2 (ci: run expensive tests on push builds to integration branches, 2026-05-08). But that job doesn't only fail, it also hangs, and that has been concealing the failure. Fix the issue by marking the test as requiring 64 bit `size_t`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t7508-status.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7508-status.sh b/t/t7508-status.sh index c2057bc94c38c1..dfdd78b6fe77ad 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1773,7 +1773,7 @@ test_expect_success 'slow status advice when core.untrackedCache true, and fsmon ) ' -test_expect_success EXPENSIVE 'status does not re-read unchanged 4 or 8 GiB file' ' +test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'status does not re-read unchanged 4 or 8 GiB file' ' ( mkdir large-file && cd large-file && From aa4ee1f0a86e2a01f68af63560d94679fc8dd4f5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:24:01 +0200 Subject: [PATCH 29/43] t7900: clean up large EXPENSIVE repository One of the tests in t7900 is marked with EXPENSIVE because we create a repository with 2GB of data that we end up repacking. We never clean up that repository though, so we occupy the full 2GB of data until the end of the test suite. Besides clogging our disk, having an EXPENSIVE test that alters the repository's state used by subsequent tests is also a bad idea, as it can easily have an impact on the heuristics used by other maintenance tasks. Adapt the test so that we create the data in a standalone repository that we clean up at the end of the test. While at it, also disable auto-maintenance so that it does not race with our manual maintenance. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t7900-maintenance.sh | 56 +++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index d7f82e1bec163f..8a7e1306d07950 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -461,36 +461,42 @@ test_expect_success 'incremental-repack task' ' ' test_expect_success EXPENSIVE 'incremental-repack 2g limit' ' - test_config core.compression 0 && + test_when_finished rm -rf expensive-repo && + git init expensive-repo && + ( + cd expensive-repo && + git config set core.compression 0 && + git config set maintenance.auto false && - for i in $(test_seq 1 5) - do - test-tool genrandom foo$i $((512 * 1024 * 1024 + 1)) >>big || - return 1 - done && - git add big && - git commit -qm "Add big file (1)" && + for i in $(test_seq 1 5) + do + test-tool genrandom foo$i $((512 * 1024 * 1024 + 1)) >>big || + return 1 + done && + git add big && + git commit -qm "Add big file (1)" && - # ensure any possible loose objects are in a pack-file - git maintenance run --task=loose-objects && + # ensure any possible loose objects are in a pack-file + git maintenance run --task=loose-objects && - rm big && - for i in $(test_seq 6 10) - do - test-tool genrandom foo$i $((512 * 1024 * 1024 + 1)) >>big || - return 1 - done && - git add big && - git commit -qm "Add big file (2)" && + rm big && + for i in $(test_seq 6 10) + do + test-tool genrandom foo$i $((512 * 1024 * 1024 + 1)) >>big || + return 1 + done && + git add big && + git commit -qm "Add big file (2)" && - # ensure any possible loose objects are in a pack-file - git maintenance run --task=loose-objects && + # ensure any possible loose objects are in a pack-file + git maintenance run --task=loose-objects && - # Now run the incremental-repack task and check the batch-size - GIT_TRACE2_EVENT="$(pwd)/run-2g.txt" git maintenance run \ - --task=incremental-repack 2>/dev/null && - test_subcommand git multi-pack-index repack \ - --no-progress --batch-size=2147483647 /dev/null && + test_subcommand git multi-pack-index repack \ + --no-progress --batch-size=2147483647 Date: Mon, 6 Jul 2026 08:24:02 +0200 Subject: [PATCH 30/43] t: use `test_bool_env` to parse GIT_TEST_LONG It's currently hard to explicitly disable GIT_TEST_LONG by setting it to `false`. Fix this by using `test_bool_env` instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ci/lib.sh | 2 +- t/test-lib.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/lib.sh b/ci/lib.sh index b939110a6eefcf..01a0bc6b7557e5 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -321,7 +321,7 @@ export SKIP_DASHED_BUILT_INS=YesPlease # enable the long tests for pushes to the integration branches as well. case "$GITHUB_EVENT_NAME,$CI_BRANCH" in pull_request,*|push,*next*|push,*master*|push,*main*|push,*maint*) - export GIT_TEST_LONG=YesPlease + export GIT_TEST_LONG=true ;; esac diff --git a/t/test-lib.sh b/t/test-lib.sh index ceefb99bff60e0..623fcfb747b529 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -210,7 +210,7 @@ parse_option () { -i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate) immediate=t ;; -l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests) - GIT_TEST_LONG=t; export GIT_TEST_LONG ;; + GIT_TEST_LONG=true; export GIT_TEST_LONG ;; -r) mark_option_requires_arg "$opt" run_list ;; @@ -1849,7 +1849,7 @@ test_lazy_prereq AUTOIDENT ' ' test_lazy_prereq EXPENSIVE ' - test -n "$GIT_TEST_LONG" + test_bool_env GIT_TEST_LONG false ' test_lazy_prereq EXPENSIVE_ON_WINDOWS ' From 3e35bf7eff61ac43c8ae3fabaf617c6fba8cf5ed Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:24:03 +0200 Subject: [PATCH 31/43] gitlab-ci: disable RAM disk on macOS jobs When we added the macOS jobs to GitLab CI in 56090a35ab (ci: add macOS jobs to GitLab CI, 2024-01-18) we had to work around some very slow disks. This workaround essentially creates a RAM disk that we mount, where all test data is being written into RAM instead of the real disk. In the next commit though we're about to enable "GIT_TEST_LONG", which will make tests run that are marked with the "EXPENSIVE" prerequisite. This change will make a couple of tests run that write up to 8GB of data into the test output directory. As our RAM disk is only 4GB in size, this change will cause ENOSPC errors. We could accommodate for this by increasing the size of the RAM disk. In c9d708b7fc (gitlab-ci: upgrade macOS runners, 2026-05-21) we have upgraded our runners to use the "large" runners, which have 16GB of RAM available. So we could easily expand the RAM disk to a capacity of for example 12GB. But some test runs have shown that this is still quite flaky overall, as we get quite close to our limits. Instead, drop the workaround completely. This does indeed slow down execution of the test jobs: - osx-clang goes from 18 minutes to 25 minutes - osx-meson goes from 21 minutes to 33 minutes - osx-reftable stays at 21 minutes The last one seems like an outlier. The only explanation that I have is that we end up writing significantly less files with the reftable backend, which ultimately causes less I/O. Overall though, it's preferable to have something that works with the least amount of flakiness compared to having something else that is faster but unstable. Despite that, the macOS jobs aren't even the slowest jobs, so this doesn't extend the overall pipeline's length. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1a8e90932cc292..a4aebe8b719ca8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -88,13 +88,8 @@ test:osx: tags: - saas-macos-large-m2pro variables: - TEST_OUTPUT_DIRECTORY: "/Volumes/RAMDisk" + TEST_OUTPUT_DIRECTORY: "/tmp/test-output" before_script: - # Create a 4GB RAM disk that we use to store test output on. This small hack - # significantly speeds up tests by more than a factor of 2 because the - # macOS runners use network-attached storage as disks, which is _really_ - # slow with the many small writes that our tests do. - - sudo diskutil apfs create $(hdiutil attach -nomount ram://8192000) RAMDisk - ./ci/install-dependencies.sh script: - ./ci/run-build-and-tests.sh From 84248444ad9577ba7f0bf0679d57c629166eb903 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jul 2026 08:24:04 +0200 Subject: [PATCH 32/43] gitlab-ci: enable "GIT_TEST_LONG" Starting with 7a094d68a2 (ci: run expensive tests on push builds to integration branches, 2026-05-08) we run expensive tests in our CI for certain events. So far, this has only been wired up for GitHub Workflows though, which creates a test gap for GitLab CI. Plug this gap by also making this work for the latter. Note that these tests cannot be run on the Windows runners, as they only have 7.5GB of RAM. This is insufficient for some of the EXPENSIVE tests, so we explicitly disable "GIT_TEST_LONG" on these jobs. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 6 ++++++ ci/lib.sh | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a4aebe8b719ca8..1c4d04da9dcd4c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -147,6 +147,9 @@ test:mingw64: needs: - job: "build:mingw64" artifacts: true + variables: + # Windows runners don't have enough RAM to run EXPENSIVE tests. + GIT_TEST_LONG: false before_script: - *windows_before_script - git-sdk/usr/bin/bash.exe -l -c 'tar xf artifacts/artifacts.tar.gz' @@ -195,6 +198,9 @@ test:msvc-meson: script: - | & "C:/Program Files/Git/usr/bin/bash.exe" -l -c 'ci/run-test-slice-meson.sh build $CI_NODE_INDEX $CI_NODE_TOTAL' + variables: + # Windows runners don't have enough RAM to run EXPENSIVE tests. + GIT_TEST_LONG: false after_script: - | if ($env:CI_JOB_STATUS -ne "success") { diff --git a/ci/lib.sh b/ci/lib.sh index 01a0bc6b7557e5..6c52154eac11e9 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -215,6 +215,7 @@ then test macos != "$CI_OS_NAME" || CI_OS_NAME=osx CI_REPO_SLUG="$GITHUB_REPOSITORY" CI_JOB_ID="$GITHUB_RUN_ID" + CI_EVENT="$GITHUB_EVENT_NAME" CC="${CC_PACKAGE:-${CC:-gcc}}" DONT_SKIP_TAGS=t handle_failed_tests () { @@ -239,6 +240,13 @@ then CI_BRANCH="$CI_COMMIT_REF_NAME" CI_COMMIT="$CI_COMMIT_SHA" + case "$CI_PIPELINE_SOURCE" in + merge_request_event) + CI_EVENT=pull_request;; + *) + CI_EVENT="$CI_PIPELINE_SOURCE";; + esac + case "$OS,$CI_JOB_IMAGE" in Windows_NT,*) CI_OS_NAME=windows @@ -319,9 +327,9 @@ export SKIP_DASHED_BUILT_INS=YesPlease # enable "expensive" tests for PR events. # In order to catch bugs introduced at integration time by mismerges, # enable the long tests for pushes to the integration branches as well. -case "$GITHUB_EVENT_NAME,$CI_BRANCH" in +case "$CI_EVENT,$CI_BRANCH" in pull_request,*|push,*next*|push,*master*|push,*main*|push,*maint*) - export GIT_TEST_LONG=true + export GIT_TEST_LONG=${GIT_TEST_LONG:-true} ;; esac From 3792b2aea4371c2c6f65cac2ece9b2718ad61b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mantas=20Mikul=C4=97nas?= Date: Wed, 13 May 2026 10:08:03 +0300 Subject: [PATCH 33/43] sideband: allow ANSI SGR with colon-separated subfields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SGR values used for 256-color formatting are officially defined to be a single field with :-separated subfields (e.g. "\e[1;38:5:XX;40m") despite the more common but kludgy use of separate values (which then become context-dependent and lead to misinterpretation by incompatible terminals). Signed-off-by: Mantas Mikulėnas Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sideband.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sideband.c b/sideband.c index 1523a53e1d781d..4a2c5f858f60ad 100644 --- a/sideband.c +++ b/sideband.c @@ -163,6 +163,10 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n) * * ESC [ [ [; ]*] m * + * where can be either zero-length, or a decimal number, or a + * series of decimal numbers separated by a colon (for 256-color or + * true-color codes). + * * These are part of the Select Graphic Rendition sequences which * contain more than just color sequences, for more details see * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR. @@ -210,7 +214,7 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n) strbuf_add(dest, src, i + 1); return i; } - if (!isdigit(src[i]) && src[i] != ';') + if (!isdigit(src[i]) && src[i] != ':' && src[i] != ';') break; } From 9b204b825bcaf656ea6a2cb0657ef907db21a0e6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Jul 2026 23:52:49 -0400 Subject: [PATCH 34/43] 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 f6473dcc8eca3f..6692f7cd812d0e 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 f3966584683990..53a8cb9dd770e7 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 57d9bd4a656d1a..22f36ecf80c824 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 19eb6a1b61c3a7..faf0f120ac186b 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 1cc82a134db22e..bf114a7856f162 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 f3849bb6542e62..93a9caa58269ab 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 b166f8962463e0..7e813915242ba2 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 1568f0ed9cb07b..589c1969e45a4e 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 3c23cbba27a9ec..60f6f8f0546cf4 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 63abbaae8a7570..0341de5031ab20 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 035d0052796060..304b83f8597fa3 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 5adfb3f2726fb3..c3b8db7c5c41a6 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 83eaf88541eefb..24033a9101545a 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 21ca58beeaa28c..1d3d9c119cfb2a 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 8232542585cad4..216100925a1843 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 fbf67fe6bd548f..89b02680119de3 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 f0ee61c8b47a65..1f7163695f6a2a 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 3fa534fbdf8fab..7719fb3a76e439 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 bd4ac6a6e1f05f..19f4efd410832c 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 00000000000000..04270ee043347f --- /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 1c1d27b0eee935..131b4f5a620ee3 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 35/43] 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 bf114a7856f162..510f193a15b682 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 7719fb3a76e439..fd116c87ba276b 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 04270ee043347f..d0e2e5f4b1899b 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 36/43] 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 0a23ef4dfd4e67..121ecf13aae6be 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 37/43] 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 55d1d41770366a..b1296f0018d0dc 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 121ecf13aae6be..cf94ad57002788 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 38/43] 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 | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/csum-file.c b/csum-file.c index 7e813915242ba2..fe18ee1de3cd0c 100644 --- a/csum-file.c +++ b/csum-file.c @@ -55,19 +55,14 @@ void hashflush(struct hashfile *f) } } -static void free_hashfile_memory(struct hashfile *f) +void free_hashfile(struct hashfile *f) { + git_hash_discard(&f->ctx); free(f->buffer); free(f->check_buffer); free(f); } -void free_hashfile(struct hashfile *f) -{ - git_hash_discard(&f->ctx); - free_hashfile_memory(f); -} - int finalize_hashfile(struct hashfile *f, unsigned char *result, enum fsync_component component, unsigned int flags) { @@ -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 39/43] 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 0341de5031ab20..caccf2108e4479 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 6b0639150f4b71..729c51904d39ad 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 40/43] 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 b1296f0018d0dc..82f7e2440455ff 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; } From 936eb75730b6e25248fcf0537811f867311d1341 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 16 Jul 2026 14:27:50 +0000 Subject: [PATCH 41/43] wincred: avoid memory corruption when erasing a credential The earlier d22a488482 (wincred: avoid memory corruption, 2025-11-17) repaired only get_credential(); match_cred_password() has the same defect and is reached on `git credential reject`. When Git asks the helper to erase a stored credential whose password was supplied by the caller, the helper copies the candidate's password into a freshly allocated buffer for comparison. That copy overruns the allocation by one WCHAR of NUL, which on uninstrumented Windows manifests as process termination with status 0xC0000374. Because the helper can die before reaching CredDeleteW(), `git credential reject` masks the failure and the rejected credential remains stored. CredentialBlobSize is documented as a byte count, so for an N-WCHAR blob it equals N * sizeof(WCHAR). The pre-fix code allocated that many bytes and asked wcsncpy_s to copy N wide characters, but wcsncpy_s always appends a terminating NUL WCHAR, writing one WCHAR past the allocation. The destination-capacity argument was also passed in bytes rather than in WCHAR elements as the API requires, so the safe-CRT runtime never rejected the copy. See GHSA-rxqw-wxqg-g7hw. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/credential/wincred/git-credential-wincred.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 73c2b9b72ab53e..190bbccdf9e84c 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -121,10 +121,10 @@ static int match_part_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim) static int match_cred_password(const CREDENTIALW *cred) { int ret; - WCHAR *cred_password = xmalloc(cred->CredentialBlobSize); - wcsncpy_s(cred_password, cred->CredentialBlobSize, - (LPCWSTR)cred->CredentialBlob, - cred->CredentialBlobSize / sizeof(WCHAR)); + size_t wlen = cred->CredentialBlobSize / sizeof(WCHAR); + WCHAR *cred_password = xmalloc((wlen + 1) * sizeof(WCHAR)); + wcsncpy_s(cred_password, wlen + 1, + (LPCWSTR)cred->CredentialBlob, wlen); ret = !wcscmp(cred_password, password); free(cred_password); return ret; From f635ab9ab496ad710fec16f3d854e064110d158f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 16 Jul 2026 14:27:51 +0000 Subject: [PATCH 42/43] wincred: prevent silent credential loss when storing OAuth tokens When `git credential approve` hands the wincred helper a password together with an `oauth_refresh_token`, the OAuth branch of `store_credential()` writes one WCHAR past the allocation while formatting both fields into a single `CredentialBlob`. On Windows this trips heap verification and tears the helper down with status `0xC0000374`; `approve` masks the failure, so the credential the user meant to save never reaches `CredWriteW()` and the next session prompts for it again. The bug has the same shape as the one fixed in the previous commit: the allocation leaves no room for the terminating NUL, and the `sizeOfBuffer` argument to `_snwprintf_s()` is a byte count where the API expects a WCHAR count, which lets the safe-CRT runtime write the terminator out of bounds. Apply the same remedy d22a488482 (wincred: avoid memory corruption, 2025-11-17) applied in `get_credential()`: allocate `(wlen + 1) * sizeof(WCHAR)` bytes and pass `wlen + 1` as the destination capacity in WCHARs. This closes the second of the two heap writes tracked under GHSA-rxqw-wxqg-g7hw. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/credential/wincred/git-credential-wincred.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 190bbccdf9e84c..22eb27ca31dea0 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -208,8 +208,8 @@ static void store_credential(void) if (oauth_refresh_token) { wlen = _scwprintf(L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token); - secret = xmalloc(sizeof(WCHAR) * wlen); - _snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token); + secret = xmalloc((wlen + 1) * sizeof(WCHAR)); + _snwprintf_s(secret, wlen + 1, wlen, L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token); } else { secret = _wcsdup(password); } From 41365c2a9ba347870b80881c0d67454edd22fd49 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 16 Jul 2026 21:31:50 -0700 Subject: [PATCH 43/43] The 4th batch for Git 2.56 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.56.0.adoc | 46 ++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/Documentation/RelNotes/2.56.0.adoc b/Documentation/RelNotes/2.56.0.adoc index 471b84194c4d58..b2d2c8a674a7d6 100644 --- a/Documentation/RelNotes/2.56.0.adoc +++ b/Documentation/RelNotes/2.56.0.adoc @@ -169,3 +169,49 @@ Fixes since v2.55 when running tests with the 'GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1' environment variable have been plugged. (merge 459088ec2e jk/bloom-leak-fixes later to maint). + + * The wincred credential helper has been updated to avoid memory + corruption when erasing credentials and to prevent silent + credential loss when storing OAuth tokens, by correcting buffer + allocations and arguments passed to safe-CRT APIs. + (merge f635ab9ab4 js/wincred-fixes later to maint). + + * Various code paths that initialize a cryptographic hash context but + bail out or finish without calling 'git_hash_final()' have been taught + to call 'git_hash_discard()' to release allocated resources, fixing + memory leaks when Git is built with non-default backends like + 'OpenSSL' or 'libgcrypt'. + (merge 600588d2aa jk/hash-algo-leak-fixes later to maint). + + * Various resource leaks, invalid file descriptor closures, and process + handle ownership issues flagged by Coverity have been fixed. + (merge 9184231173 js/coverity-fixes later to maint). + + * Dockerized CI jobs running in private GitHub repositories have been + adjusted to use explicit process and file limits, preventing resource + exhaustion errors on private runners. + (merge bad766fbac js/ci-dockerized-pid-limit later to maint). + + * Various test scripts have been updated to clean up large temporary + files and repositories, reducing peak disk usage during testing. + Also, expensive tests have been disabled on platforms that lack + sufficient resources (like 32-bit platforms and Windows CI runners), + and the long test suite has been enabled in GitLab CI. + (merge 84248444ad ps/t-fixes-for-git-test-long later to maint). + + * The UTF-8 precomposition wrapper on macOS has been updated to use a + flexible array member to represent the name of a directory entry, + preventing fortified libc checks from failing when the name is + reallocated to be larger than 'NAME_MAX' bytes. + (merge 1eb281159f ih/precompose-flex-array later to maint). + + * The 'git_hash_*()' wrappers have been updated to be used consistently + across the codebase instead of direct calls to members of 'struct + git_hash_algo', and 'git_hash_discard()' has been made idempotent to + simplify cleanups. + (merge 9e396aa553 jk/git-hash-cleanups later to maint). + + * The sideband demultiplexer has been updated to recognize ANSI SGR + escape sequences that use colon-separated subfields (e.g., for + 256-color or true-color codes). + (merge 3792b2aea4 mm/sideband-ansi-sgr-colon-fix later to maint).