linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Borislav Petkov <bp@alien8.de>
Cc: Arnd Bergmann <arnd@arndb.de>, Ingo Molnar <mingo@kernel.org>,
	Michal Marek <mmarek@suse.com>, Sam Ravnborg <sam@ravnborg.org>,
	lkml <linux-kernel@vger.kernel.org>, Michael Matz <matz@suse.de>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	x86-ml <x86@kernel.org>
Subject: Re: [PATCH] Kbuild: Move -Wmaybe-uninitialized to W=1
Date: Fri, 29 Jul 2016 11:26:16 -0700	[thread overview]
Message-ID: <CA+55aFzuBLmHBz5zm4fWk_-1EnR9CkBKS-6h5ns1iBDEyVOp5Q@mail.gmail.com> (raw)
In-Reply-To: <20160729101932.GA27543@nazgul.tnic>

On Fri, Jul 29, 2016 at 3:19 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> So this is exactly the problem: we should not fix perfectly fine code
> just so that gcc remains quiet. So when you say "fixed false positives"
> you actually mean, "changed it so that gcc -Wmaybe-u... doesn't fire"
> right?
>
> And we should not do that.

It's perfectly fine to do that when it makes sense and doesn't make
the code worse. Adding a few unnecessary initializations to make the
compiler shut up is not a problem.

But in the cases I looked at, that *really* didn't make sense. The
pattern was along the lines of

  struct something var;

  if (initialize_var(&var) < 0)
     return error;

  .. use "var.xyz" .. - gcc complains that "var.xyz" may be uninitialized

and quite frankly,. the code made sense, and adding crazy
initializations for the fact that gcc has a shit-for-brains warning
didn't work well seemed to just make the code worse.

And there was no sane *pattern* to why some cases warned. We have
things like the above in many places. The issue seems to be that
"initialize_var()" needs to be inlined (automatically or explicitly
asked for), and then the error flow in the init function is just
complex enough.

At the point where it doesn't make sense when to initialize things
explicitly, and it changes randomly depending on compiler version and
compiler command line flags, there is *no* sane way to work around it.

We could do whack-a-mole with random code cases, but I really feel
that when the warning is that unreliable and the changes to the source
code to make the broken compiler warning shut up are completely
arbitrary and random, it's worse than useless.

                Linus

      parent reply	other threads:[~2016-07-29 18:26 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-16 13:20 [RFC] Turn off -Wmaybe-uninitialized completely and move it to W=1 Borislav Petkov
2014-06-16 21:14 ` Sam Ravnborg
2014-06-24 21:38   ` [PATCH] Kbuild: Move -Wmaybe-uninitialized " Borislav Petkov
2014-07-07 10:53     ` Borislav Petkov
2014-07-08  9:25       ` Paul Bolle
2014-07-08 11:37         ` Borislav Petkov
2014-07-10 10:42           ` Borislav Petkov
2014-07-10 11:03             ` Paul Bolle
2016-07-28  4:20       ` Borislav Petkov
2016-07-28  8:29         ` Ingo Molnar
2016-07-28  8:46           ` Borislav Petkov
2016-07-28 16:49             ` Ingo Molnar
2016-07-28 17:04               ` Ingo Molnar
2016-07-28 17:56             ` Markus Trippelsdorf
2016-07-28 19:03           ` Linus Torvalds
2016-07-28 19:08             ` Linus Torvalds
2016-07-28 20:28               ` Ingo Molnar
2016-07-28 21:22             ` Linus Torvalds
2016-07-29 10:08               ` Arnd Bergmann
2016-07-29 10:19                 ` Borislav Petkov
2016-07-29 10:35                   ` Arnd Bergmann
2016-07-29 18:26                   ` Linus Torvalds [this message]

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=CA+55aFzuBLmHBz5zm4fWk_-1EnR9CkBKS-6h5ns1iBDEyVOp5Q@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matz@suse.de \
    --cc=mingo@kernel.org \
    --cc=mmarek@suse.com \
    --cc=sam@ravnborg.org \
    --cc=x86@kernel.org \
    /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).