linux-ext4 - Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

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>] [day] [month] [year] [list]
Date:   Wed, 25 Nov 2020 18:04:15 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Nick Desaulniers <ndesaulniers@...gle.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        Kees Cook <keescook@...omium.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Joe Perches <joe@...ches.com>, alsa-devel@...a-project.org,
        linux-atm-general@...ts.sourceforge.net,
        reiserfs-devel@...r.kernel.org, linux-iio@...r.kernel.org,
        linux-wireless <linux-wireless@...r.kernel.org>,
        linux-fbdev@...r.kernel.org,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Nathan Chancellor <natechancellor@...il.com>,
        linux-ide@...r.kernel.org, dm-devel@...hat.com,
        keyrings@...r.kernel.org, linux-mtd@...ts.infradead.org,
        GR-everest-linux-l2@...vell.com, wcn36xx@...ts.infradead.org,
        samba-technical@...ts.samba.org, linux-i3c@...ts.infradead.org,
        linux1394-devel@...ts.sourceforge.net,
        linux-afs@...ts.infradead.org,
        usb-storage@...ts.one-eyed-alien.net, drbd-dev@...ts.linbit.com,
        devel@...verdev.osuosl.org, linux-cifs@...r.kernel.org,
        rds-devel@....oracle.com, linux-scsi@...r.kernel.org,
        linux-rdma@...r.kernel.org, oss-drivers@...ronome.com,
        bridge@...ts.linux-foundation.org,
        linux-security-module@...r.kernel.org,
        amd-gfx list <amd-gfx@...ts.freedesktop.org>,
        linux-stm32@...md-mailman.stormreply.com, cluster-devel@...hat.com,
        linux-acpi@...r.kernel.org, coreteam@...filter.org,
        intel-wired-lan@...ts.osuosl.org,
        linux-input <linux-input@...r.kernel.org>,
        Miguel Ojeda <ojeda@...nel.org>,
        tipc-discussion@...ts.sourceforge.net,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        linux-watchdog@...r.kernel.org, selinux@...r.kernel.org,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        intel-gfx@...ts.freedesktop.org, linux-geode@...ts.infradead.org,
        linux-can@...r.kernel.org, linux-block@...r.kernel.org,
        linux-gpio@...r.kernel.org, op-tee@...ts.trustedfirmware.org,
        linux-mediatek@...ts.infradead.org, xen-devel@...ts.xenproject.org,
        nouveau@...ts.freedesktop.org, linux-hams@...r.kernel.org,
        ceph-devel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-hwmon@...r.kernel.org,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        linux-nfs@...r.kernel.org, GR-Linux-NIC-Dev@...vell.com,
        Linux Memory Management List <linux-mm@...ck.org>,
        Network Development <netdev@...r.kernel.org>,
        linux-decnet-user@...ts.sourceforge.net, linux-mmc@...r.kernel.org,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        linux-sctp@...r.kernel.org, linux-usb@...r.kernel.org,
        netfilter-devel@...r.kernel.org,
        open list:HARDWARE RANDOM NUMBER GENERATOR CORE <linux-crypto@...r.kernel.org>, patches@...nsource.cirrus.com, linux-integrity@...r.kernel.org, target-devel@...r.kernel.org, linux-hardening@...r.kernel.org, Jonathan Cameron <Jonathan.Cameron@...wei.com>, Greg KH 
        <gregkh@...uxfoundation.org>
Subject: Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

On Wed, Nov 25, 2020 at 5:24 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> And just to spell it out,
>
> case ENUM_VALUE1:
>         bla();
>         break;
> case ENUM_VALUE2:
>         bla();
> default:
>         break;
>
> is a fairly idiomatic way of indicating that not all values of the enum
> are expected to be handled by the switch statement.

It looks like a benign typo to me -- `ENUM_VALUE2` does not follow the
same pattern like `ENUM_VALUE1`. To me, the presence of the `default`
is what indicates (explicitly) that not everything is handled.

> Applying a real patch set and then getting a few follow ups the next day
> for trivial coding things like fallthrough missing or static missing,
> just because I didn't have the full range of compilers to check with
> before applying makes me feel pretty shitty, like I'm not doing a good
> job. YMMV.

The number of compilers, checkers, static analyzers, tests, etc. we
use keeps going up. That, indeed, means maintainers will miss more
things (unless maintainers do more work than before). But catching
bugs before they happen is *not* a bad thing.

Perhaps we could encourage more rebasing in -next (while still giving
credit to bots and testers) to avoid having many fixing commits
afterwards, but that is orthogonal.

I really don't think we should encourage the feeling that a maintainer
is doing a bad job if they don't catch everything on their reviews.
Any review is worth it. Maintainers, in the end, are just the
"guaranteed" reviewers that decide when the code looks reasonable
enough. They should definitely not feel pressured to be perfect.

Cheers,
Miguel