# [PATCH] name-rev: fix an 'may be used uninitialized' error 8 messages from 2026-05-03 to 2026-05-05. Participants: Ramsay Jones, Kristoffer Haugsbakk, Junio C Hamano. Thread: https://gitlist.dev/t/65581 ## Ramsay Jones, 2026-05-03 15:16 Subject: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: ``` Today's seen branch fails to build (with DEVELOPER=1), like so: CC builtin/name-rev.o builtin/name-rev.c: In function ‘cmd_format_rev’: builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized] 885 | if (!commit) { | ^ builtin/name-rev.c:867:40: note: ‘commit’ was declared here 867 | struct commit *commit; | ^~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:2932: builtin/name-rev.o] Error 1 This can be fixed in several ways; initialise the 'commit' variable to NULL (on line 867), initialise 'commit' to NULL on the line before the conditional on line 883, or (as I chose here) initialise the 'commit' variable in an else arm of the conditional. Signed-off-by: Ramsay Jones --- Hi Kristoffer, I wrote this patch yesterday, just before I had to go out, and didn't get around to sending it to the list. Today, the problem has gone away ... (along with the 'kh/name-rev-custom-format' branch)! Assuming you will be sending a new version soon, ... could you please squash this (or similar) into the patch corresponding to commit 5903855b1c ("format-rev: introduce builtin for on-demand pretty formatting", 2026-04-29). Note that I don't think this particular fix is better than any other, it was just that my cursor was on that line in vim ... :) ATB, Ramsay Jones builtin/name-rev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/name-rev.c b/builtin/name-rev.c index b941e93834..5b7f7a00e5 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -882,6 +882,8 @@ int cmd_format_rev(int argc, peeled = deref_tag(the_repository, object, scratch_buf.buf, 0); if (peeled && peeled->type == OBJ_COMMIT) commit = (struct commit *)peeled; + else + commit = NULL; if (!commit) { fprintf(stderr, "Could not get commit for %s. Skipping.\n", *argv); -- 2.54.0 ``` ## Kristoffer Haugsbakk, 2026-05-03 18:52 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: <66710fd7-23bb-4b1f-852a-f61ea1f188e0@app.fastmail.com> In-Reply-To: ``` On Sun, May 3, 2026, at 17:16, Ramsay Jones wrote: > Today's seen branch fails to build (with DEVELOPER=1), like so: > > CC builtin/name-rev.o > builtin/name-rev.c: In function ‘cmd_format_rev’: > builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized > [-Werror=maybe-uninitialized] > 885 | if (!commit) { > | ^ > builtin/name-rev.c:867:40: note: ‘commit’ was declared here > 867 | struct commit *commit; > | ^~~~~~ > cc1: all warnings being treated as errors > make: *** [Makefile:2932: builtin/name-rev.o] Error 1 > > This can be fixed in several ways; initialise the 'commit' variable to > NULL (on line 867), initialise 'commit' to NULL on the line before the > conditional on line 883, or (as I chose here) initialise the 'commit' > variable in an else arm of the conditional. > > Signed-off-by: Ramsay Jones > --- > > Hi Kristoffer, > > I wrote this patch yesterday, just before I had to go out, and didn't > get around to sending it to the list. Today, the problem has gone > away ... (along with the 'kh/name-rev-custom-format' branch)! > > Assuming you will be sending a new version soon, ... could you please > squash this (or similar) into the patch corresponding to commit 5903855b1c > ("format-rev: introduce builtin for on-demand pretty formatting", 2026-04-29). > > Note that I don't think this particular fix is better than any other, it > was just that my cursor was on that line in vim ... :) > > ATB, > Ramsay Jones I’ll incorporate it. Thank you! > > builtin/name-rev.c | 2 ++ > 1 file changed, 2 insertions(+) > >[snip] ``` ## Junio C Hamano, 2026-05-04 01:13 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: In-Reply-To: ``` Ramsay Jones writes: > Today's seen branch fails to build (with DEVELOPER=1), like so: > > CC builtin/name-rev.o > builtin/name-rev.c: In function ‘cmd_format_rev’: > builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized] > 885 | if (!commit) { > | ^ > builtin/name-rev.c:867:40: note: ‘commit’ was declared here > 867 | struct commit *commit; > | ^~~~~~ > cc1: all warnings being treated as errors > make: *** [Makefile:2932: builtin/name-rev.o] Error 1 > ... > diff --git a/builtin/name-rev.c b/builtin/name-rev.c > index b941e93834..5b7f7a00e5 100644 > --- a/builtin/name-rev.c > +++ b/builtin/name-rev.c > @@ -882,6 +882,8 @@ int cmd_format_rev(int argc, > peeled = deref_tag(the_repository, object, scratch_buf.buf, 0); > if (peeled && peeled->type == OBJ_COMMIT) > commit = (struct commit *)peeled; > + else > + commit = NULL; > if (!commit) { > fprintf(stderr, "Could not get commit for %s. Skipping.\n", > *argv); Why not if (peeled && peeled->type == OBJ_COMMIT) { commit = (struct commit *)peeled; } else { fprintf(stderr, "... skipping ..."); continue; } get_format_rev(commit, &format_pp, &scratch); or even if (!peeled || peeled->type != OBJ_COMMIT) { fprintf(stderr, "... skipping ..."); continue; } get_format_rev((struct commit *)peeled->type, &format_pp, &scratch); and dropping the variable "struct commit *commit" altogether? ``` ## Kristoffer Haugsbakk, 2026-05-04 08:55 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: <592c01fd-1e1b-4850-adf1-77fffdf71321@app.fastmail.com> In-Reply-To: ``` Hi Junio On Mon, May 4, 2026, at 03:13, Junio C Hamano wrote: > Ramsay Jones writes: > >> Today's seen branch fails to build (with DEVELOPER=1), like so: >> >> CC builtin/name-rev.o >> builtin/name-rev.c: In function ‘cmd_format_rev’: >> builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized] >> 885 | if (!commit) { >> | ^ >> builtin/name-rev.c:867:40: note: ‘commit’ was declared here >> 867 | struct commit *commit; >> | ^~~~~~ >> cc1: all warnings being treated as errors >> make: *** [Makefile:2932: builtin/name-rev.o] Error 1 >> ... >> diff --git a/builtin/name-rev.c b/builtin/name-rev.c >> index b941e93834..5b7f7a00e5 100644 >> --- a/builtin/name-rev.c >> +++ b/builtin/name-rev.c >> @@ -882,6 +882,8 @@ int cmd_format_rev(int argc, >> peeled = deref_tag(the_repository, object, scratch_buf.buf, 0); >> if (peeled && peeled->type == OBJ_COMMIT) >> commit = (struct commit *)peeled; >> + else >> + commit = NULL; >> if (!commit) { >> fprintf(stderr, "Could not get commit for %s. Skipping.\n", >> *argv); > > Why not > > if (peeled && peeled->type == OBJ_COMMIT) { > commit = (struct commit *)peeled; > } else { > fprintf(stderr, "... skipping ..."); > continue; > } > > get_format_rev(commit, &format_pp, &scratch); > > or even > > if (!peeled || peeled->type != OBJ_COMMIT) { > fprintf(stderr, "... skipping ..."); > continue; > } > > get_format_rev((struct commit *)peeled->type, > &format_pp, &scratch); > > and dropping the variable "struct commit *commit" altogether? I see that you added this as one of two “SQUASH???” commits on your kh/name-rev-custom-format branch. I will squash both of them in for the next round. Thanks to both of you. ``` ## Ramsay Jones, 2026-05-04 20:26 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: In-Reply-To: ``` On 04/05/2026 2:13 am, Junio C Hamano wrote: > Ramsay Jones writes: > >> Today's seen branch fails to build (with DEVELOPER=1), like so: >> >> CC builtin/name-rev.o >> builtin/name-rev.c: In function ‘cmd_format_rev’: >> builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized] >> 885 | if (!commit) { >> | ^ >> builtin/name-rev.c:867:40: note: ‘commit’ was declared here >> 867 | struct commit *commit; >> | ^~~~~~ >> cc1: all warnings being treated as errors >> make: *** [Makefile:2932: builtin/name-rev.o] Error 1 >> ... >> diff --git a/builtin/name-rev.c b/builtin/name-rev.c >> index b941e93834..5b7f7a00e5 100644 >> --- a/builtin/name-rev.c >> +++ b/builtin/name-rev.c >> @@ -882,6 +882,8 @@ int cmd_format_rev(int argc, >> peeled = deref_tag(the_repository, object, scratch_buf.buf, 0); >> if (peeled && peeled->type == OBJ_COMMIT) >> commit = (struct commit *)peeled; >> + else >> + commit = NULL; >> if (!commit) { >> fprintf(stderr, "Could not get commit for %s. Skipping.\n", >> *argv); > > Why not Heh, you noticed that I spent all of a few seconds writing this patch, just to get the branch to build, as I was in a rush to go out. I wasn't quick enough anyway, so I didn't send it until the next day. But, as I said in the patch, I wasn't pushing this patch as _the_ fix ... > > if (peeled && peeled->type == OBJ_COMMIT) { > commit = (struct commit *)peeled; > } else { > fprintf(stderr, "... skipping ..."); > continue; > } > > get_format_rev(commit, &format_pp, &scratch); > > or even > > if (!peeled || peeled->type != OBJ_COMMIT) { > fprintf(stderr, "... skipping ..."); > continue; > } > > get_format_rev((struct commit *)peeled->type, > &format_pp, &scratch); > > and dropping the variable "struct commit *commit" altogether? Having now spent some time (well at least 30 seconds :) ) looking at the surrounding code, then your final suggestion looks really good to me! ;) However, these 'maybe-uninitialized' errors (historically have been) somewhat sensitive to the level of optimization used in the compilation and even algo used by the compiler changing frequently from one version to the next ... So, I wasn't sure if Kristoffer was actually seeing the error or had the DEVELOPER variable set (which is why I mentioned it in passing!). Thanks! ATB, Ramsay Jones ``` ## Kristoffer Haugsbakk, 2026-05-04 21:56 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: In-Reply-To: ``` On Mon, May 4, 2026, at 22:26, Ramsay Jones wrote: > On 04/05/2026 2:13 am, Junio C Hamano wrote: >> Ramsay Jones writes: >>>[snip] > > Having now spent some time (well at least 30 seconds :) ) looking at the > surrounding code, then your final suggestion looks really good to me! ;) > > However, these 'maybe-uninitialized' errors (historically have been) somewhat > sensitive to the level of optimization used in the compilation and even algo > used by the compiler changing frequently from one version to the next ... > So, I wasn't sure if Kristoffer was actually seeing the error or had the > DEVELOPER variable set (which is why I mentioned it in passing!). This is what I had when maybe-uninit. didn’t fail for me. $ cat config.mak DEVELOPER=1 DEBUG=1 CC = ccache gcc CFLAGS+=-O0 CFLAGS+=-ggdb3 USE_ASCIIDOCTOR=true I switched to the whole config.mak.dev enchilada and now it fails as it should. ``` ## Ramsay Jones, 2026-05-05 00:41 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: In-Reply-To: ``` On 04/05/2026 10:56 pm, Kristoffer Haugsbakk wrote: > On Mon, May 4, 2026, at 22:26, Ramsay Jones wrote: >> On 04/05/2026 2:13 am, Junio C Hamano wrote: >>> Ramsay Jones writes: >>>> [snip] >> >> Having now spent some time (well at least 30 seconds :) ) looking at the >> surrounding code, then your final suggestion looks really good to me! ;) >> >> However, these 'maybe-uninitialized' errors (historically have been) somewhat >> sensitive to the level of optimization used in the compilation and even algo >> used by the compiler changing frequently from one version to the next ... >> So, I wasn't sure if Kristoffer was actually seeing the error or had the >> DEVELOPER variable set (which is why I mentioned it in passing!). > > This is what I had when maybe-uninit. didn’t fail for me. > > $ cat config.mak > DEVELOPER=1 > DEBUG=1 > CC = ccache gcc > CFLAGS+=-O0 Ah, yes -O0 will disable the warning/error. Normally CFLAGS would be set to something like 'CFLAGS = -g -O2 -Wall'. (which still produces a binary you can reasonably use with gdb). > CFLAGS+=-ggdb3 > USE_ASCIIDOCTOR=true > > I switched to the whole config.mak.dev enchilada and now it fails > as it should. > ``` ## Kristoffer Haugsbakk, 2026-05-05 19:09 Subject: Re: [PATCH] name-rev: fix an 'may be used uninitialized' error Message-ID: In-Reply-To: ``` On Tue, May 5, 2026, at 02:41, Ramsay Jones wrote: > On 04/05/2026 10:56 pm, Kristoffer Haugsbakk wrote: >> On Mon, May 4, 2026, at 22:26, Ramsay Jones wrote: >>>[snip] >> >> This is what I had when maybe-uninit. didn’t fail for me. >> >> $ cat config.mak >> DEVELOPER=1 >> DEBUG=1 >> CC = ccache gcc >> CFLAGS+=-O0 > > Ah, yes -O0 will disable the warning/error. Normally CFLAGS would be set to > something like 'CFLAGS = -g -O2 -Wall'. (which still produces a binary you > can reasonably use with gdb). Thanks :) > >> CFLAGS+=-ggdb3 >> USE_ASCIIDOCTOR=true >>[snip] ```