netdev - Re: [PATCH v2] wireless: mark expected switch fall-throughs

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] [day] [month] [year] [list]

Date:   Wed, 24 Oct 2018 20:40:47 +1100
From:   Julian Calaby <julian.calaby@...il.com>
To:     gustavo@...eddedor.com
Cc:     "David S. Miller" <davem@...emloft.net>,
        Johannes Berg <johannes@...solutions.net>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v2] wireless: mark expected switch fall-throughs

Hi Gustavo,

On Wed, Oct 24, 2018 at 7:36 AM Johannes Berg <johannes@...solutions.net> wrote:
>
> On Tue, 2018-10-23 at 12:58 +0200, Gustavo A. R. Silva wrote:
> > On 10/23/18 10:59 AM, Gustavo A. R. Silva wrote:
> > >
> > > On 10/23/18 9:01 AM, Johannes Berg wrote:
> > > > On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote:
> > > > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > > > > where we are expecting to fall through.
> > > > >
> > > > > Warning level 3 was used: -Wimplicit-fallthrough=3
> > > > >
> > > > > This code was not tested and GCC 7.2.0 was used to compile it.
> > > >
> > > > Look, I'm not going to make this any clearer: I'm not applying patches
> > > > like that where you've invested no effort whatsoever on verifying that
> > > > they're correct.
> > > >
> > >
> > > How do you suggest me to verify that every part is correct in this type
> > > of patches?
> > >
> >
> > BTW... I'm under the impression you think that I don't even look at
> > the code. Is that correct?
>
> That's what your commit log looks like, yes. This is your full commit
> log:
>
>    In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>    where we are expecting to fall through.
>
>    Warning level 3 was used: -Wimplicit-fallthrough=3
>
>    This code was not tested and GCC 7.2.0 was used to compile it.
>
>    For all I know, you could've run spatch to add the comments wherever
>    there was no break, and then compiled it once.

It might also help to split these up by the "type" of change. For
example this patch contains:

1. A whole lot of places where there's a "if (condition) { break; }"
just before the fall through

You could add a line describing the logic there to the commit message,
maybe something like:

As there's a conditional break at the end of the case, the fall
through appears to be intentional.


2. Reformatting an existing comment

These should definitely be split out as they're just reformatting
comments to match gcc's expectations. You could describe this like:

Reformat existing fall through messages into the format expected by gcc.


Thanks,

-- 
Julian Calaby

Email: julian.calaby@...il.com
Profile: http://www.google.com/profiles/julian.calaby/