{"thread":{"id":"65581","subject":"[PATCH] name-rev: fix an 'may be used uninitialized' error","startedAt":"2026-05-03T15:19:49Z","lastAt":"2026-05-05T19:09:50Z","messageCount":8,"participants":["Ramsay Jones","Kristoffer Haugsbakk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"542611","messageId":"e74a8fd8-0617-46a8-8bef-a454d51a99c1@ramsayjones.plus.com","threadId":"65581","inReplyTo":null,"subject":"[PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2026-05-03T15:16:38Z","receivedAt":"2026-05-03T15:19:49Z","isPatch":true,"body":"\nToday's seen branch fails to build (with DEVELOPER=1), like so:\n\n CC builtin/name-rev.o\n builtin/name-rev.c: In function ‘cmd_format_rev’:\n builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized]\n 885 | if (!commit) {\n | ^\n builtin/name-rev.c:867:40: note: ‘commit’ was declared here\n 867 | struct commit *commit;\n | ^~~~~~\n cc1: all warnings being treated as errors\n make: *** [Makefile:2932: builtin/name-rev.o] Error 1\n\nThis can be fixed in several ways; initialise the 'commit' variable to\nNULL (on line 867), initialise 'commit' to NULL on the line before the\nconditional on line 883, or (as I chose here) initialise the 'commit'\nvariable in an else arm of the conditional.\n\nSigned-off-by: Ramsay Jones \n---\n\nHi Kristoffer,\n\nI wrote this patch yesterday, just before I had to go out, and didn't\nget around to sending it to the list. Today, the problem has gone\naway ... (along with the 'kh/name-rev-custom-format' branch)!\n\nAssuming you will be sending a new version soon, ... could you please\nsquash this (or similar) into the patch corresponding to commit 5903855b1c\n(\"format-rev: introduce builtin for on-demand pretty formatting\", 2026-04-29).\n\nNote that I don't think this particular fix is better than any other, it\nwas just that my cursor was on that line in vim ... :)\n\nATB,\nRamsay Jones\n\n builtin/name-rev.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex b941e93834..5b7f7a00e5 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -882,6 +882,8 @@ int cmd_format_rev(int argc,\n \t\t\tpeeled = deref_tag(the_repository, object, scratch_buf.buf, 0);\n \t\t\tif (peeled && peeled->type == OBJ_COMMIT)\n \t\t\t\tcommit = (struct commit *)peeled;\n+\t\t\telse\n+\t\t\t\tcommit = NULL;\n \t\t\tif (!commit) {\n \t\t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n \t\t\t\t\t*argv);\n-- \n2.54.0\n"},{"id":"542618","messageId":"66710fd7-23bb-4b1f-852a-f61ea1f188e0@app.fastmail.com","threadId":"65581","inReplyTo":"e74a8fd8-0617-46a8-8bef-a454d51a99c1@ramsayjones.plus.com","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2026-05-03T18:52:00Z","receivedAt":"2026-05-03T18:52:23Z","isPatch":true,"body":"On Sun, May 3, 2026, at 17:16, Ramsay Jones wrote:\n> Today's seen branch fails to build (with DEVELOPER=1), like so:\n>\n> CC builtin/name-rev.o\n> builtin/name-rev.c: In function ‘cmd_format_rev’:\n> builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized\n> [-Werror=maybe-uninitialized]\n> 885 | if (!commit) {\n> | ^\n> builtin/name-rev.c:867:40: note: ‘commit’ was declared here\n> 867 | struct commit *commit;\n> | ^~~~~~\n> cc1: all warnings being treated as errors\n> make: *** [Makefile:2932: builtin/name-rev.o] Error 1\n>\n> This can be fixed in several ways; initialise the 'commit' variable to\n> NULL (on line 867), initialise 'commit' to NULL on the line before the\n> conditional on line 883, or (as I chose here) initialise the 'commit'\n> variable in an else arm of the conditional.\n>\n> Signed-off-by: Ramsay Jones \n> ---\n>\n> Hi Kristoffer,\n>\n> I wrote this patch yesterday, just before I had to go out, and didn't\n> get around to sending it to the list. Today, the problem has gone\n> away ... (along with the 'kh/name-rev-custom-format' branch)!\n>\n> Assuming you will be sending a new version soon, ... could you please\n> squash this (or similar) into the patch corresponding to commit 5903855b1c\n> (\"format-rev: introduce builtin for on-demand pretty formatting\", 2026-04-29).\n>\n> Note that I don't think this particular fix is better than any other, it\n> was just that my cursor was on that line in vim ... :)\n>\n> ATB,\n> Ramsay Jones\n\nI’ll incorporate it. Thank you!\n\n>\n> builtin/name-rev.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n>[snip]\n"},{"id":"542641","messageId":"xmqqv7d4ou3m.fsf@gitster.g","threadId":"65581","inReplyTo":"e74a8fd8-0617-46a8-8bef-a454d51a99c1@ramsayjones.plus.com","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-04T01:13:01Z","receivedAt":"2026-05-04T01:13:04Z","isPatch":true,"body":"Ramsay Jones writes:\n\n> Today's seen branch fails to build (with DEVELOPER=1), like so:\n>\n> CC builtin/name-rev.o\n> builtin/name-rev.c: In function ‘cmd_format_rev’:\n> builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized]\n> 885 | if (!commit) {\n> | ^\n> builtin/name-rev.c:867:40: note: ‘commit’ was declared here\n> 867 | struct commit *commit;\n> | ^~~~~~\n> cc1: all warnings being treated as errors\n> make: *** [Makefile:2932: builtin/name-rev.o] Error 1\n> ...\n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index b941e93834..5b7f7a00e5 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -882,6 +882,8 @@ int cmd_format_rev(int argc,\n> \t\t\tpeeled = deref_tag(the_repository, object, scratch_buf.buf, 0);\n> \t\t\tif (peeled && peeled->type == OBJ_COMMIT)\n> \t\t\t\tcommit = (struct commit *)peeled;\n> +\t\t\telse\n> +\t\t\t\tcommit = NULL;\n> \t\t\tif (!commit) {\n> \t\t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n> \t\t\t\t\t*argv);\n\nWhy not\n\n\t\t\tif (peeled && peeled->type == OBJ_COMMIT) {\n\t\t\t\tcommit = (struct commit *)peeled;\n\t\t\t} else {\n\t\t\t\tfprintf(stderr, \"... skipping ...\");\n\t\t\t\tcontinue;\n\t\t\t}\n\n\t\t\tget_format_rev(commit, &format_pp, &scratch);\n\nor even\n\n\t\t\tif (!peeled || peeled->type != OBJ_COMMIT) {\n\t\t\t\tfprintf(stderr, \"... skipping ...\");\n\t\t\t\tcontinue;\n\t\t\t}\n\n\t\t\tget_format_rev((struct commit *)peeled->type,\n\t\t\t\t\t&format_pp, &scratch);\n\nand dropping the variable \"struct commit *commit\" altogether?\n\n\n"},{"id":"542646","messageId":"592c01fd-1e1b-4850-adf1-77fffdf71321@app.fastmail.com","threadId":"65581","inReplyTo":"xmqqv7d4ou3m.fsf@gitster.g","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-05-04T08:55:16Z","receivedAt":"2026-05-04T08:55:37Z","isPatch":true,"body":"Hi Junio\n\nOn Mon, May 4, 2026, at 03:13, Junio C Hamano wrote:\n> Ramsay Jones writes:\n>\n>> Today's seen branch fails to build (with DEVELOPER=1), like so:\n>>\n>> CC builtin/name-rev.o\n>> builtin/name-rev.c: In function ‘cmd_format_rev’:\n>> builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized]\n>> 885 | if (!commit) {\n>> | ^\n>> builtin/name-rev.c:867:40: note: ‘commit’ was declared here\n>> 867 | struct commit *commit;\n>> | ^~~~~~\n>> cc1: all warnings being treated as errors\n>> make: *** [Makefile:2932: builtin/name-rev.o] Error 1\n>> ...\n>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n>> index b941e93834..5b7f7a00e5 100644\n>> --- a/builtin/name-rev.c\n>> +++ b/builtin/name-rev.c\n>> @@ -882,6 +882,8 @@ int cmd_format_rev(int argc,\n>> \t\t\tpeeled = deref_tag(the_repository, object, scratch_buf.buf, 0);\n>> \t\t\tif (peeled && peeled->type == OBJ_COMMIT)\n>> \t\t\t\tcommit = (struct commit *)peeled;\n>> +\t\t\telse\n>> +\t\t\t\tcommit = NULL;\n>> \t\t\tif (!commit) {\n>> \t\t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n>> \t\t\t\t\t*argv);\n>\n> Why not\n>\n> \t\t\tif (peeled && peeled->type == OBJ_COMMIT) {\n> \t\t\t\tcommit = (struct commit *)peeled;\n> \t\t\t} else {\n> \t\t\t\tfprintf(stderr, \"... skipping ...\");\n> \t\t\t\tcontinue;\n> \t\t\t}\n>\n> \t\t\tget_format_rev(commit, &format_pp, &scratch);\n>\n> or even\n>\n> \t\t\tif (!peeled || peeled->type != OBJ_COMMIT) {\n> \t\t\t\tfprintf(stderr, \"... skipping ...\");\n> \t\t\t\tcontinue;\n> \t\t\t}\n>\n> \t\t\tget_format_rev((struct commit *)peeled->type,\n> \t\t\t\t\t&format_pp, &scratch);\n>\n> and dropping the variable \"struct commit *commit\" altogether?\n\nI see that you added this as one of two “SQUASH???” commits on your\nkh/name-rev-custom-format branch. I will squash both of them in for the\nnext round.\n\nThanks to both of you.\n"},{"id":"542729","messageId":"b04e98e3-0840-456d-a627-351f2378c037@ramsayjones.plus.com","threadId":"65581","inReplyTo":"xmqqv7d4ou3m.fsf@gitster.g","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2026-05-04T20:26:21Z","receivedAt":"2026-05-04T20:29:31Z","isPatch":true,"body":"\n\nOn 04/05/2026 2:13 am, Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> Today's seen branch fails to build (with DEVELOPER=1), like so:\n>>\n>> CC builtin/name-rev.o\n>> builtin/name-rev.c: In function ‘cmd_format_rev’:\n>> builtin/name-rev.c:885:28: error: ‘commit’ may be used uninitialized [-Werror=maybe-uninitialized]\n>> 885 | if (!commit) {\n>> | ^\n>> builtin/name-rev.c:867:40: note: ‘commit’ was declared here\n>> 867 | struct commit *commit;\n>> | ^~~~~~\n>> cc1: all warnings being treated as errors\n>> make: *** [Makefile:2932: builtin/name-rev.o] Error 1\n>> ...\n>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n>> index b941e93834..5b7f7a00e5 100644\n>> --- a/builtin/name-rev.c\n>> +++ b/builtin/name-rev.c\n>> @@ -882,6 +882,8 @@ int cmd_format_rev(int argc,\n>> \t\t\tpeeled = deref_tag(the_repository, object, scratch_buf.buf, 0);\n>> \t\t\tif (peeled && peeled->type == OBJ_COMMIT)\n>> \t\t\t\tcommit = (struct commit *)peeled;\n>> +\t\t\telse\n>> +\t\t\t\tcommit = NULL;\n>> \t\t\tif (!commit) {\n>> \t\t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n>> \t\t\t\t\t*argv);\n> \n> Why not\n\nHeh, you noticed that I spent all of a few seconds writing this patch, just to get\nthe branch to build, as I was in a rush to go out. I wasn't quick enough anyway, so\nI didn't send it until the next day. But, as I said in the patch, I wasn't pushing\nthis patch as _the_ fix ...\n\n> \n> \t\t\tif (peeled && peeled->type == OBJ_COMMIT) {\n> \t\t\t\tcommit = (struct commit *)peeled;\n> \t\t\t} else {\n> \t\t\t\tfprintf(stderr, \"... skipping ...\");\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \n> \t\t\tget_format_rev(commit, &format_pp, &scratch);\n> \n> or even\n> \n> \t\t\tif (!peeled || peeled->type != OBJ_COMMIT) {\n> \t\t\t\tfprintf(stderr, \"... skipping ...\");\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \n> \t\t\tget_format_rev((struct commit *)peeled->type,\n> \t\t\t\t\t&format_pp, &scratch);\n> \n> and dropping the variable \"struct commit *commit\" altogether?\n\nHaving now spent some time (well at least 30 seconds :) ) looking at the\nsurrounding code, then your final suggestion looks really good to me! ;)\n\nHowever, these 'maybe-uninitialized' errors (historically have been) somewhat\nsensitive to the level of optimization used in the compilation and even algo\nused by the compiler changing frequently from one version to the next ...\nSo, I wasn't sure if Kristoffer was actually seeing the error or had the\nDEVELOPER variable set (which is why I mentioned it in passing!).\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n"},{"id":"542732","messageId":"cccf9618-31de-447b-ab17-4fb8cee23363@app.fastmail.com","threadId":"65581","inReplyTo":"b04e98e3-0840-456d-a627-351f2378c037@ramsayjones.plus.com","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-05-04T21:56:57Z","receivedAt":"2026-05-04T21:57:19Z","isPatch":true,"body":"On Mon, May 4, 2026, at 22:26, Ramsay Jones wrote:\n> On 04/05/2026 2:13 am, Junio C Hamano wrote:\n>> Ramsay Jones writes:\n>>>[snip]\n>\n> Having now spent some time (well at least 30 seconds :) ) looking at the\n> surrounding code, then your final suggestion looks really good to me! ;)\n>\n> However, these 'maybe-uninitialized' errors (historically have been) somewhat\n> sensitive to the level of optimization used in the compilation and even algo\n> used by the compiler changing frequently from one version to the next ...\n> So, I wasn't sure if Kristoffer was actually seeing the error or had the\n> DEVELOPER variable set (which is why I mentioned it in passing!).\n\nThis is what I had when maybe-uninit. didn’t fail for me.\n\n $ cat config.mak\n DEVELOPER=1\n DEBUG=1\n CC = ccache gcc\n CFLAGS+=-O0\n CFLAGS+=-ggdb3\n USE_ASCIIDOCTOR=true\n\nI switched to the whole config.mak.dev enchilada and now it fails\nas it should.\n"},{"id":"542736","messageId":"aad833e9-d34e-4e57-a1e7-99dc0c6c7d24@ramsayjones.plus.com","threadId":"65581","inReplyTo":"cccf9618-31de-447b-ab17-4fb8cee23363@app.fastmail.com","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2026-05-05T00:41:45Z","receivedAt":"2026-05-05T00:44:55Z","isPatch":true,"body":"\n\nOn 04/05/2026 10:56 pm, Kristoffer Haugsbakk wrote:\n> On Mon, May 4, 2026, at 22:26, Ramsay Jones wrote:\n>> On 04/05/2026 2:13 am, Junio C Hamano wrote:\n>>> Ramsay Jones writes:\n>>>> [snip]\n>>\n>> Having now spent some time (well at least 30 seconds :) ) looking at the\n>> surrounding code, then your final suggestion looks really good to me! ;)\n>>\n>> However, these 'maybe-uninitialized' errors (historically have been) somewhat\n>> sensitive to the level of optimization used in the compilation and even algo\n>> used by the compiler changing frequently from one version to the next ...\n>> So, I wasn't sure if Kristoffer was actually seeing the error or had the\n>> DEVELOPER variable set (which is why I mentioned it in passing!).\n> \n> This is what I had when maybe-uninit. didn’t fail for me.\n> \n> $ cat config.mak\n> DEVELOPER=1\n> DEBUG=1\n> CC = ccache gcc\n> CFLAGS+=-O0\n\nAh, yes -O0 will disable the warning/error. Normally CFLAGS would be set to\nsomething like 'CFLAGS = -g -O2 -Wall'. (which still produces a binary you\ncan reasonably use with gdb).\n\n> CFLAGS+=-ggdb3\n> USE_ASCIIDOCTOR=true\n> \n> I switched to the whole config.mak.dev enchilada and now it fails\n> as it should.\n> \n\n"},{"id":"542767","messageId":"f3e3130b-40a7-43f9-b8dc-41ab57de5f2b@app.fastmail.com","threadId":"65581","inReplyTo":"aad833e9-d34e-4e57-a1e7-99dc0c6c7d24@ramsayjones.plus.com","subject":"Re: [PATCH] name-rev: fix an 'may be used uninitialized' error","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-05-05T19:09:28Z","receivedAt":"2026-05-05T19:09:50Z","isPatch":true,"body":"On Tue, May 5, 2026, at 02:41, Ramsay Jones wrote:\n> On 04/05/2026 10:56 pm, Kristoffer Haugsbakk wrote:\n>> On Mon, May 4, 2026, at 22:26, Ramsay Jones wrote:\n>>>[snip]\n>>\n>> This is what I had when maybe-uninit. didn’t fail for me.\n>>\n>> $ cat config.mak\n>> DEVELOPER=1\n>> DEBUG=1\n>> CC = ccache gcc\n>> CFLAGS+=-O0\n>\n> Ah, yes -O0 will disable the warning/error. Normally CFLAGS would be set to\n> something like 'CFLAGS = -g -O2 -Wall'. (which still produces a binary you\n> can reasonably use with gdb).\n\nThanks :)\n\n>\n>> CFLAGS+=-ggdb3\n>> USE_ASCIIDOCTOR=true\n>>[snip]\n"}]}