Re: [PATCH] name-rev: fix an 'may be used uninitialized' error
- From
Ramsay Jones <ramsay@ramsayjones.plus.com>
- Date
- May 4, 2026, 20:26 UTC
- Message-ID
- <b04e98e3-0840-456d-a627-351f2378c037@ramsayjones.plus.com>
- In-Reply-To
- <xmqqv7d4ou3m.fsf@gitster.g>
On 04/05/2026 2:13 am, Junio C Hamano wrote:
Show 30 quoted lines
> Ramsay Jones <ramsay@ramsayjones.plus.com> 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 notHeh, 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 ...
Show 21 quoted lines
>
> 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