From 1bad05bacc8b9bf45c3b8f576e2ea514472e5737 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 4 Aug 2025 09:00:11 -0400 Subject: [PATCH 1/6] revert: initialize const value When building with clang-22 and DEVELOPER=1 mode, this warning causes us to fail compilation: builtin/revert.c:114:13: error: default initialization of an object of type 'const char' leaves the object uninitialized [-Werror,-Wdefault-const-init-var-unsafe] 114 | const char sentinel_value; | ^ The compiler is right that this code is a bit funny. We declare a const value without an initializer. It cannot be assigned to because of the const, but without an initializer it has no predictable value. So as a variable it can never have any useful function, and if we tried to look at it, we'd get undefined behavior. But it does have a function. We never use its value, but rather use its address as a sentinel value for some other variables: const char *gpg_sign = &sentinel_value; ...maybe set gpg_sign via parse_options... if (gpg_sign != &sentinel_value) ...we got a non-default value... Normally we'd use NULL as a sentinel value for a pointer, but it doesn't work here because we also want to detect --no-gpg-sign, which is marked by setting the pointer to NULL. We need a separate "this was not touched" value, which is what this sentinel variable gives us. So the code is correct as-is, but the sentinel variable itself is funny enough that it's understandable for a compiler warning to flag it. Let's try to appease the compiler. There are a few possible options: 1. Instead of a variable, we could just construct an artificial sentinel address like "1", "-1", etc. I think these technically fall afoul of the C standard (even if we do not access them, even constructing invalid pointers is not always allowed). But it's also something we do elsewhere, and even happens in some standard interfaces (e.g., mmap()'s MMAP_FAILED value). It does involve some annoying casts, though. 2. We can mark it as static. That gives it a definite value, but perhaps makes people wonder if the static-ness is important, when it's not. 3. We can just give it a value to shut the compiler up, even though nobody cares about that value. I went with (3) here as the smallest and most obvious change. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/revert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/revert.c b/builtin/revert.c index aca6c293cdfb2f..a5b8c206f2ed76 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -111,7 +111,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix, const char * const * usage_str = revert_or_cherry_pick_usage(opts); const char *me = action_name(opts); const char *cleanup_arg = NULL; - const char sentinel_value; + const char sentinel_value = 0; /* value not important */ const char *strategy = &sentinel_value; const char *gpg_sign = &sentinel_value; enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED; From 56730059884ef97b60c7a2347a22bbfc009142b2 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Sat, 2 Aug 2025 17:08:03 -0500 Subject: [PATCH 2/6] archive: flush deflate stream until Z_STREAM_END In `archive-zip.c:write_zip_entry()` when using a stream as input for deflating a file, the call to `git_deflate()` with Z_FINISH always expects Z_STREAM_END to be returned. Per zlib documentation[1]: If the parameter flush is set to Z_FINISH, pending input is processed, pending output is flushed and deflate returns with Z_STREAM_END if there was enough output space. If deflate returns with Z_OK or Z_BUF_ERROR, this function must be called again with Z_FINISH and more output space (updated avail_out) but no more input data, until it returns with Z_STREAM_END or an error. After deflate has returned Z_STREAM_END, the only possible operations on the stream are deflateReset or deflateEnd. In scenarios where the output buffer is not large enough to write all the compressed data, it is perfectly valid for the underlying `deflate()` to return Z_OK. Thus, expecting a single pass of `deflate()` here to always return Z_STREAM_END is a bug. Update the code to flush the deflate stream until Z_STREAM_END is returned. [1]: https://zlib.net/manual.html Helped-by: Toon Claes Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- archive-zip.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/archive-zip.c b/archive-zip.c index dbd90d9c3d4b32..bea5bdd43dc43e 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -492,14 +492,22 @@ static int write_zip_entry(struct archiver_args *args, zstream.next_in = buf; zstream.avail_in = 0; - result = git_deflate(&zstream, Z_FINISH); - if (result != Z_STREAM_END) - die("deflate error (%d)", result); + + do { + result = git_deflate(&zstream, Z_FINISH); + if (result != Z_OK && result != Z_STREAM_END) + die("deflate error (%d)", result); + + out_len = zstream.next_out - compressed; + if (out_len > 0) { + write_or_die(1, compressed, out_len); + compressed_size += out_len; + zstream.next_out = compressed; + zstream.avail_out = sizeof(compressed); + } + } while (result != Z_STREAM_END); git_deflate_end(&zstream); - out_len = zstream.next_out - compressed; - write_or_die(1, compressed, out_len); - compressed_size += out_len; zip_offset += compressed_size; write_zip_data_desc(size, compressed_size, crc); From eb883b05da4489cef3fe7961a61faaa7533a9541 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 4 Aug 2025 22:31:13 -0700 Subject: [PATCH 3/6] remote: bail early from set_head() if missing remote name In "git remote set-head", we can take varying numbers of arguments depending on whether we saw the "-d" or "-a" options. But the first argument is always the remote name. The current code is somewhat awkward in that it conditionally handles the remote name up-front like this: if (argc) remote = ...from argv[0]... and then only later decides to bail if we do not have the right number of arguments for the options we saw. This makes it hard to figure out if "remote" is always set when it needs to be. Both for humans, but also for compilers; with -Og, gcc complains that "remote" can be accessed without being initialized (although this is not true, as we'd always die with a usage message in that case). Let's instead enforce the presence of the remote argument up front, which fixes the compiler warning and is easier to understand. It does mean duplicating the code to print a usage message, but it's a single line. Noticed-by: Denton Liu Signed-off-by: Jeff King Tested-by: Denton Liu Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/remote.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 0d6755bcb71e3d..b6808bb28b0d96 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1457,10 +1457,13 @@ static int set_head(int argc, const char **argv, const char *prefix, }; argc = parse_options(argc, argv, prefix, options, builtin_remote_sethead_usage, 0); - if (argc) { - strbuf_addf(&b_head, "refs/remotes/%s/HEAD", argv[0]); - remote = remote_get(argv[0]); - } + + /* All modes require at least a remote name. */ + if (!argc) + usage_with_options(builtin_remote_sethead_usage, options); + + strbuf_addf(&b_head, "refs/remotes/%s/HEAD", argv[0]); + remote = remote_get(argv[0]); if (!opt_a && !opt_d && argc == 2) { head_name = xstrdup(argv[1]); From 3a7e783d9c5334cc5ea1af74b42297560ce697ce Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 4 Aug 2025 22:31:16 -0700 Subject: [PATCH 4/6] t/unit-tests/clar: fix -Wmaybe-uninitialized with -Og When building with -Og on gcc 15.1.1, the build produces a warning. In practice, though, this cannot be hit because `exact` acts as a guard and that variable can only be set after `matchlen` is already initialized Assign a default value to `matchlen` so that the warning is silenced. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/unit-tests/clar/clar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index d54e4553674908..03a3aa8e873bfc 100644 --- a/t/unit-tests/clar/clar.c +++ b/t/unit-tests/clar/clar.c @@ -350,7 +350,7 @@ static void clar_run_suite(const struct clar_suite *suite, const char *filter) { const struct clar_func *test = suite->tests; - size_t i, matchlen; + size_t i, matchlen = 0; struct clar_report *report; int exact = 0; From 45dea789b0d7852a0dafbd14e3cdf7e518c4d0a3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 7 Aug 2025 07:09:25 +0200 Subject: [PATCH 5/6] Documentation/RelNotes/2.51.0: improve wording for a couple entries Improve wording and fix typos for a couple entries part of the Git 2.51 release notes. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.51.0.adoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/RelNotes/2.51.0.adoc b/Documentation/RelNotes/2.51.0.adoc index f8adc2c5cf3b61..e2cd673f4339de 100644 --- a/Documentation/RelNotes/2.51.0.adoc +++ b/Documentation/RelNotes/2.51.0.adoc @@ -97,8 +97,8 @@ Performance, Internal Implementation, Development Support etc. * "git push" and "git fetch" are taught to update refs in batches to gain performance. - * Some code paths in the "git prune" used to ignore passed in - repository object and used the_repository singleton instance + * Some code paths in "git prune" used to ignore the passed-in + repository object and used the `the_repository` singleton instance instead, which has been corrected. * Update ".clang-format" and ".editorconfig" to match our style guide @@ -139,7 +139,7 @@ Performance, Internal Implementation, Development Support etc. * Redefine where the multi-pack-index sits in the object subsystem, which recently was restructured to allow multiple backends that support a single object source that belongs to one repository. A - midx does span mulitple "object sources". + MIDX does span multiple "object sources". * Reduce implicit assumption and dependence on the_repository in the object-file subsystem. @@ -292,8 +292,8 @@ including security updates, are included in this release. and also they learn to honor the -U command-line option. (merge 2b3ae04011 lm/add-p-context later to maint). - * The case where a new submodule takes a path where used to be a - completely different subproject is now dealt a bit better than + * The case where a new submodule takes a path where there used to be a + completely different subproject is now dealt with a bit better than before. (merge 5ed8c5b465 kj/renamed-submodule later to maint). From 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 7 Aug 2025 08:12:53 -0700 Subject: [PATCH 6/6] Git 2.51-rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.51.0.adoc | 6 ++++++ GIT-VERSION-GEN | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.51.0.adoc b/Documentation/RelNotes/2.51.0.adoc index e2cd673f4339de..a73ea3e8086ce4 100644 --- a/Documentation/RelNotes/2.51.0.adoc +++ b/Documentation/RelNotes/2.51.0.adoc @@ -297,6 +297,10 @@ including security updates, are included in this release. before. (merge 5ed8c5b465 kj/renamed-submodule later to maint). + * The deflate codepath in "git archive --format=zip" had a + longstanding bug coming from misuse of zlib API, which has been + corrected. + * Other code cleanup, docfix, build fix, etc. (merge b257adb571 lo/my-first-ow-doc-update later to maint). (merge 8b34b6a220 ly/sequencer-update-squash-is-fixup-only later to maint). @@ -333,3 +337,5 @@ including security updates, are included in this release. (merge 3bdd897413 ms/meson-with-ancient-git-wo-ls-files-dedup later to maint). (merge cca758d324 kh/doc-fast-import-historical later to maint). (merge 9b0781196a jc/test-hashmap-is-still-here later to maint). + (merge 1bad05bacc jk/revert-squelch-compiler-warning later to maint). + (merge 3a7e783d9c dl/squelch-maybe-uninitialized later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 63463c87739aa0..e7e91190ae2d0d 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,6 +1,6 @@ #!/bin/sh -DEF_VER=v2.50.GIT +DEF_VER=v2.51.0-rc1 LF=' '