{"thread":{"id":"29189","subject":"[PATCH] Fix an \"variable might be used uninitialized\" gcc warning","startedAt":"2011-12-16T22:44:38Z","lastAt":"2012-02-02T18:25:29Z","messageCount":7,"participants":["Ramsay Jones","Jonathan Nieder","Andreas Schwab","Miles Bader"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"181348","messageId":"4EEBC9D6.6010204@ramsay1.demon.co.uk","threadId":"29189","inReplyTo":null,"subject":"[PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-12-16T22:44:38Z","receivedAt":"2011-12-16T22:44:38Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nIn particular, gcc issues the following warning:\n\n CC builtin/checkout.o\n builtin/checkout.c: In function `cmd_checkout':\n builtin/checkout.c:160: warning: 'mode' might be used uninitialized \\\n in this function\n\nHowever, the analysis performed by gcc is too conservative, in this\ncase, since the mode variable will not be used uninitialised. Note that,\nif the mode variable is not set in the loop, then \"threeway[1]\" will\nalso still be set to the null SHA1. This will then result in control\nleaving the function, almost directly after the loop, well before the\npotential use in the call to make_cache_entry().\n\nIn order to suppress the warning, we initialise the mode variable to\nzero in it's declaration.\n\nSigned-off-by: Ramsay Jones \n---\n\nJust in case you haven't found the time to apply your own patch!\n\n[Note that only 2 out of the 3 versions of gcc I use issues this\nwarning]\n\nATB,\nRamsay Jones\n\n builtin/checkout.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 787d468..f1984d9 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -157,7 +157,7 @@ static int checkout_merged(int pos, struct checkout *state)\n \tunsigned char sha1[20];\n \tmmbuffer_t result_buf;\n \tunsigned char threeway[3][20];\n-\tunsigned mode;\n+\tunsigned mode = 0;\n \n \tmemset(threeway, 0, sizeof(threeway));\n \twhile (pos < active_nr) {\n-- \n1.7.8\n"},{"id":"181354","messageId":"20111216235908.GA5858@elie.hsd1.il.comcast.net","threadId":"29189","inReplyTo":"4EEBC9D6.6010204@ramsay1.demon.co.uk","subject":"Re: [PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-16T23:59:08Z","receivedAt":"2011-12-16T23:59:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramsay Jones wrote:\n\n> CC builtin/checkout.o\n> builtin/checkout.c: In function `cmd_checkout':\n> builtin/checkout.c:160: warning: 'mode' might be used uninitialized \\\n> in this function\n[...]\n> [Note that only 2 out of the 3 versions of gcc I use issues this\n> warning]\n\nWhich version of gcc is that? Is gcc getting more sane, so we won't\nhave to worry about this after a while, or is the false positive a\nnew regression that should be reported to them?\n"},{"id":"181386","messageId":"m2iplffqgg.fsf@igel.home","threadId":"29189","inReplyTo":"20111216235908.GA5858@elie.hsd1.il.comcast.net","subject":"Re: [PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2011-12-17T10:22:07Z","receivedAt":"2011-12-17T10:22:07Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jonathan Nieder writes:\n\n> Ramsay Jones wrote:\n>\n>> CC builtin/checkout.o\n>> builtin/checkout.c: In function `cmd_checkout':\n>> builtin/checkout.c:160: warning: 'mode' might be used uninitialized \\\n>> in this function\n> [...]\n>> [Note that only 2 out of the 3 versions of gcc I use issues this\n>> warning]\n>\n> Which version of gcc is that? Is gcc getting more sane, so we won't\n> have to worry about this after a while, or is the false positive a\n> new regression that should be reported to them?\n\nThe regression is that the function has been changed in a way that makes\nit impossible to infer the intended flow.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"183423","messageId":"4F2834AD.20004@ramsay1.demon.co.uk","threadId":"29189","inReplyTo":"20111216235908.GA5858@elie.hsd1.il.comcast.net","subject":"Re: [PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-01-31T18:36:29Z","receivedAt":"2012-01-31T18:36:29Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n> Ramsay Jones wrote:\n> \n>> CC builtin/checkout.o\n>> builtin/checkout.c: In function `cmd_checkout':\n>> builtin/checkout.c:160: warning: 'mode' might be used uninitialized \\\n>> in this function\n> [...]\n>> [Note that only 2 out of the 3 versions of gcc I use issues this\n>> warning]\n> \n> Which version of gcc is that? Is gcc getting more sane, so we won't\n> have to worry about this after a while, or is the false positive a\n> new regression that should be reported to them?\n\n[Sorry for the late reply, I've been away from email for several weeks...]\n\nThe versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles\nthe code without complaint. So, gcc *may* be getting more sane, but I wouldn't\nbet on it! :-P\n\nI've had examples of this kind of warning, which relies heavily on the\nanalysis performed primarily for the optimizer, come-and-go in gcc before; so\ndon't hold your breath (this is the most volatile part of the compiler).\n\nHaving said that, unless you are going to decree that the project only\nsupports gcc (and presumably only some particular versions of gcc), then you\nmay well find similar warnings triggered when using other compilers anyway ...\n\nATB,\nRamsay Jones\n"},{"id":"183428","messageId":"20120131194302.GD12443@burratino","threadId":"29189","inReplyTo":"4F2834AD.20004@ramsay1.demon.co.uk","subject":"Re: [PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-01-31T19:43:02Z","receivedAt":"2012-01-31T19:43:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramsay Jones wrote:\n\n> The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles\n> the code without complaint. So, gcc *may* be getting more sane, but I wouldn't\n> bet on it! :-P\n>\n> I've had examples of this kind of warning, which relies heavily on the\n> analysis performed primarily for the optimizer, come-and-go in gcc before\n\nYep, judging from the commit message, Junio found the same warning\nin 4.6.2.\n\n[...]\n> Having said that, unless you are going to decree that the project only\n> supports gcc (and presumably only some particular versions of gcc), then you\n> may well find similar warnings triggered when using other compilers anyway ...\n\nSure, when the control flow grows too complicated, that's probably worth\nfixing anyway, for the sake of humans especially.\n\nSometimes gcc is the only crazy one, though. ;-)\n\nThanks for the update.\nJonathan\n"},{"id":"183460","messageId":"buo7h07rpl8.fsf@dhlpc061.dev.necel.com","threadId":"29189","inReplyTo":"20120131194302.GD12443@burratino","subject":"Re: [PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2012-02-01T07:16:03Z","receivedAt":"2012-02-01T07:16:03Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Jonathan Nieder writes:\n>> The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles\n>> the code without complaint. So, gcc *may* be getting more sane, but I wouldn't\n>> bet on it! :-P\n>>\n>> I've had examples of this kind of warning, which relies heavily on the\n>> analysis performed primarily for the optimizer, come-and-go in gcc before\n>\n> Yep, judging from the commit message, Junio found the same warning\n> in 4.6.2.\n>\n>> Having said that, unless you are going to decree that the project only\n>> supports gcc (and presumably only some particular versions of gcc), then you\n>> may well find similar warnings triggered when using other compilers anyway ...\n>\n> Sure, when the control flow grows too complicated, that's probably worth\n> fixing anyway, for the sake of humans especially.\n>\n> Sometimes gcc is the only crazy one, though. ;-)\n\nIt's hard to see how any compiler could detect that \"mode\" always\nreceives a value here .... it would have to realize that \"stage\" always\nbecomes 2 before the loop is exited, and that seems to depend on\nnon-trivial properties of external data structures...\n\n-miles\n\n-- \nJoy, n. An emotion variously excited, but in its highest degree arising from\nthe contemplation of grief in another.\n"},{"id":"183623","messageId":"4F2AD519.2010706@ramsay1.demon.co.uk","threadId":"29189","inReplyTo":"20120131194302.GD12443@burratino","subject":"Re: [PATCH] Fix an \"variable might be used uninitialized\" gcc warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-02-02T18:25:29Z","receivedAt":"2012-02-02T18:25:29Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n> Sure, when the control flow grows too complicated, that's probably worth\n> fixing anyway, for the sake of humans especially.\n> \n> Sometimes gcc is the only crazy one, though. ;-)\n\nIndeed. :-D\n\nATB,\nRamsay Jones\n"}]}