* [PATCH] kdb: kdb_main: refactor code in kdb_md_line
@ 2018-08-15 15:06 Gustavo A. R. Silva
2018-08-16 9:32 ` Daniel Thompson
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2018-08-15 15:06 UTC (permalink / raw)
To: Daniel Thompson, Jason Wessel
Cc: kgdb-bugreport, linux-kernel, Gustavo A. R. Silva
Replace the whole switch statement with a for loop.
This makes the code much clear and easy to read.
This also addresses the following Coverity warnings:
Addresses-Coverity-ID: 115090 ("Missing break in switch")
Addresses-Coverity-ID: 115091 ("Missing break in switch")
Addresses-Coverity-ID: 114700 ("Missing break in switch")
Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
kernel/debug/kdb/kdb_main.c | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 2ddfce8..4b896a6 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1538,25 +1538,9 @@ static void kdb_md_line(const char *fmtstr, unsigned long addr,
wc.word = word;
#define printable_char(c) \
({unsigned char __c = c; isascii(__c) && isprint(__c) ? __c : '.'; })
- switch (bytesperword) {
- case 8:
+ for (i = 0; i < bytesperword; i++)
*c++ = printable_char(*cp++);
- *c++ = printable_char(*cp++);
- *c++ = printable_char(*cp++);
- *c++ = printable_char(*cp++);
- addr += 4;
- case 4:
- *c++ = printable_char(*cp++);
- *c++ = printable_char(*cp++);
- addr += 2;
- case 2:
- *c++ = printable_char(*cp++);
- addr++;
- case 1:
- *c++ = printable_char(*cp++);
- addr++;
- break;
- }
+ addr += bytesperword;
#undef printable_char
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] kdb: kdb_main: refactor code in kdb_md_line
2018-08-15 15:06 [PATCH] kdb: kdb_main: refactor code in kdb_md_line Gustavo A. R. Silva
@ 2018-08-16 9:32 ` Daniel Thompson
2018-08-16 13:47 ` Gustavo A. R. Silva
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Thompson @ 2018-08-16 9:32 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: Jason Wessel, kgdb-bugreport, linux-kernel
On Wed, Aug 15, 2018 at 10:06:16AM -0500, Gustavo A. R. Silva wrote:
> Replace the whole switch statement with a for loop.
> This makes the code much clear and easy to read.
>
> This also addresses the following Coverity warnings:
>
> Addresses-Coverity-ID: 115090 ("Missing break in switch")
> Addresses-Coverity-ID: 115091 ("Missing break in switch")
> Addresses-Coverity-ID: 114700 ("Missing break in switch")
>
> Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> kernel/debug/kdb/kdb_main.c | 20 ++------------------
> 1 file changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 2ddfce8..4b896a6 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1538,25 +1538,9 @@ static void kdb_md_line(const char *fmtstr, unsigned long addr,
> wc.word = word;
> #define printable_char(c) \
> ({unsigned char __c = c; isascii(__c) && isprint(__c) ? __c : '.'; })
> - switch (bytesperword) {
> - case 8:
> + for (i = 0; i < bytesperword; i++)
i already a control variable for the outer loop. It is not safe to reuse
it here.
Daniel.
> *c++ = printable_char(*cp++);
> - *c++ = printable_char(*cp++);
> - *c++ = printable_char(*cp++);
> - *c++ = printable_char(*cp++);
> - addr += 4;
> - case 4:
> - *c++ = printable_char(*cp++);
> - *c++ = printable_char(*cp++);
> - addr += 2;
> - case 2:
> - *c++ = printable_char(*cp++);
> - addr++;
> - case 1:
> - *c++ = printable_char(*cp++);
> - addr++;
> - break;
> - }
> + addr += bytesperword;
> #undef printable_char
> }
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] kdb: kdb_main: refactor code in kdb_md_line
2018-08-16 9:32 ` Daniel Thompson
@ 2018-08-16 13:47 ` Gustavo A. R. Silva
0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2018-08-16 13:47 UTC (permalink / raw)
To: Daniel Thompson; +Cc: Jason Wessel, kgdb-bugreport, linux-kernel
On 8/16/18 4:32 AM, Daniel Thompson wrote:
>> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
>> index 2ddfce8..4b896a6 100644
>> --- a/kernel/debug/kdb/kdb_main.c
>> +++ b/kernel/debug/kdb/kdb_main.c
>> @@ -1538,25 +1538,9 @@ static void kdb_md_line(const char *fmtstr, unsigned long addr,
>> wc.word = word;
>> #define printable_char(c) \
>> ({unsigned char __c = c; isascii(__c) && isprint(__c) ? __c : '.'; })
>> - switch (bytesperword) {
>> - case 8:
>> + for (i = 0; i < bytesperword; i++)
>
> i already a control variable for the outer loop. It is not safe to reuse
> it here.
>
Oh I see. I'll send v2 shortly.
Thanks
--
Gustavo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-16 13:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-15 15:06 [PATCH] kdb: kdb_main: refactor code in kdb_md_line Gustavo A. R. Silva
2018-08-16 9:32 ` Daniel Thompson
2018-08-16 13:47 ` 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.