{"thread":{"id":"28365","subject":"[PATCH 2/3] Fix some \"variable might be used uninitialized\" warnings","startedAt":"2011-09-11T19:39:32Z","lastAt":"2011-10-10T23:59:53Z","messageCount":5,"participants":["Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"175308","messageId":"4E6D0E74.1020801@ramsay1.demon.co.uk","threadId":"28365","inReplyTo":null,"subject":"[PATCH 2/3] Fix some \"variable might be used uninitialized\" warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-09-11T19:39:32Z","receivedAt":"2011-09-11T19:39:32Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nIn particular, gcc complains as follows:\n\n        CC tree-walk.o\n    tree-walk.c: In function `traverse_trees':\n    tree-walk.c:347: warning: 'e' might be used uninitialized in this \\\n        function\n\n        CC builtin/revert.o\n    builtin/revert.c: In function `verify_opt_mutually_compatible':\n    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \\\n        this function\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n builtin/revert.c |    2 +-\n tree-walk.c      |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 5f1cee8..30538a1 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n \n static void verify_opt_mutually_compatible(const char *me, ...)\n {\n-\tconst char *opt1, *opt2;\n+\tconst char *opt1, *opt2 = NULL;\n \tva_list ap;\n \n \tva_start(ap, me);\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 808bb55..a8d8a66 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)\n \t\tunsigned long mask, dirmask;\n \t\tconst char *first = NULL;\n \t\tint first_len = 0;\n-\t\tstruct name_entry *e;\n+\t\tstruct name_entry *e = NULL;\n \t\tint len;\n \n \t\tfor (i = 0; i < n; i++) {\n-- \n1.7.6\n"},{"id":"175313","messageId":"7vpqj6olfa.fsf@alter.siamese.dyndns.org","threadId":"28365","inReplyTo":"4E6D0E74.1020801@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/3] Fix some \"variable might be used uninitialized\" warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-11T20:48:09Z","receivedAt":"2011-09-11T20:48:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n> In particular, gcc complains as follows:\n>\n>         CC tree-walk.o\n>     tree-walk.c: In function `traverse_trees':\n>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \\\n>         function\n>\n>         CC builtin/revert.o\n>     builtin/revert.c: In function `verify_opt_mutually_compatible':\n>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \\\n>         this function\n\nCould you also add something to this effect to the commit log message:\n\n\tbut I have verified that these are gcc being not careful\n\tenough and they are never used uninitialized.\n\nIf that is what you indeed have done, that is.\n\nThanks.\n"},{"id":"175436","messageId":"4E6FDBA4.6050505@ramsay1.demon.co.uk","threadId":"28365","inReplyTo":"7vpqj6olfa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] Fix some \"variable might be used uninitialized\" warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-09-13T22:39:32Z","receivedAt":"2011-09-13T22:39:32Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n> \n>> In particular, gcc complains as follows:\n>>\n>>         CC tree-walk.o\n>>     tree-walk.c: In function `traverse_trees':\n>>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \\\n>>         function\n>>\n>>         CC builtin/revert.o\n>>     builtin/revert.c: In function `verify_opt_mutually_compatible':\n>>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \\\n>>         this function\n> \n> Could you also add something to this effect to the commit log message:\n> \n> \tbut I have verified that these are gcc being not careful\n> \tenough and they are never used uninitialized.\n\nsee below for the v2 patch.\n\n> If that is what you indeed have done, that is.\n\nIndeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c\nwarning is somewhat less so! ;-)\n\nImagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore\nthe effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).\nAt first blush it looked like 'e' would remain uninitialized in the call to\nprune_traversal() at line 403.  Indeed it *would* be if you ever got to that line.\nHowever, since the 'mask' variable (set at line 391) remains set to zero at line 401,\nthe flow of control leaves the loop before 'e' is used.\n\n[I don't think traverse_trees() would ever be called with n == 0 anyway; the call\nsite in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)\nwhich start from unpack_trees() are protected by \"if (len)\", where 'len' is unsigned.]\n\nATB,\nRamsay Jones\n\n-- >8 --\nSubject: [PATCH v2 2/3] Fix some \"variable might be used uninitialized\" warnings\n\nIn particular, gcc complains as follows:\n\n        CC tree-walk.o\n    tree-walk.c: In function `traverse_trees':\n    tree-walk.c:347: warning: 'e' might be used uninitialized in this \\\n        function\n\n        CC builtin/revert.o\n    builtin/revert.c: In function `verify_opt_mutually_compatible':\n    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \\\n        this function\n\nHowever, I have verified that the analysis performed by gcc was too\nconservative and that these variables are not, in fact, used while\nuninitialized.\n\nIn order to suppress the warnings, we add an NULL pointer initializer\nto the declarations of the 'e' and 'opt2' variables.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n builtin/revert.c |    2 +-\n tree-walk.c      |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex ba27cf1..200149e 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n \n static void verify_opt_mutually_compatible(const char *me, ...)\n {\n-\tconst char *opt1, *opt2;\n+\tconst char *opt1, *opt2 = NULL;\n \tva_list ap;\n \n \tva_start(ap, me);\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 808bb55..a8d8a66 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)\n \t\tunsigned long mask, dirmask;\n \t\tconst char *first = NULL;\n \t\tint first_len = 0;\n-\t\tstruct name_entry *e;\n+\t\tstruct name_entry *e = NULL;\n \t\tint len;\n \n \t\tfor (i = 0; i < n; i++) {\n-- \n1.7.6\n"},{"id":"177230","messageId":"4E90751C.4030409@ramsay1.demon.co.uk","threadId":"28365","inReplyTo":"4E6FDBA4.6050505@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/3] Fix some \"variable might be used uninitialized\" warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-10-08T16:06:52Z","receivedAt":"2011-10-08T16:06:52Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Ramsay Jones wrote:\n> Junio C Hamano wrote:\n>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n>>\n>>> In particular, gcc complains as follows:\n>>>\n>>>         CC tree-walk.o\n>>>     tree-walk.c: In function `traverse_trees':\n>>>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \\\n>>>         function\n>>>\n>>>         CC builtin/revert.o\n>>>     builtin/revert.c: In function `verify_opt_mutually_compatible':\n>>>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \\\n>>>         this function\n>> Could you also add something to this effect to the commit log message:\n>>\n>> \tbut I have verified that these are gcc being not careful\n>> \tenough and they are never used uninitialized.\n> \n> see below for the v2 patch.\n> \n>> If that is what you indeed have done, that is.\n> \n> Indeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c\n> warning is somewhat less so! ;-)\n> \n> Imagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore\n> the effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).\n> At first blush it looked like 'e' would remain uninitialized in the call to\n> prune_traversal() at line 403.  Indeed it *would* be if you ever got to that line.\n> However, since the 'mask' variable (set at line 391) remains set to zero at line 401,\n> the flow of control leaves the loop before 'e' is used.\n> \n> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call\n> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)\n> which start from unpack_trees() are protected by \"if (len)\", where 'len' is unsigned.]\n\nWhen patches don't even make it to pu I just assume you hate them so much that\nthere is not much chance of them being applied and simply forget about them.\nIn this case, since compiler warnings are a bugbear of mine, I'm hoping that\nyou just forgot about this one ... :-D  [if not, sorry for the noise].\n\nFor your convenience, I've attached the patch below (rebased against current master).\n\nATB,\nRamsay Jones\n\n-- >8 --\nSubject: [PATCH] Fix some \"variable might be used uninitialized\" warnings\n\nIn particular, gcc complains as follows:\n\n        CC tree-walk.o\n    tree-walk.c: In function `traverse_trees':\n    tree-walk.c:347: warning: 'e' might be used uninitialized in this \\\n        function\n\n        CC builtin/revert.o\n    builtin/revert.c: In function `verify_opt_mutually_compatible':\n    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \\\n        this function\n\nHowever, I have verified that the analysis performed by gcc was too\nconservative and that these variables are not, in fact, used while\nuninitialized.\n\nIn order to suppress the warnings, we add an NULL pointer initializer\nto the declarations of the 'e' and 'opt2' variables.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n builtin/revert.c |    2 +-\n tree-walk.c      |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex ba27cf1..200149e 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n \n static void verify_opt_mutually_compatible(const char *me, ...)\n {\n-\tconst char *opt1, *opt2;\n+\tconst char *opt1, *opt2 = NULL;\n \tva_list ap;\n \n \tva_start(ap, me);\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 808bb55..a8d8a66 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)\n \t\tunsigned long mask, dirmask;\n \t\tconst char *first = NULL;\n \t\tint first_len = 0;\n-\t\tstruct name_entry *e;\n+\t\tstruct name_entry *e = NULL;\n \t\tint len;\n \n \t\tfor (i = 0; i < n; i++) {\n-- \n1.7.7\n"},{"id":"177344","messageId":"7vty7gieiu.fsf@alter.siamese.dyndns.org","threadId":"28365","inReplyTo":"4E90751C.4030409@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/3] Fix some \"variable might be used uninitialized\" warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-10T23:59:53Z","receivedAt":"2011-10-10T23:59:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:\n\n>> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call\n>> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)\n>> which start from unpack_trees() are protected by \"if (len)\", where 'len' is unsigned.]\n>\n> When patches don't even make it to pu I just assume you hate them so much that\n> there is not much chance of them being applied and simply forget about them.\n> In this case, since compiler warnings are a bugbear of mine, I'm hoping that\n> you just forgot about this one ... :-D  [if not, sorry for the noise].\n\nThanks for a reminder.\n\nThe reason a patch may not hit 'pu', unless I or other people whose\njudgement I trust explicity say \"the approach taken by the patch is\nutterly wrong\" is either because (1) the discussion for or against the\ntopic is still going strong and there is little chance of it getting\nforgotten by everybody, (2) I do not see much discussion for or against\nthe topic, and I am indifferent, or (3) the patch was just lost in the\nnoise.\n\nSo a good default strategy for a series that do not hit 'pu' is to\nre-post. Such a perseverance was what took format-patch to hit Linus's\ntree in June-July 2005 timeframe---we wouldn't have the command today, had\nI given up back then ;-).\n"}]}