linux-kernel - Re: [PATCH 0/7] Silence even more W=2 warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140923184456.GA16467@pd.tnic>
Date:	Tue, 23 Sep 2014 20:44:56 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	"Rustad, Mark D" <mark.d.rustad@...el.com>
Cc:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"sparse@...isli.org" <sparse@...isli.org>,
	"linux-sparse@...r.kernel.org" <linux-sparse@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/7] Silence even more W=2 warnings

On Tue, Sep 23, 2014 at 05:24:22PM +0000, Rustad, Mark D wrote:
> Yes, but I think there are a few cases where it could be helpful. When
> there is something exceptional that will throw a warning. In one of the
> patches that Jeff sent, I used the DIAG_CLANG_IGNORE macro to suppress
> the warning that is thrown for every entry of the syscall table when
> compiled with clang. The code is right and doing exactly what is wanted,
> so note the exception and make it shut up.

So we're doing some dancing around solely to shut up the compiler? Even
if the code is correct?!? Sorry, this is just ass backwards.

> I would generally be in favor of that over time, but I recognize that with
> the range of architectures and toolchains that need to be supported, that
> may be difficult.

Right.

> Generally there should be relatively few exceptions that need to be tagged.
> If there were 1000, that would be way too many. The full series of my
> patches had 90 instances. Some of the few that have been sent have been
> resolved in better ways, which we all agree is better. Suppose that 40
> can't be reasonably resolved in direct ways. Is that really costly? I'm
> trying to understand what your perception of the cost is.

If it were me, I'd say even one is too much. Because the very thing
of adding code just to shut up the compiler which generates otherwise
correct code is simply very very wrong in my book.

Bear in mind that even if initially you have a low number of macro
invocations, that number will grow because people will start using it in
other places too. And all of a sudden, the thing has spread like weed
and there's no removing it anymore. So we better not start in the first
place.

> Perhaps checkpatch would be a better gatekeeper for new code. OTOH,
> some of those nested externs have already been eliminated, so at
> least for now the warning is serving a purpose since it is scrubbing
> existing code.

Yep, eliminating would be optimal. If it is in checkpatch, it is much
easier to manage.

> I take it as a given that the kernel sometimes has special needs
> and needs to do special things. The macros make it possible to mark
> those special usages. I prefer adding the macros in a few places to
> eliminating a warning altogether unless the warning is always useless.

Please, no compiler-shutting-up code, see above.

> I'm sure we agree that we don't want to ever turn on -pedantic! :-)

Again, we should take compiler warnings with a grain of salt and judge
them only by the quality of the generated code. IMO.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/