* [PATCH] kdb: kdb_main: mark expected switch fall-throughs
@ 2018-08-05 4:14 Gustavo A. R. Silva
2018-08-15 14:34 ` Daniel Thompson
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2018-08-05 4:14 UTC (permalink / raw)
To: Jason Wessel, Daniel Thompson
Cc: kgdb-bugreport, linux-kernel, Gustavo A. R. Silva
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>
---
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++;
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] kdb: kdb_main: mark expected switch fall-throughs
2018-08-05 4:14 [PATCH] kdb: kdb_main: mark expected switch fall-throughs Gustavo A. R. Silva
@ 2018-08-15 14:34 ` Daniel Thompson
2018-08-15 14:43 ` Gustavo A. R. Silva
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Thompson @ 2018-08-15 14:34 UTC (permalink / raw)
To: Gustavo A. R. Silva, Jason Wessel; +Cc: kgdb-bugreport, linux-kernel
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;
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++;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] kdb: kdb_main: mark expected switch fall-throughs
2018-08-15 14:34 ` Daniel Thompson
@ 2018-08-15 14:43 ` Gustavo A. R. Silva
0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2018-08-15 14:43 UTC (permalink / raw)
To: Daniel Thompson, Jason Wessel; +Cc: kgdb-bugreport, linux-kernel
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++;
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-15 15:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-05 4:14 [PATCH] kdb: kdb_main: mark expected switch fall-throughs Gustavo A. R. Silva
2018-08-15 14:34 ` Daniel Thompson
2018-08-15 14:43 ` Gustavo A. R. Silva
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.