{"thread":{"id":"58800","subject":"[PATCH] builtin/gc.c: fix -Wdeclaration-after-statement","startedAt":"2022-11-15T08:04:25Z","lastAt":"2022-11-15T18:54:51Z","messageCount":10,"participants":["Ævar Arnfjörð Bjarmason","Johannes Schindelin","ronan@rjp.ie","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"467301","messageId":"patch-1.1-54d405f15f1-20221115T080212Z-avarab@gmail.com","threadId":"58800","inReplyTo":null,"subject":"[PATCH] builtin/gc.c: fix -Wdeclaration-after-statement","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-15T08:04:12Z","receivedAt":"2022-11-15T08:04:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In 1f80129d61b (maintenance: add option to register in a specific\nconfig, 2022-11-09) code was added which triggers a\n\"-Wdeclaration-after-statement\" warning, which is on by default with\nDEVELOPER=1.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nThis landed in the latest \"next\" push-out, causing e.g this CI\nfailure:\nhttps://github.com/git/git/actions/runs/3467538041/jobs/5792504315\n\n builtin/gc.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 56b107e7f0b..22a990db0be 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1543,6 +1543,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \tint found = 0;\n \tstruct string_list_item *item;\n \tconst struct string_list *list;\n+\tstruct config_set cs;\n \n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t builtin_maintenance_unregister_usage, 0);\n@@ -1550,7 +1551,6 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tusage_with_options(builtin_maintenance_unregister_usage,\n \t\t\t\t options);\n \n-\tstruct config_set cs;\n \tif (config_file) {\n \t\tgit_configset_init(&cs);\n \t\tgit_configset_add_file(&cs, config_file);\n-- \n2.38.0.1473.g172bcc0511c\n\n"},{"id":"467304","messageId":"3p6n2r7s-q136-n7q9-8o27-740q4qr5os84@tzk.qr","threadId":"58800","inReplyTo":"patch-1.1-54d405f15f1-20221115T080212Z-avarab@gmail.com","subject":"Re: [PATCH] builtin/gc.c: fix -Wdeclaration-after-statement","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-11-15T09:37:29Z","receivedAt":"2022-11-15T09:37:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Tue, 15 Nov 2022, Ævar Arnfjörð Bjarmason wrote:\n\n> In 1f80129d61b (maintenance: add option to register in a specific\n> config, 2022-11-09) code was added which triggers a\n> \"-Wdeclaration-after-statement\" warning, which is on by default with\n> DEVELOPER=1.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n>\n> This landed in the latest \"next\" push-out, causing e.g this CI\n> failure:\n> https://github.com/git/git/actions/runs/3467538041/jobs/5792504315\n\nI looked at the same thing all morning, and found that\nhttps://lore.kernel.org/git/20221111231910.26769-1-ronan@rjp.ie/\n_probably_ was designed to fix the same issue (and I think you agree that\nthat patch should be used instead of yours because it is more complete in\nfixing left-over issues).\n\nHowever, try as I might, I did not find out yet why it does not apply\ncleanly over here (I got side-tracked into range-diff'ing patches\ndownloaded from the archive, which is quite the challenge and not even fun\nbecause `range-diff` requires commits, not emails, so I side-tracked\nmyself into teaching `range-diff` to accept mbox arguments).\n\nMaybe you can adjust that patch so it applies cleanly?\n\nCiao,\nDscho\n\n>\n> builtin/gc.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 56b107e7f0b..22a990db0be 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -1543,6 +1543,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n> \tint found = 0;\n> \tstruct string_list_item *item;\n> \tconst struct string_list *list;\n> +\tstruct config_set cs;\n>\n> \targc = parse_options(argc, argv, prefix, options,\n> \t\t\t builtin_maintenance_unregister_usage, 0);\n> @@ -1550,7 +1551,6 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n> \t\tusage_with_options(builtin_maintenance_unregister_usage,\n> \t\t\t\t options);\n>\n> -\tstruct config_set cs;\n> \tif (config_file) {\n> \t\tgit_configset_init(&cs);\n> \t\tgit_configset_add_file(&cs, config_file);\n> --\n> 2.38.0.1473.g172bcc0511c\n>\n>\n>\n"},{"id":"467305","messageId":"r31o6p7q-po67-osr1-3qq8-93s4p3o23nq2@tzk.qr","threadId":"58800","inReplyTo":"3p6n2r7s-q136-n7q9-8o27-740q4qr5os84@tzk.qr","subject":"Re: [PATCH] builtin/gc.c: fix -Wdeclaration-after-statement","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-11-15T09:54:05Z","receivedAt":"2022-11-15T09:54:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Tue, 15 Nov 2022, Johannes Schindelin wrote:\n\n> On Tue, 15 Nov 2022, Ævar Arnfjörð Bjarmason wrote:\n>\n> > In 1f80129d61b (maintenance: add option to register in a specific\n> > config, 2022-11-09) code was added which triggers a\n> > \"-Wdeclaration-after-statement\" warning, which is on by default with\n> > DEVELOPER=1.\n> >\n> > Signed-off-by: Ævar Arnfjörð Bjarmason \n> > ---\n> >\n> > This landed in the latest \"next\" push-out, causing e.g this CI\n> > failure:\n> > https://github.com/git/git/actions/runs/3467538041/jobs/5792504315\n>\n> I looked at the same thing all morning, and found that\n> https://lore.kernel.org/git/20221111231910.26769-1-ronan@rjp.ie/\n> _probably_ was designed to fix the same issue (and I think you agree that\n> that patch should be used instead of yours because it is more complete in\n> fixing left-over issues).\n>\n> However, try as I might, I did not find out yet why it does not apply\n> cleanly over here (I got side-tracked into range-diff'ing patches\n> downloaded from the archive, which is quite the challenge and not even fun\n> because `range-diff` requires commits, not emails, so I side-tracked\n> myself into teaching `range-diff` to accept mbox arguments).\n>\n> Maybe you can adjust that patch so it applies cleanly?\n\nSeems that the v2 of \"maintenance: improve error reporting for\nunregister\" [*1*] was _partially_ folded into v3 of \"maintenance: add\noption to register in a specific config\": At least the config change that\ntalks about `--config-file` inste of `--config` _is_ part of v3, and one\nreason that the former patch does not apply cleanly on top of the latter\npatch. Which is a bit funny because the former patch was sent out two days\n_after_ the latter patch.\n\nAnd it looks as if there is one more thing that needs to be forward-ported\nfrom that no-longer-applying patch: the initialization of the configset,\notherwise we're potentially trying to clear an uninitialized data\nstructure.\n\nThis is the patch I am currently testing (on top of Git for Windows'\n`shears/seen` branch:\nhttps://github.com/git-for-windows/git/commit/cd7b86d19f):\n\n-- snip --\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 635f12499d68..a3f63880dfba 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1569,6 +1569,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \tint found = 0;\n \tstruct string_list_item *item;\n \tconst struct string_list *list;\n+\tstruct config_set cs;\n\n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t builtin_maintenance_unregister_usage, 0);\n@@ -1576,9 +1577,8 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tusage_with_options(builtin_maintenance_unregister_usage,\n \t\t\t\t options);\n\n-\tstruct config_set cs;\n+\tgit_configset_init(&cs);\n \tif (config_file) {\n-\t\tgit_configset_init(&cs);\n \t\tgit_configset_add_file(&cs, config_file);\n \t\tlist = git_configset_get_value_multi(&cs, key);\n \t} else {\n-- snap --\n\nÆvar, can you please have a thorough look and see whether there is\nanything else we were missing before advancing this to `next`?\n\nThanks,\nDscho\n\nFootnote *1*: https://lore.kernel.org/git/20221111231910.26769-1-ronan@rjp.ie/\nFootnote *2*: https://lore.kernel.org/git/20221109190708.22725-3-ronan@rjp.ie/\n"},{"id":"467308","messageId":"patch-v2-1.1-f37e99c9d59-20221115T160240Z-avarab@gmail.com","threadId":"58800","inReplyTo":"patch-1.1-54d405f15f1-20221115T080212Z-avarab@gmail.com","subject":"[PATCH v2] maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-15T16:04:27Z","receivedAt":"2022-11-15T16:04:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since (maintenance: add option to register in a specific config,\n2022-11-09) we've been unable to build with \"DEVELOPER=1\" without\n\"DEVOPTS=no-error\", as the added code triggers a\n\"-Wdeclaration-after-statement\" warning.\n\nAnd worse than that, the data handed to git_configset_clear() is\nuninitialized, as can be spotted with e.g.:\n\n\t./t7900-maintenance.sh -vixd --run=23 --valgrind\n\t[...]\n\t+ git maintenance unregister --force\n\tConditional jump or move depends on uninitialised value(s)\n\t at 0x6B5F1E: git_configset_clear (config.c:2367)\n\t by 0x4BA64E: maintenance_unregister (gc.c:1619)\n\t by 0x4BD278: cmd_maintenance (gc.c:2650)\n\t by 0x409905: run_builtin (git.c:466)\n\t by 0x40A21C: handle_builtin (git.c:721)\n\t by 0x40A58E: run_argv (git.c:788)\n\t by 0x40AF68: cmd_main (git.c:926)\n\t by 0x5D39FE: main (common-main.c:57)\n\t Uninitialised value was created by a stack allocation\n\t at 0x4BA22C: maintenance_unregister (gc.c:1557)\n\nLet's fix both of these issues, and also move the scope of the\nvariable to the \"if\" statement it's used in, to make it obvious where\nit's used.\n\nHelped-by: Johannes Schindelin \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\nRange-diff against v1:\n1: 54d405f15f1 ! 1: f37e99c9d59 builtin/gc.c: fix -Wdeclaration-after-statement\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - builtin/gc.c: fix -Wdeclaration-after-statement\n + maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement\n \n - In 1f80129d61b (maintenance: add option to register in a specific\n - config, 2022-11-09) code was added which triggers a\n - \"-Wdeclaration-after-statement\" warning, which is on by default with\n - DEVELOPER=1.\n + Since (maintenance: add option to register in a specific config,\n + 2022-11-09) we've been unable to build with \"DEVELOPER=1\" without\n + \"DEVOPTS=no-error\", as the added code triggers a\n + \"-Wdeclaration-after-statement\" warning.\n \n + And worse than that, the data handed to git_configset_clear() is\n + uninitialized, as can be spotted with e.g.:\n +\n + ./t7900-maintenance.sh -vixd --run=23 --valgrind\n + [...]\n + + git maintenance unregister --force\n + Conditional jump or move depends on uninitialised value(s)\n + at 0x6B5F1E: git_configset_clear (config.c:2367)\n + by 0x4BA64E: maintenance_unregister (gc.c:1619)\n + by 0x4BD278: cmd_maintenance (gc.c:2650)\n + by 0x409905: run_builtin (git.c:466)\n + by 0x40A21C: handle_builtin (git.c:721)\n + by 0x40A58E: run_argv (git.c:788)\n + by 0x40AF68: cmd_main (git.c:926)\n + by 0x5D39FE: main (common-main.c:57)\n + Uninitialised value was created by a stack allocation\n + at 0x4BA22C: maintenance_unregister (gc.c:1557)\n +\n + Let's fix both of these issues, and also move the scope of the\n + variable to the \"if\" statement it's used in, to make it obvious where\n + it's used.\n +\n + Helped-by: Johannes Schindelin \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## builtin/gc.c ##\n -@@ builtin/gc.c: static int maintenance_unregister(int argc, const char **argv, const char *prefi\n - \tint found = 0;\n - \tstruct string_list_item *item;\n - \tconst struct string_list *list;\n -+\tstruct config_set cs;\n - \n - \targc = parse_options(argc, argv, prefix, options,\n - \t\t\t builtin_maintenance_unregister_usage, 0);\n @@ builtin/gc.c: static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tusage_with_options(builtin_maintenance_unregister_usage,\n \t\t\t\t options);\n \n -\tstruct config_set cs;\n \tif (config_file) {\n ++\t\tstruct config_set cs;\n ++\n \t\tgit_configset_init(&cs);\n \t\tgit_configset_add_file(&cs, config_file);\n + \t\tlist = git_configset_get_value_multi(&cs, key);\n ++\t\tgit_configset_clear(&cs);\n + \t} else {\n + \t\tlist = git_config_get_value_multi(key);\n + \t}\n +@@ builtin/gc.c: static int maintenance_unregister(int argc, const char **argv, const char *prefi\n + \t\tdie(_(\"repository '%s' is not registered\"), maintpath);\n + \t}\n + \n +-\tgit_configset_clear(&cs);\n + \tfree(maintpath);\n + \treturn 0;\n + }\n\n builtin/gc.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 56b107e7f0b..d87cf84041f 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1550,11 +1550,13 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tusage_with_options(builtin_maintenance_unregister_usage,\n \t\t\t\t options);\n \n-\tstruct config_set cs;\n \tif (config_file) {\n+\t\tstruct config_set cs;\n+\n \t\tgit_configset_init(&cs);\n \t\tgit_configset_add_file(&cs, config_file);\n \t\tlist = git_configset_get_value_multi(&cs, key);\n+\t\tgit_configset_clear(&cs);\n \t} else {\n \t\tlist = git_config_get_value_multi(key);\n \t}\n@@ -1590,7 +1592,6 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n \t\tdie(_(\"repository '%s' is not registered\"), maintpath);\n \t}\n \n-\tgit_configset_clear(&cs);\n \tfree(maintpath);\n \treturn 0;\n }\n-- \n2.38.0.1473.g172bcc0511c\n\n"},{"id":"467309","messageId":"221115.86v8ngjj72.gmgdl@evledraar.gmail.com","threadId":"58800","inReplyTo":"r31o6p7q-po67-osr1-3qq8-93s4p3o23nq2@tzk.qr","subject":"Re: [PATCH] builtin/gc.c: fix -Wdeclaration-after-statement","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-15T16:05:32Z","receivedAt":"2022-11-15T16:06:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 15 2022, Johannes Schindelin wrote:\n\n> Hi Ævar,\n>\n> On Tue, 15 Nov 2022, Johannes Schindelin wrote:\n>\n>> On Tue, 15 Nov 2022, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> > In 1f80129d61b (maintenance: add option to register in a specific\n>> > config, 2022-11-09) code was added which triggers a\n>> > \"-Wdeclaration-after-statement\" warning, which is on by default with\n>> > DEVELOPER=1.\n>> >\n>> > Signed-off-by: Ævar Arnfjörð Bjarmason \n>> > ---\n>> >\n>> > This landed in the latest \"next\" push-out, causing e.g this CI\n>> > failure:\n>> > https://github.com/git/git/actions/runs/3467538041/jobs/5792504315\n>>\n>> I looked at the same thing all morning, and found that\n>> https://lore.kernel.org/git/20221111231910.26769-1-ronan@rjp.ie/\n>> _probably_ was designed to fix the same issue (and I think you agree that\n>> that patch should be used instead of yours because it is more complete in\n>> fixing left-over issues).\n>>\n>> However, try as I might, I did not find out yet why it does not apply\n>> cleanly over here (I got side-tracked into range-diff'ing patches\n>> downloaded from the archive, which is quite the challenge and not even fun\n>> because `range-diff` requires commits, not emails, so I side-tracked\n>> myself into teaching `range-diff` to accept mbox arguments).\n>>\n>> Maybe you can adjust that patch so it applies cleanly?\n>\n> Seems that the v2 of \"maintenance: improve error reporting for\n> unregister\" [*1*] was _partially_ folded into v3 of \"maintenance: add\n> option to register in a specific config\": At least the config change that\n> talks about `--config-file` inste of `--config` _is_ part of v3, and one\n> reason that the former patch does not apply cleanly on top of the latter\n> patch. Which is a bit funny because the former patch was sent out two days\n> _after_ the latter patch.\n>\n> And it looks as if there is one more thing that needs to be forward-ported\n> from that no-longer-applying patch: the initialization of the configset,\n> otherwise we're potentially trying to clear an uninitialized data\n> structure.\n>\n> This is the patch I am currently testing (on top of Git for Windows'\n> `shears/seen` branch:\n> https://github.com/git-for-windows/git/commit/cd7b86d19f):\n>\n> -- snip --\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 635f12499d68..a3f63880dfba 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -1569,6 +1569,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n> \tint found = 0;\n> \tstruct string_list_item *item;\n> \tconst struct string_list *list;\n> +\tstruct config_set cs;\n>\n> \targc = parse_options(argc, argv, prefix, options,\n> \t\t\t builtin_maintenance_unregister_usage, 0);\n> @@ -1576,9 +1577,8 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi\n> \t\tusage_with_options(builtin_maintenance_unregister_usage,\n> \t\t\t\t options);\n>\n> -\tstruct config_set cs;\n> +\tgit_configset_init(&cs);\n> \tif (config_file) {\n> -\t\tgit_configset_init(&cs);\n> \t\tgit_configset_add_file(&cs, config_file);\n> \t\tlist = git_configset_get_value_multi(&cs, key);\n> \t} else {\n> -- snap --\n>\n> Ævar, can you please have a thorough look and see whether there is\n> anything else we were missing before advancing this to `next`?\n\nThanks, I submitted a v2 just now that addresses the unit'd issue as\nwell, and as we're fixing that we can just reduce the scope of the\nvariable, which makes it lifetime more obvious:\n\nhttps://lore.kernel.org/git/patch-v2-1.1-f37e99c9d59-20221115T160240Z-avarab@gmail.com/\n"},{"id":"467311","messageId":"1ab6aa0e8178c88689f26b8df9ec75cb@rjp.ie","threadId":"58800","inReplyTo":"patch-v2-1.1-f37e99c9d59-20221115T160240Z-avarab@gmail.com","subject":"Re: [PATCH v2] maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement","fromName":"","fromEmail":"ronan@rjp.ie","sentAt":"2022-11-15T16:32:03Z","receivedAt":"2022-11-15T16:32:14Z","isPatch":true,"sender":{"key":"ronan@rjp.ie","avatar":"https://avatars.githubusercontent.com/u/12577529?v=4"},"body":"November 15, 2022 9:04 AM, \"Ævar Arnfjörð Bjarmason\" wrote:\n\n> Since (maintenance: add option to register in a specific config,\n> 2022-11-09) we've been unable to build with \"DEVELOPER=1\" without\n> \"DEVOPTS=no-error\", as the added code triggers a\n> \"-Wdeclaration-after-statement\" warning.\n> \n> And worse than that, the data handed to git_configset_clear() is\n> uninitialized, as can be spotted with e.g.:\n\nSorry, I'm a little confused. I had sent v1, v2, and v3 of the \"maintenance: add option [...]\" patch, but as I understand it v2 had already been applied so I was asked to resubmit the changes from v3 rebased on next.\n\nI had done that in [1], but these issues were caught in review so I submitted a v2 of that correction in [2] which declares the configsset earlier and unconditionally initializes it is cleared. Are these further issues discovered after [2] was applied, or was there some issue rebasing the patches?\n\n[1] https://lore.kernel.org/git/20221110225310.7488-1-ronan@rjp.ie/\n[2] https://lore.kernel.org/git/20221111231910.26769-1-ronan@rjp.ie/\n"},{"id":"467319","messageId":"Y3PNolwofKgbkLh5@nand.local","threadId":"58800","inReplyTo":"patch-v2-1.1-f37e99c9d59-20221115T160240Z-avarab@gmail.com","subject":"Re: [PATCH v2] maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-15T17:34:26Z","receivedAt":"2022-11-15T17:34:36Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 15, 2022 at 05:04:27PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> Since (maintenance: add option to register in a specific config,\n> 2022-11-09) we've been unable to build with \"DEVELOPER=1\" without\n> \"DEVOPTS=no-error\", as the added code triggers a\n> \"-Wdeclaration-after-statement\" warning.\n>\n> And worse than that, the data handed to git_configset_clear() is\n> uninitialized, as can be spotted with e.g.:\n>\n> \t./t7900-maintenance.sh -vixd --run=23 --valgrind\n> \t[...]\n> \t+ git maintenance unregister --force\n> \tConditional jump or move depends on uninitialised value(s)\n> \t at 0x6B5F1E: git_configset_clear (config.c:2367)\n> \t by 0x4BA64E: maintenance_unregister (gc.c:1619)\n> \t by 0x4BD278: cmd_maintenance (gc.c:2650)\n> \t by 0x409905: run_builtin (git.c:466)\n> \t by 0x40A21C: handle_builtin (git.c:721)\n> \t by 0x40A58E: run_argv (git.c:788)\n> \t by 0x40AF68: cmd_main (git.c:926)\n> \t by 0x5D39FE: main (common-main.c:57)\n> \t Uninitialised value was created by a stack allocation\n> \t at 0x4BA22C: maintenance_unregister (gc.c:1557)\n>\n> Let's fix both of these issues, and also move the scope of the\n> variable to the \"if\" statement it's used in, to make it obvious where\n> it's used.\n>\n> Helped-by: Johannes Schindelin \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n\nThanks, both. I could have sworn I picked up the right version of this\ntopic when queuing, but apparently not.\n\nI pushed this out to 'next' and rebuilt jch and seen based on the new\nversion. The result now should be OK. Thanks again for noticing and\nworking together.\n\nThanks,\nTaylor\n"},{"id":"467320","messageId":"Y3PN4K1ULrvGCEEE@nand.local","threadId":"58800","inReplyTo":"1ab6aa0e8178c88689f26b8df9ec75cb@rjp.ie","subject":"Re: [PATCH v2] maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-15T17:35:28Z","receivedAt":"2022-11-15T17:35:36Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 15, 2022 at 04:32:03PM +0000, ronan@rjp.ie wrote:\n> I had done that in [1], but these issues were caught in review so I\n> submitted a v2 of that correction in [2] which declares the configsset\n> earlier and unconditionally initializes it is cleared. Are these\n> further issues discovered after [2] was applied, or was there some\n> issue rebasing the patches?\n>\n> [1] https://lore.kernel.org/git/20221110225310.7488-1-ronan@rjp.ie/\n> [2] https://lore.kernel.org/git/20221111231910.26769-1-ronan@rjp.ie/\n\nIt was human error on my part. Please let me know if the result in\n'next' now is OK to you.\n\nThanks,\nTaylor\n"},{"id":"467322","messageId":"d66fad29eb4c80fd1e9db4cfe5077085@rjp.ie","threadId":"58800","inReplyTo":"Y3PN4K1ULrvGCEEE@nand.local","subject":"Re: [PATCH v2] maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement","fromName":"","fromEmail":"ronan@rjp.ie","sentAt":"2022-11-15T18:00:45Z","receivedAt":"2022-11-15T18:01:43Z","isPatch":true,"sender":{"key":"ronan@rjp.ie","avatar":"https://avatars.githubusercontent.com/u/12577529?v=4"},"body":"November 15, 2022 10:35 AM, \"Taylor Blau\" wrote:\n\n> It was human error on my part. Please let me know if the result in\n> 'next' now is OK to you.\n\nHad a look and I'm just wondering if we're sure it's alright to clear\nthe configset before using the list we found in it. Doesn't that list\npoint into the configset?\n"},{"id":"467330","messageId":"Y3PgdXp30kLA15E+@nand.local","threadId":"58800","inReplyTo":"d66fad29eb4c80fd1e9db4cfe5077085@rjp.ie","subject":"Re: [PATCH v2] maintenance --unregister: fix uninit'd data use & -Wdeclaration-after-statement","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-15T18:54:45Z","receivedAt":"2022-11-15T18:54:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 15, 2022 at 06:00:45PM +0000, ronan@rjp.ie wrote:\n> November 15, 2022 10:35 AM, \"Taylor Blau\" wrote:\n>\n> > It was human error on my part. Please let me know if the result in\n> > 'next' now is OK to you.\n>\n> Had a look and I'm just wondering if we're sure it's alright to clear\n> the configset before using the list we found in it. Doesn't that list\n> point into the configset?\n\nIt does. I should not have trusted the incoming patch so blindly. See:\n\n https://lore.kernel.org/git/2cbead254b77cb02d219bca8f628dc4362c045b0.1668538355.git.me@ttaylorr.com/\n\nfor a fixup on top.\n\nThanks,\nTaylor\n"}]}