# [PATCH] Fix an unitialized pointer in merge-recursive.c 7 messages from 2007-08-16 to 2007-08-16. Participants: Marco Costalba, Junio C Hamano, David Kastrup, Florian Weimer. Thread: https://gitlist.dev/t/9552 ## Marco Costalba, 2007-08-16 08:00 Subject: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: URL: https://gitlist.dev/e/e5bfff550708160100t5aa93430x6cbe83e6e7a61d73%40mail.gmail.com ``` Indeed &mrtree is passed to merge_trees() that not always seems to set the value, so on some paths mrtree could return uninitialized. Spotted by a gcc 4.2.1 warning Signed-off-by: Marco Costalba --- merge-recursive.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 16f6a0f..934b0d6 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1610,7 +1610,7 @@ static int merge(struct commit { struct commit_list *iter; struct commit *merged_common_ancestors; - struct tree *mrtree; + struct tree *mrtree = NULL; int clean; if (show(4)) { -- 1.5.3.rc4.67.gf9286 ``` ## Junio C Hamano, 2007-08-16 08:08 Subject: Re: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: <7vsl6jkila.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsl6jkila.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` "Marco Costalba" writes: > Indeed &mrtree is passed to merge_trees() that not always > seems to set the value, so on some paths mrtree could > return uninitialized. > > Spotted by a gcc 4.2.1 warning Are you sure that gcc is correctly seeing the codeflow? In merge(), mrtree is used only under index_only, and merge_trees() always sets *result under index_only. ``` ## Marco Costalba, 2007-08-16 08:11 Subject: Re: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: URL: https://gitlist.dev/e/e5bfff550708160111n66b5ea94k26cf2fecc06fd84c%40mail.gmail.com In-Reply-To: <7vsl6jkila.fsf@gitster.siamese.dyndns.org> ``` On 8/16/07, Junio C Hamano wrote: > "Marco Costalba" writes: > > > Indeed &mrtree is passed to merge_trees() that not always > > seems to set the value, so on some paths mrtree could > > return uninitialized. > > > > Spotted by a gcc 4.2.1 warning > > Are you sure that gcc is correctly seeing the codeflow? > > In merge(), mrtree is used only under index_only, and > merge_trees() always sets *result under index_only. > Ok ;-) Now two options: - discard the patch - change the title in 'silence a gcc bougus warning' ``` ## Junio C Hamano, 2007-08-16 08:23 Subject: Re: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: <7vodh7khuo.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vodh7khuo.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` "Marco Costalba" writes: > On 8/16/07, Junio C Hamano wrote: >> "Marco Costalba" writes: >> >> > Indeed &mrtree is passed to merge_trees() that not always >> > seems to set the value, so on some paths mrtree could >> > return uninitialized. >> > >> > Spotted by a gcc 4.2.1 warning >> >> Are you sure that gcc is correctly seeing the codeflow? >> >> In merge(), mrtree is used only under index_only, and >> merge_trees() always sets *result under index_only. >> > > Ok ;-) > > Now two options: > > - discard the patch > > - change the title in 'silence a gcc bougus warning' Third option. Change the assignment from "mrtree = NULL" to "mrtree = mtree". It is a standard idiom to work around stupid gcc warnings. ``` ## Junio C Hamano, 2007-08-16 08:30 Subject: Re: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: <7vk5rvkhjy.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vk5rvkhjy.fsf%40gitster.siamese.dyndns.org In-Reply-To: <7vodh7khuo.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano writes: > Third option. Change the assignment from "mrtree = NULL" to > "mrtree = mtree". It is a standard idiom to work around stupid > gcc warnings. Oops; it might be obvious but that's "mrtree = mrtree". ``` ## David Kastrup, 2007-08-16 09:07 Subject: Re: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: <86643f4zlf.fsf@lola.quinscape.zz> URL: https://gitlist.dev/e/86643f4zlf.fsf%40lola.quinscape.zz In-Reply-To: <7vodh7khuo.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano writes: > "Marco Costalba" writes: > >> On 8/16/07, Junio C Hamano wrote: >>> "Marco Costalba" writes: >>> >>> > Indeed &mrtree is passed to merge_trees() that not always >>> > seems to set the value, so on some paths mrtree could >>> > return uninitialized. >>> > >>> > Spotted by a gcc 4.2.1 warning >>> >>> Are you sure that gcc is correctly seeing the codeflow? >>> >>> In merge(), mrtree is used only under index_only, and >>> merge_trees() always sets *result under index_only. >> >> Ok ;-) >> >> Now two options: >> >> - discard the patch >> >> - change the title in 'silence a gcc bougus warning' > > Third option. Change the assignment from "mrtree = NULL" to > "mrtree = mtree". It is a standard idiom to work around stupid > gcc warnings. I think it is more efficient to write mrtree = NULL: For the computer, it makes a minuscule difference, and it can save programmers a bit of confusion and worrying times. Time that may be better spent improving things elsewhere. I don't think that we have a contest running for least redundancy in code layout, have we? Why then require the programmer to do a complete call trace analysis before he can feel comfortable about the code? And what if callers change at some point of time? -- David Kastrup ``` ## Florian Weimer, 2007-08-16 13:16 Subject: Re: [PATCH] Fix an unitialized pointer in merge-recursive.c Message-ID: <87ps1nwrfr.fsf@mid.deneb.enyo.de> URL: https://gitlist.dev/e/87ps1nwrfr.fsf%40mid.deneb.enyo.de In-Reply-To: <7vodh7khuo.fsf@gitster.siamese.dyndns.org> ``` * Junio C. Hamano: > Third option. Change the assignment from "mrtree = NULL" to > "mrtree = mtree". It is a standard idiom to work around stupid > gcc warnings. Is it? It's also undefined according to the C standard. In other words, the compiler may assume that this particular declaration is never reached. ```