Re: [PATCH] name-rev: fix an 'may be used uninitialized' error

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 04/05/2026 2:13 am, Junio C Hamano wrote:
> 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 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