linux-kernel - Re: [PATCH] arm64/cache: silence -Woverride-init 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]

Date:   Fri, 9 Aug 2019 10:04:13 +0100
From:   Will Deacon <will@...nel.org>
To:     Qian Cai <cai@....pw>
Cc:     Mark Rutland <mark.rutland@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        linux-arm-kernel@...ts.infradead.org,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64/cache: silence -Woverride-init warnings

On Thu, Aug 08, 2019 at 06:18:39PM -0400, Qian Cai wrote:
> > On Aug 8, 2019, at 6:38 AM, Mark Rutland <mark.rutland@....com> wrote:
> > Please just turn this off by default for clang.
> 
> As mentioned before, it is very valuable to run “make W=1” given it found
> many real developer mistakes which will enable “-Woverride-init” for both
> compilers. Even “-Woverride-init” itself is useful find real issues as in,

Every single patch you've sent to me resulting from building with "W=1" has
been a false positive. Every. Single. One. That's not what I would call
"very valuable". It's probably what I'd call a "colossal waste of
everybody's time", especially as your tendancy to shoot from the hip when
writing these so-called fixes has resulted in some patches that ACTUALLY
INTRODUCED REAL BUGS. Do you see the insanity here?

> ae5e033d65a (“mfd: rk808: Fix RK818_IRQ_DISCHG_ILIM initializer”)
> 32df34d875bb (“[media] rc: img-ir: jvc: Remove unused no-leader timings”)
> 
> Especially, to find redundant initializations in large structures. e.g.,
> 
> e6ea0b917875 (“[media] dvb_frontend: Don't declare values twice at a table”)
> 
> It is important to keep the noise-level as low as possible by keeping the
> amount of false positives under control to be truly benefit from those
> valuable compiler warnings. 

So that's where you and I have a disagreement. I think maintainability
of the code is the single most important thing to consider after
correctness.

If code isn't maintainable, then it's liable to churn and be a constant
source of bugs as people keep tripping over whatever subtleties lie within.
In some cases, we have little choice and the combination of things like
performance requirements and compatibility force us down a path which we
wouldn't otherwise have considered. However, appeasing a compiler warning
that *doesn't even appear with the default build options* does not quality
for this sort of treatment in my opinion, so I will not be applying your
patch.

Please stop sending it. Real fixes, sure, but not this rubbish.

Will