From: Erik Faye-Lund <kusmabite@googlemail.com>
To: Johannes Sixt <j.sixt@viscovery.net>
Cc: Frank Li <lznuaa@gmail.com>, git@vger.kernel.org
Subject: Re: possible code error at run_command.c
Date: Mon, 21 Dec 2009 10:08:49 +0100 [thread overview]
Message-ID: <40aa078e0912210108y4ea67dbfs2025bae20bdfb381@mail.gmail.com> (raw)
In-Reply-To: <4B2F3732.6030903@viscovery.net>
On Mon, Dec 21, 2009 at 9:52 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Erik Faye-Lund schrieb:
>> On Mon, Dec 21, 2009 at 9:30 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Frank Li schrieb:
>>>> Maybe some excute path miss initialized it. Otherwise compiler will
>>>> not report warning.
>>> LOOK AT THE CODE. There is no such code path.
>>>
>>
>> That's odd.
>
> Only if Frank did the homework and removed the initialization from
>
> int failed_errno = failed_errno;
>
>> I agree, there isn't such a code path. But this is the
>> first time I've ever seen this MSVC-feature turn up false positives,
>> which puzzles me.
>
> This line will trigger the check. It initializes failed_errno with itself,
> which is still uninitialized at this time.
>
> Note that we have more definitions of this kind:
>
> $ git grep -E ' ([a-zA-Z_][a-zA-Z_0-9]*) = \1[;,]' *.c
> builtin-rev-list.c: int reaches = reaches, all = all;
> match-trees.c: unsigned mode1 = mode1;
> match-trees.c: unsigned mode2 = mode2;
> run-command.c: int failed_errno = failed_errno;
> transport.c: int cmp = cmp, len;
> wt-status.c: int status = status;
>
> -- Hannes
>
>
Right, I didn't think of that. Since that is the case, I'd say
disabling run-time checks is perfectly sane.
That being said, it might make sense for MSVC people to be able to
have this feature turned on so they can easier find REAL
programmer-errors. But I guess removing the self-assignments makes
trouble for non-MSVC people.
Perhaps some of these warnings could be avoided in a "safer" way? IMO,
assigning variables to themselves like this is a bit fishy, as it
effectively hides uninitialized-use warnings for the entire lifetime
of that variable. Sure, the programmer who did that probably knows
what he or she does -- but they can't possibly know what people will
do in the future. If we could somehow turn these (really few)
occasions into something that produces reliable warnings on
uninitialized variables, I think we might be able to catch some bugs
earlier.
--
Erik "kusma" Faye-Lund
next prev parent reply other threads:[~2009-12-21 9:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-21 6:46 possible code error at run_command.c Frank Li
2009-12-21 7:09 ` Wincent Colaiuta
2009-12-21 7:18 ` Johannes Sixt
2009-12-21 7:29 ` Frank Li
2009-12-21 7:43 ` Johannes Sixt
2009-12-21 8:18 ` Frank Li
2009-12-21 8:30 ` Johannes Sixt
2009-12-21 8:41 ` Erik Faye-Lund
2009-12-21 8:52 ` Johannes Sixt
2009-12-21 9:08 ` Erik Faye-Lund [this message]
2009-12-21 9:13 ` Frank Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40aa078e0912210108y4ea67dbfs2025bae20bdfb381@mail.gmail.com \
--to=kusmabite@googlemail.com \
--cc=git@vger.kernel.org \
--cc=j.sixt@viscovery.net \
--cc=kusmabite@gmail.com \
--cc=lznuaa@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).