From: Bjørn Mork > Sent: 28 October 2017 11:57 > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > > where we are expecting to fall through. > > > > Notice that in this particular case I replaced "...drop on through" > > comments with a proper "fall through" comment on its own line, which > > is what GCC is expecting to find. > > Sounds to me like GCC is the wrong tool for this. But I would of course > not mind if it was *just* the text. However, as your patch cleary > shows, the simplified logic leads to real problems: > > > @@ -1819,8 +1819,8 @@ static void process_rcvd_data(struct edgeport_serial *edge_serial, > > edge_serial->rxState = EXPECT_DATA; > > break; > > } > > - /* Else, drop through */ > > } > > + /* fall through */ > > case EXPECT_DATA: /* Expect data */ > > if (bufferLength < edge_serial->rxBytesRemaining) { > > rxLen = bufferLength; > > > The original comment clearly marked a *conditional* fall through at the > correct place. Your patch makes it appear as if there is an > unconditional fall through here. There is not. The fallthrough only > applies to one of a number of nested if blocks. There are no less than > 3 break statements in the same case block. > > Not a big deal maybe, just as the lack of any "fall through" comment > isn't a big deal in the first place. But this change is clearly making > this code harder to read, and the change is therefore harmful IMHO. > > If you can't make -Wimplicit-fallthrough work without removing such > precise fallthrough markings, then my proposal is to drop it and use > some other tool. Just remove the 'else' after the 'break' further up. David ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥