linux-kernel - Re: [PATCH, RESEND 03/14] drm/vmwgfx: avoid gcc-7 parentheses warning

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, 14 Jul 2017 22:28:29 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        VMware Graphics <linux-graphics-maintainer@...are.com>,
        Sinclair Yeh <syeh@...are.com>,
        Thomas Hellstrom <thellstrom@...are.com>,
        David Airlie <airlied@...ux.ie>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejun Heo <tj@...nel.org>, Guenter Roeck <linux@...ck-us.net>,
        IDE-ML <linux-ide@...r.kernel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        DRI <dri-devel@...ts.freedesktop.org>,
        Brian Paul <brianp@...are.com>
Subject: Re: [PATCH, RESEND 03/14] drm/vmwgfx: avoid gcc-7 parentheses warning

On Fri, Jul 14, 2017 at 9:23 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Fri, Jul 14, 2017 at 12:21 PM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> NAK. This takes unintentionally insane code and turns it intentionally
>> insane. Any non-zero return is considered an error.
>>
>> The right fix is almost certainly to just return -EINVAL unconditionally.
>
> Btw, this is why I hate compiler warning fix patch series. Even when
> they don't actually break the code (and sometimes they do that too),
> they can actually end up making the code worse.

I generally agree, and this is also why I held up sending patches for the
-Wformat warnings until you brought those up. I also frequently send
patches for recently introduced warnings, which tend to have a better
chance of getting reviewed by the person that just introduced the code,
to catch this kind of mistake in my patches.

I also regularly run into cases where I send a correct patch and find
that another broken patch has been applied the following day ;-)

> The *intent* of that code was to return zero for the CAP_SYS_ADMIN.
> But the code has never done that in its lifetime and nobody ever
> noticed, so clearly the code shouldn't even have tried.

Makes sense, yes. In this case, the review process has failed as
well, as one of the maintainers even gave an Ack on the wrong patch,
and then the patch got dropped without any feedback.

        Arnd