On Mon, Jul 02, 2018 at 08:00:43AM -0500, Gustavo A. R. Silva wrote: > Hi Johan, > > On 07/02/2018 03:51 AM, Johan Hovold wrote: > > On Thu, Jun 28, 2018 at 01:40:30PM -0500, Gustavo A. R. Silva wrote: > >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases > >> where we are expecting to fall through. > >> > >> Signed-off-by: Gustavo A. R. Silva <gustavo@xxxxxxxxxxxxxx> > >> --- > >> drivers/usb/serial/io_edgeport.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c > >> index 97c69d3..441dab6 100644 > >> --- a/drivers/usb/serial/io_edgeport.c > >> +++ b/drivers/usb/serial/io_edgeport.c > >> @@ -1760,7 +1760,7 @@ static void process_rcvd_data(struct edgeport_serial *edge_serial, > >> edge_serial->rxState = EXPECT_HDR2; > >> break; > >> } > >> - /* otherwise, drop on through */ > >> + /* else: fall through */ > > > > This doesn't silence the compiler warning with gcc 7.2.0 as the "else: " > > pattern isn't recognised. > > > > I'm using level 2: > > -Wimplicit-fallthrough=2 > > The thing here is that some people have pointed out that it can be misleading to > place a plain fall-through comment after an if-else code block containing a "break". > So, the solution above has proved to be a good one. I don't mind the "else", but I would expect you to mention in the commit message that you're now relying on the non-default warning level (2). > >> case EXPECT_HDR2: > >> edge_serial->rxHeader2 = *buffer; > >> ++buffer; > >> @@ -1820,7 +1820,7 @@ static void process_rcvd_data(struct edgeport_serial *edge_serial, > >> edge_serial->rxState = EXPECT_DATA; > >> break; > >> } > >> - /* Else, drop through */ > >> + /* else: fall through */ > >> } > > > > And this doesn't work either due to the "else: " as well as the fact > > that the compiler expects the fallthrough comment to precede the case > > statement directly (e.g. it would need to be moved out of the else > > block, but that isn't necessarily desirable as we discussed last year: > > > > lkml.kernel.org/r/20171027203906.GA7054@xxxxxxxxxxxxxx > > > > Yes. I'm aware of that. This certainly is still triggering a warning, > so I just consider this > as a temporal approach. I still need to define how are we going to > manage cases like this. Ok, so why did you not mention that in the commit message? If this isn't even addressing the warning you get with the non-default -Wimplicit-fallthrough=2, I don't see this as much of an improvement. Might as well leave this unchanged, until all warnings in that switch statement are addressed. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
