Messages in this thread | | | Subject | Re: [PATCH] kdb: kdb_main: mark expected switch fall-throughs | From | "Gustavo A. R. Silva" <> | Date | Wed, 15 Aug 2018 09:43:46 -0500 |
| |
Hi Daniel,
On 8/15/18 9:34 AM, Daniel Thompson wrote: > On 05/08/18 05:14, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >> where we are expecting to fall through. >> >> Addresses-Coverity-ID: 115090 ("Missing break in switch") >> Addresses-Coverity-ID: 115091 ("Missing break in switch") >> Addresses-Coverity-ID: 114700 ("Missing break in switch") >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > Adding fall through isn't wrong but its reasonable to ask why there is a complex hand unrolled loop here in the first place (and doubly so without a comment). The whole switch statement would be much clear expressed as: > > for (j=0; j<bytesperword; j++) > *c++ = printable_char(*cp++); > addr += bytesperword; >
Yeah, I agree. I can send a patch for that.
Thanks for the feedback. -- Gustavo
> > Daniel. > > >> --- >> kernel/debug/kdb/kdb_main.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c >> index 2ddfce8..2249645 100644 >> --- a/kernel/debug/kdb/kdb_main.c >> +++ b/kernel/debug/kdb/kdb_main.c >> @@ -1545,13 +1545,16 @@ static void kdb_md_line(const char *fmtstr, unsigned long addr, >> *c++ = printable_char(*cp++); >> *c++ = printable_char(*cp++); >> addr += 4; >> + /* fall through */ >> case 4: >> *c++ = printable_char(*cp++); >> *c++ = printable_char(*cp++); >> addr += 2; >> + /* fall through */ >> case 2: >> *c++ = printable_char(*cp++); >> addr++; >> + /* fall through */ >> case 1: >> *c++ = printable_char(*cp++); >> addr++; >> >
| |