{"thread":{"id":"21681","subject":"[PATCH] submodule.c: Squelch a \"use before assignment\" warning","startedAt":"2009-11-20T01:33:05Z","lastAt":"2009-11-22T03:32:42Z","messageCount":6,"participants":["David Aguilar","Junio C Hamano","Christoph Bartoschek"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127951","messageId":"1258680785-42235-1-git-send-email-davvid@gmail.com","threadId":"21681","inReplyTo":null,"subject":"[PATCH] submodule.c: Squelch a \"use before assignment\" warning","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-20T01:33:05Z","receivedAt":"2009-11-20T01:33:05Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler\n(and probably others) mistakenly thinks variable 'right' is used\nbefore assigned. Work it around by giving it a fake initialization.\n\nSigned-off-by: David Aguilar \n---\n submodule.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 461faf0..0145a62 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstruct rev_info rev;\n-\tstruct commit *commit, *left = left, *right;\n+\tstruct commit *commit, *left = left, *right = NULL;\n \tstruct commit_list *merge_bases, *list;\n \tconst char *message = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\n-- \n1.6.5.3.171.ge36e\n"},{"id":"127984","messageId":"7v8we17ha9.fsf@alter.siamese.dyndns.org","threadId":"21681","inReplyTo":"1258680785-42235-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] submodule.c: Squelch a \"use before assignment\" warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-20T08:05:18Z","receivedAt":"2009-11-20T08:05:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler\n> (and probably others) mistakenly thinks variable 'right' is used\n> before assigned. Work it around by giving it a fake initialization.\n\nWe see the same \"fake initialization\" of 'left' on the same line. By\ninitializing it to NULL, you are hinting that initializing 'right' to\nNULL actually means something.\n\n> diff --git a/submodule.c b/submodule.c\n> index 461faf0..0145a62 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,\n> \t\tconst char *del, const char *add, const char *reset)\n> {\n> \tstruct rev_info rev;\n> -\tstruct commit *commit, *left = left, *right;\n> +\tstruct commit *commit, *left = left, *right = NULL;\n"},{"id":"128008","messageId":"1258716833-49974-1-git-send-email-davvid@gmail.com","threadId":"21681","inReplyTo":"7v8we17ha9.fsf@alter.siamese.dyndns.org","subject":"[PATCH] submodule.c: Squelch a \"use before assignment\" warning","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-20T11:33:53Z","receivedAt":"2009-11-20T11:33:53Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler\n(and probably others) mistakenly thinks variable 'right' is used\nbefore assigned. Work around it by giving it a fake initialization.\n\nSigned-off-by: David Aguilar \n---\n submodule.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 461faf0..86aad65 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstruct rev_info rev;\n-\tstruct commit *commit, *left = left, *right;\n+\tstruct commit *commit, *left = left, *right = right;\n \tstruct commit_list *merge_bases, *list;\n \tconst char *message = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\n-- \n1.6.5.3.171.ge36e\n"},{"id":"128082","messageId":"c5plt6-5me.ln1@burns.bruehl.pontohonk.de","threadId":"21681","inReplyTo":"7v8we17ha9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] submodule.c: Squelch a \"use before assignment\" warning","fromName":"Christoph Bartoschek","fromEmail":"bartoschek@gmx.de","sentAt":"2009-11-21T18:46:36Z","receivedAt":"2009-11-21T18:46:36Z","isPatch":true,"sender":{"key":"bartoschek@gmx.de","avatar":null},"body":"Junio C Hamano wrote:\n\n> David Aguilar writes:\n> \n>> i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler\n>> (and probably others) mistakenly thinks variable 'right' is used\n>> before assigned. Work it around by giving it a fake initialization.\n> \n> We see the same \"fake initialization\" of 'left' on the same line. By\n> initializing it to NULL, you are hinting that initializing 'right' to\n> NULL actually means something.\n\nWhy is the compiler not complaining about the fake initalization? For \ninitialization a value is used that is not initialized.\n\nAt least a static analyzer complains: \n\n\"submodule.c\", line 41: The variable `left' is used before its \ninitialization \n\nChristoph\n"},{"id":"128091","messageId":"7vws1jl0xp.fsf@alter.siamese.dyndns.org","threadId":"21681","inReplyTo":"c5plt6-5me.ln1@burns.bruehl.pontohonk.de","subject":"Re: [PATCH] submodule.c: Squelch a \"use before assignment\" warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T02:59:14Z","receivedAt":"2009-11-22T02:59:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christoph Bartoschek writes:\n\n> Why is the compiler not complaining about the fake initalization? For \n> initialization a value is used that is not initialized.\n\nThat is a fairly well established idiom to tell gcc \"you may mistakenly\nthink it isn't, but this is used\".\n"},{"id":"128092","messageId":"7vhbsnkzdx.fsf@alter.siamese.dyndns.org","threadId":"21681","inReplyTo":"7vws1jl0xp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] submodule.c: Squelch a \"use before assignment\" warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T03:32:42Z","receivedAt":"2009-11-22T03:32:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Christoph Bartoschek writes:\n>\n>> Why is the compiler not complaining about the fake initalization? For \n>> initialization a value is used that is not initialized.\n>\n> That is a fairly well established idiom to tell gcc \"you may mistakenly\n> think it isn't, but this is used\".\n\nSorry, but I was called away from the keyboard in mid sentence. It should\nhave read \"...this is used after getting assigned, so do not worry\".\n\nBut I need to clarify a few things about this issue.\n\nAs the maintainer, I do not like a fake initialisation myself very much.\nIt is a declaration that \"left\" is always assigned before getting used by\nthe person who wrote _that particular version_, but later updates to the\ncode might introduce a codepath to incorrectly use \"left\" before getting\nassinged to, and the fake initialisation will prevent the compiler from\ncatching it.\n\nWhen the variable in question is a pointer, assigning NULL instead of\n\"left = left\" will at least give us a predictable and more reproducible\nbreakage when later updates break the code. I wouldn't have minded if\nDavid's patch were to update both the existing fake initialisation and the\nnew one to assign NULL. At least dereferencing such a pointer will give\nus a segfault reliably. But unfortunately there is no such \"magic\" values\nfor variables of other types (e.g. int) to help us catch uninitialized use\nof variables at runtime.\n\nBy the way, if a static analyser is meant to be useful for real-world\nprograms, as opposed to merely an academic exercise, it should know this\nconvention; like it or not, it is used fairly widely. That is, it should\ncheck \"left\" is assigned before used in the rest of the function without\nthis \"gcc hack\" initializer, and if the only questionable use of \"left\" is\nthe RHS of this fake initialisation, should refrain from warning.\n"}]}