{"thread":{"id":"9552","subject":"[PATCH] Fix an unitialized pointer in merge-recursive.c","startedAt":"2007-08-16T08:00:26Z","lastAt":"2007-08-16T13:16:08Z","messageCount":7,"participants":["Marco Costalba","Junio C Hamano","David Kastrup","Florian Weimer"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"50855","messageId":"e5bfff550708160100t5aa93430x6cbe83e6e7a61d73@mail.gmail.com","threadId":"9552","inReplyTo":null,"subject":"[PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-08-16T08:00:26Z","receivedAt":"2007-08-16T08:00:26Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"Indeed &mrtree is passed to merge_trees() that not always\nseems to set the value, so on some paths mrtree could\nreturn uninitialized.\n\nSpotted by a gcc 4.2.1 warning\n\nSigned-off-by: Marco Costalba \n---\n merge-recursive.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 16f6a0f..934b0d6 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1610,7 +1610,7 @@ static int merge(struct commit\n {\n \tstruct commit_list *iter;\n \tstruct commit *merged_common_ancestors;\n-\tstruct tree *mrtree;\n+\tstruct tree *mrtree = NULL;\n \tint clean;\n\n \tif (show(4)) {\n-- \n1.5.3.rc4.67.gf9286\n"},{"id":"50857","messageId":"7vsl6jkila.fsf@gitster.siamese.dyndns.org","threadId":"9552","inReplyTo":"e5bfff550708160100t5aa93430x6cbe83e6e7a61d73@mail.gmail.com","subject":"Re: [PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-16T08:08:01Z","receivedAt":"2007-08-16T08:08:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> Indeed &mrtree is passed to merge_trees() that not always\n> seems to set the value, so on some paths mrtree could\n> return uninitialized.\n>\n> Spotted by a gcc 4.2.1 warning\n\nAre you sure that gcc is correctly seeing the codeflow?\n\nIn merge(), mrtree is used only under index_only, and\nmerge_trees() always sets *result under index_only.\n"},{"id":"50858","messageId":"e5bfff550708160111n66b5ea94k26cf2fecc06fd84c@mail.gmail.com","threadId":"9552","inReplyTo":"7vsl6jkila.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-08-16T08:11:00Z","receivedAt":"2007-08-16T08:11:00Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 8/16/07, Junio C Hamano wrote:\n> \"Marco Costalba\" writes:\n>\n> > Indeed &mrtree is passed to merge_trees() that not always\n> > seems to set the value, so on some paths mrtree could\n> > return uninitialized.\n> >\n> > Spotted by a gcc 4.2.1 warning\n>\n> Are you sure that gcc is correctly seeing the codeflow?\n>\n> In merge(), mrtree is used only under index_only, and\n> merge_trees() always sets *result under index_only.\n>\n\nOk ;-)\n\nNow two options:\n\n- discard the patch\n\n- change the title in 'silence a gcc bougus warning'\n"},{"id":"50861","messageId":"7vodh7khuo.fsf@gitster.siamese.dyndns.org","threadId":"9552","inReplyTo":"e5bfff550708160111n66b5ea94k26cf2fecc06fd84c@mail.gmail.com","subject":"Re: [PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-16T08:23:59Z","receivedAt":"2007-08-16T08:23:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 8/16/07, Junio C Hamano wrote:\n>> \"Marco Costalba\" writes:\n>>\n>> > Indeed &mrtree is passed to merge_trees() that not always\n>> > seems to set the value, so on some paths mrtree could\n>> > return uninitialized.\n>> >\n>> > Spotted by a gcc 4.2.1 warning\n>>\n>> Are you sure that gcc is correctly seeing the codeflow?\n>>\n>> In merge(), mrtree is used only under index_only, and\n>> merge_trees() always sets *result under index_only.\n>>\n>\n> Ok ;-)\n>\n> Now two options:\n>\n> - discard the patch\n>\n> - change the title in 'silence a gcc bougus warning'\n\nThird option. Change the assignment from \"mrtree = NULL\" to\n\"mrtree = mtree\". It is a standard idiom to work around stupid\ngcc warnings.\n"},{"id":"50863","messageId":"7vk5rvkhjy.fsf@gitster.siamese.dyndns.org","threadId":"9552","inReplyTo":"7vodh7khuo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-16T08:30:25Z","receivedAt":"2007-08-16T08:30:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Third option. Change the assignment from \"mrtree = NULL\" to\n> \"mrtree = mtree\". It is a standard idiom to work around stupid\n> gcc warnings.\n\nOops; it might be obvious but that's \"mrtree = mrtree\".\n"},{"id":"50868","messageId":"86643f4zlf.fsf@lola.quinscape.zz","threadId":"9552","inReplyTo":"7vodh7khuo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-16T09:07:24Z","receivedAt":"2007-08-16T09:07:24Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> \"Marco Costalba\" writes:\n>\n>> On 8/16/07, Junio C Hamano wrote:\n>>> \"Marco Costalba\" writes:\n>>>\n>>> > Indeed &mrtree is passed to merge_trees() that not always\n>>> > seems to set the value, so on some paths mrtree could\n>>> > return uninitialized.\n>>> >\n>>> > Spotted by a gcc 4.2.1 warning\n>>>\n>>> Are you sure that gcc is correctly seeing the codeflow?\n>>>\n>>> In merge(), mrtree is used only under index_only, and\n>>> merge_trees() always sets *result under index_only.\n>>\n>> Ok ;-)\n>>\n>> Now two options:\n>>\n>> - discard the patch\n>>\n>> - change the title in 'silence a gcc bougus warning'\n>\n> Third option. Change the assignment from \"mrtree = NULL\" to\n> \"mrtree = mtree\". It is a standard idiom to work around stupid\n> gcc warnings.\n\nI think it is more efficient to write mrtree = NULL: For the computer,\nit makes a minuscule difference, and it can save programmers a bit of\nconfusion and worrying times. Time that may be better spent improving\nthings elsewhere.\n\nI don't think that we have a contest running for least redundancy in\ncode layout, have we? Why then require the programmer to do a\ncomplete call trace analysis before he can feel comfortable about the\ncode? And what if callers change at some point of time?\n\n-- \nDavid Kastrup\n"},{"id":"50885","messageId":"87ps1nwrfr.fsf@mid.deneb.enyo.de","threadId":"9552","inReplyTo":"7vodh7khuo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix an unitialized pointer in merge-recursive.c","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2007-08-16T13:16:08Z","receivedAt":"2007-08-16T13:16:08Z","isPatch":true,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Junio C. Hamano:\n\n> Third option. Change the assignment from \"mrtree = NULL\" to\n> \"mrtree = mtree\". It is a standard idiom to work around stupid\n> gcc warnings.\n\nIs it? It's also undefined according to the C standard. In other\nwords, the compiler may assume that this particular declaration is\nnever reached.\n"}]}