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

[Julian Calaby]

Hi Gustavo,

On Wed, Oct 24, 2018 at 7:36 AM Johannes Berg <johannes@xxxxxxxxxxxxxxxx> 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@xxxxxxxxx
Profile: http://www.google.com/profiles/julian.calaby/