# [PATCH] Fix compiler warning by properly initialize failed_errno 6 messages from 2009-08-02 to 2009-08-04. Participants: David Soria Parra, Junio C Hamano, sn_, Johannes Sixt. Thread: https://gitlist.dev/t/20348 ## David Soria Parra, 2009-08-02 19:34 Subject: [PATCH] Fix compiler warning by properly initialize failed_errno Message-ID: <1249241675-77329-1-git-send-email-sn_@gmx.net> URL: https://gitlist.dev/e/1249241675-77329-1-git-send-email-sn_%40gmx.net ``` From: David Soria Parra Initilize failed_error in start_command to avoid compiler warnings Signed-off-by: David Soria Parra --- run-command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/run-command.c b/run-command.c index dc09433..510349b 100644 --- a/run-command.c +++ b/run-command.c @@ -19,7 +19,7 @@ int start_command(struct child_process *cmd) { int need_in, need_out, need_err; int fdin[2], fdout[2], fderr[2]; - int failed_errno; + int failed_errno = 0; /* * In case of errors we must keep the promise to close FDs -- 1.6.4.212.g4719.dirty ``` ## Junio C Hamano, 2009-08-04 06:07 Subject: Re: [PATCH] Fix compiler warning by properly initialize failed_errno Message-ID: <7vmy6g6rj1.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmy6g6rj1.fsf%40alter.siamese.dyndns.org In-Reply-To: <1249241675-77329-1-git-send-email-sn_@gmx.net> ``` David Soria Parra writes: > From: David Soria Parra > > Initilize failed_error in start_command to avoid compiler warnings > > Signed-off-by: David Soria Parra > --- > run-command.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/run-command.c b/run-command.c > index dc09433..510349b 100644 > --- a/run-command.c > +++ b/run-command.c > @@ -19,7 +19,7 @@ int start_command(struct child_process *cmd) > { > int need_in, need_out, need_err; > int fdin[2], fdout[2], fderr[2]; > - int failed_errno; > + int failed_errno = 0; > > /* > * In case of errors we must keep the promise to close FDs We would want to be able to distinguish between a workaround for a compiler that is not clever/careful enough, and a necessary initialization. In this particular case, it is the former, and we should say int failed_errno = failed_errno; instead. The potentially uninitialized use your compiler is worried about is inside if (cmd->pid < 0) after #ifdef/#else/#endif. (1) if not on MINGW32, we would have already assigned to failed_errno after fork() returns negative value to cmd->pid; (2) if on MINGW32, we would have assigned to failed_errno unconditionally after calling mingw_spawnvpe(). so its worry is unfounded. ``` ## sn_, 2009-08-04 09:27 Subject: Re: [PATCH] Fix compiler warning by properly initialize failed_errno Message-ID: <20090804092759.24120@gmx.net> URL: https://gitlist.dev/e/20090804092759.24120%40gmx.net In-Reply-To: <7vmy6g6rj1.fsf@alter.siamese.dyndns.org> ``` > The potentially uninitialized use your compiler is worried about is inside > if (cmd->pid < 0) after #ifdef/#else/#endif. > > (1) if not on MINGW32, we would have already assigned to failed_errno > after fork() returns negative value to cmd->pid; > > (2) if on MINGW32, we would have assigned to failed_errno unconditionally > after calling mingw_spawnvpe(). > > so its worry is unfounded. The worry is definatly unfounded, but I think it's still worth to apply the attached patch to get rid of the warning using the i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5490) compiler. I sended a corrected version of the patch to the ml. -- Jetzt kostenlos herunterladen: Internet Explorer 8 und Mozilla Firefox 3 - sicherer, schneller und einfacher! http://portal.gmx.net/de/go/atbrowser ``` ## Johannes Sixt, 2009-08-04 18:51 Subject: Re: [PATCH] Fix compiler warning by properly initialize failed_errno Message-ID: <4A78834C.20002@kdbg.org> URL: https://gitlist.dev/e/4A78834C.20002%40kdbg.org In-Reply-To: <7vmy6g6rj1.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano schrieb: > David Soria Parra writes: > >> From: David Soria Parra >> >> Initilize failed_error in start_command to avoid compiler warnings >> >> Signed-off-by: David Soria Parra >> --- >> run-command.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/run-command.c b/run-command.c >> index dc09433..510349b 100644 >> --- a/run-command.c >> +++ b/run-command.c >> @@ -19,7 +19,7 @@ int start_command(struct child_process *cmd) >> { >> int need_in, need_out, need_err; >> int fdin[2], fdout[2], fderr[2]; >> - int failed_errno; >> + int failed_errno = 0; >> >> /* >> * In case of errors we must keep the promise to close FDs > > We would want to be able to distinguish between a workaround for a > compiler that is not clever/careful enough, and a necessary > initialization. In this particular case, it is the former, and we should > say > > int failed_errno = failed_errno; > > instead. Frankly, I prefer the initialization with 0; this is not a performance critical place and micro-optimization is not appropriate here. (If this were C++ then I *know* that int x = x; is undefined behavior, strictly speaking; I don't know whether it is the same with C.) Nevertheless, for both versions: Acked-by: Johannes Sixt -- Hannes ``` ## Junio C Hamano, 2009-08-04 19:09 Subject: Re: [PATCH] Fix compiler warning by properly initialize failed_errno Message-ID: <7v3a87o0pe.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v3a87o0pe.fsf%40alter.siamese.dyndns.org In-Reply-To: <4A78834C.20002@kdbg.org> ``` Johannes Sixt writes: > Junio C Hamano schrieb: > >> We would want to be able to distinguish between a workaround for a >> compiler that is not clever/careful enough, and a necessary >> initialization. In this particular case, it is the former, and we should >> say >> >> int failed_errno = failed_errno; >> >> instead. > > Frankly, I prefer the initialization with 0; this is not a performance > critical place and micro-optimization is not appropriate here. It is not about optimization at all. This is about documenting the fact that we have audited and know that the use of this variable in the code that follows is Ok. Initializing to 0 gives a false impression that the code may rely on that value, but in this case nobody will ever read that zero before overwriting it with an assignment. The compiler may optimize this out, but that is an insignificant (I agree this is not a performance critical codepath) side effect. ``` ## Junio C Hamano, 2009-08-04 22:22 Subject: Re: [PATCH] Fix compiler warning by properly initialize failed_errno Message-ID: <7vfxc7dxsf.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfxc7dxsf.fsf%40alter.siamese.dyndns.org In-Reply-To: <20090804092759.24120@gmx.net> ``` "sn_" writes: >> The potentially uninitialized use your compiler is worried about is inside >> if (cmd->pid < 0) after #ifdef/#else/#endif. >> >> (1) if not on MINGW32, we would have already assigned to failed_errno >> after fork() returns negative value to cmd->pid; >> >> (2) if on MINGW32, we would have assigned to failed_errno unconditionally >> after calling mingw_spawnvpe(). >> >> so its worry is unfounded. > > The worry is definatly unfounded, but I think it's still worth to apply > the attached patch to get rid of the warning using the > i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5490) > compiler. I sended a corrected version of the patch to the ml. Oh, there was no need for you to say "but..." and everything that followed. I said "we should say ... instead" in my review comments, didn't I? We are obviously in agreement ;-) ```