{"thread":{"id":"20348","subject":"[PATCH] Fix compiler warning by properly initialize failed_errno","startedAt":"2009-08-02T19:34:35Z","lastAt":"2009-08-04T22:22:40Z","messageCount":6,"participants":["David Soria Parra","Junio C Hamano","sn_","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"119385","messageId":"1249241675-77329-1-git-send-email-sn_@gmx.net","threadId":"20348","inReplyTo":null,"subject":"[PATCH] Fix compiler warning by properly initialize failed_errno","fromName":"David Soria Parra","fromEmail":"sn_@gmx.net","sentAt":"2009-08-02T19:34:35Z","receivedAt":"2009-08-02T19:34:35Z","isPatch":true,"sender":{"key":"sn_@gmx.net","avatar":"https://gravatar.com/avatar/b1075ecdd33ea094cbc23798fe8b95c73ec1ccf7bb213ac8260c719e2dd97b55?d=mp&s=160"},"body":"From: David Soria Parra <dsp@php.net>\n\nInitilize failed_error in start_command to avoid compiler warnings\n\nSigned-off-by: David Soria Parra <dsp@php.net>\n---\n run-command.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex dc09433..510349b 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -19,7 +19,7 @@ int start_command(struct child_process *cmd)\n {\n \tint need_in, need_out, need_err;\n \tint fdin[2], fdout[2], fderr[2];\n-\tint failed_errno;\n+\tint failed_errno = 0;\n \n \t/*\n \t * In case of errors we must keep the promise to close FDs\n-- \n1.6.4.212.g4719.dirty\n"},{"id":"119461","messageId":"7vmy6g6rj1.fsf@alter.siamese.dyndns.org","threadId":"20348","inReplyTo":"1249241675-77329-1-git-send-email-sn_@gmx.net","subject":"Re: [PATCH] Fix compiler warning by properly initialize failed_errno","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-04T06:07:30Z","receivedAt":"2009-08-04T06:07:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Soria Parra <sn_@gmx.net> writes:\n\n> From: David Soria Parra <dsp@php.net>\n>\n> Initilize failed_error in start_command to avoid compiler warnings\n>\n> Signed-off-by: David Soria Parra <dsp@php.net>\n> ---\n>  run-command.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/run-command.c b/run-command.c\n> index dc09433..510349b 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -19,7 +19,7 @@ int start_command(struct child_process *cmd)\n>  {\n>  \tint need_in, need_out, need_err;\n>  \tint fdin[2], fdout[2], fderr[2];\n> -\tint failed_errno;\n> +\tint failed_errno = 0;\n>  \n>  \t/*\n>  \t * In case of errors we must keep the promise to close FDs\n\nWe would want to be able to distinguish between a workaround for a\ncompiler that is not clever/careful enough, and a necessary\ninitialization.  In this particular case, it is the former, and we should\nsay\n\n\tint failed_errno = failed_errno;\n\ninstead.\n\nThe potentially uninitialized use your compiler is worried about is inside\nif (cmd->pid < 0) after #ifdef/#else/#endif.\n\n (1) if not on MINGW32, we would have already assigned to failed_errno\n     after fork() returns negative value to cmd->pid;\n\n (2) if on MINGW32, we would have assigned to failed_errno unconditionally\n     after calling mingw_spawnvpe().\n\nso its worry is unfounded.\n"},{"id":"119481","messageId":"20090804092759.24120@gmx.net","threadId":"20348","inReplyTo":"7vmy6g6rj1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix compiler warning by properly initialize failed_errno","fromName":"sn_","fromEmail":"sn_@gmx.net","sentAt":"2009-08-04T09:27:59Z","receivedAt":"2009-08-04T09:27:59Z","isPatch":true,"sender":{"key":"sn_@gmx.net","avatar":"https://gravatar.com/avatar/b1075ecdd33ea094cbc23798fe8b95c73ec1ccf7bb213ac8260c719e2dd97b55?d=mp&s=160"},"body":"\n> The potentially uninitialized use your compiler is worried about is inside\n> if (cmd->pid < 0) after #ifdef/#else/#endif.\n> \n>  (1) if not on MINGW32, we would have already assigned to failed_errno\n>      after fork() returns negative value to cmd->pid;\n> \n>  (2) if on MINGW32, we would have assigned to failed_errno unconditionally\n>      after calling mingw_spawnvpe().\n> \n> so its worry is unfounded.\n\nThe 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.\n\n-- \nJetzt kostenlos herunterladen: Internet Explorer 8 und Mozilla Firefox 3 -\nsicherer, schneller und einfacher! http://portal.gmx.net/de/go/atbrowser\n"},{"id":"119511","messageId":"4A78834C.20002@kdbg.org","threadId":"20348","inReplyTo":"7vmy6g6rj1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix compiler warning by properly initialize failed_errno","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-08-04T18:51:56Z","receivedAt":"2009-08-04T18:51:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> David Soria Parra <sn_@gmx.net> writes:\n> \n>> From: David Soria Parra <dsp@php.net>\n>>\n>> Initilize failed_error in start_command to avoid compiler warnings\n>>\n>> Signed-off-by: David Soria Parra <dsp@php.net>\n>> ---\n>>  run-command.c |    2 +-\n>>  1 files changed, 1 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/run-command.c b/run-command.c\n>> index dc09433..510349b 100644\n>> --- a/run-command.c\n>> +++ b/run-command.c\n>> @@ -19,7 +19,7 @@ int start_command(struct child_process *cmd)\n>>  {\n>>  \tint need_in, need_out, need_err;\n>>  \tint fdin[2], fdout[2], fderr[2];\n>> -\tint failed_errno;\n>> +\tint failed_errno = 0;\n>>  \n>>  \t/*\n>>  \t * In case of errors we must keep the promise to close FDs\n> \n> We would want to be able to distinguish between a workaround for a\n> compiler that is not clever/careful enough, and a necessary\n> initialization.  In this particular case, it is the former, and we should\n> say\n> \n> \tint failed_errno = failed_errno;\n> \n> instead.\n\nFrankly, I prefer the initialization with 0; this is not a performance \ncritical place and micro-optimization is not appropriate here.\n\n(If this were C++ then I *know* that int x = x; is undefined behavior, \nstrictly speaking; I don't know whether it is the same with C.)\n\nNevertheless, for both versions:\n\nAcked-by: Johannes Sixt <j6t@kdbg.org>\n\n-- Hannes\n"},{"id":"119512","messageId":"7v3a87o0pe.fsf@alter.siamese.dyndns.org","threadId":"20348","inReplyTo":"4A78834C.20002@kdbg.org","subject":"Re: [PATCH] Fix compiler warning by properly initialize failed_errno","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-04T19:09:33Z","receivedAt":"2009-08-04T19:09:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Junio C Hamano schrieb:\n>\n>> We would want to be able to distinguish between a workaround for a\n>> compiler that is not clever/careful enough, and a necessary\n>> initialization.  In this particular case, it is the former, and we should\n>> say\n>>\n>> \tint failed_errno = failed_errno;\n>>\n>> instead.\n>\n> Frankly, I prefer the initialization with 0; this is not a performance\n> critical place and micro-optimization is not appropriate here.\n\nIt is not about optimization at all.  This is about documenting the fact\nthat we have audited and know that the use of this variable in the code\nthat follows is Ok.  Initializing to 0 gives a false impression that the\ncode may rely on that value, but in this case nobody will ever read that\nzero before overwriting it with an assignment.\n\nThe compiler may optimize this out, but that is an insignificant (I agree\nthis is not a performance critical codepath) side effect.\n"},{"id":"119537","messageId":"7vfxc7dxsf.fsf@alter.siamese.dyndns.org","threadId":"20348","inReplyTo":"20090804092759.24120@gmx.net","subject":"Re: [PATCH] Fix compiler warning by properly initialize failed_errno","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-04T22:22:40Z","receivedAt":"2009-08-04T22:22:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"sn_\" <sn_@gmx.net> writes:\n\n>> The potentially uninitialized use your compiler is worried about is inside\n>> if (cmd->pid < 0) after #ifdef/#else/#endif.\n>> \n>>  (1) if not on MINGW32, we would have already assigned to failed_errno\n>>      after fork() returns negative value to cmd->pid;\n>> \n>>  (2) if on MINGW32, we would have assigned to failed_errno unconditionally\n>>      after calling mingw_spawnvpe().\n>> \n>> so its worry is unfounded.\n>\n> The worry is definatly unfounded, but I think it's still worth to apply\n> the attached patch to get rid of the warning using the\n> i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5490)\n> compiler. I sended a corrected version of the patch to the ml.\n\nOh, there was no need for you to say \"but...\" and everything that followed.\nI said \"we should say ... instead\" in my review comments, didn't I?  \n\nWe are obviously in agreement ;-)\n"}]}