[PATCH 06/13] kdb: Remove "mdW" and "mdWcN" handling of "W" == 0

Douglas Anderson posted 13 patches 1 year, 6 months ago
[PATCH 06/13] kdb: Remove "mdW" and "mdWcN" handling of "W" == 0
Posted by Douglas Anderson 1 year, 6 months ago
The "mdW" and "mdWcN" generally lets the user control more carefully
what word size we display memory in and exactly how many words should
be displayed. Specifically, "md4" says to display memory w/ 4
bytes-per word and "md4c6" says to display 6 words of memory w/
4-bytes-per word.

The kdb "md" implementation has a special rule for when "W" is 0. In
this case:
* If you run with "W" == 0 and you've never run a kdb "md" command
  this reboot then it will pick 4 bytes-per-word, ignoring the normal
  default from the environment.
* If you run with "W" == 0 and you've run a kdb "md" command this
  reboot then it will pick up the bytes per word of the last command.

As an example:
  [1]kdb> md2 0xffffff80c8e2b280 1
  0xffffff80c8e2b280 0200 0000 0000 0000 e000 8235 0000 0000   ...
  [1]kdb> md0 0xffffff80c8e2b280 1
  0xffffff80c8e2b280 0200 0000 0000 0000 e000 8235 0000 0000   ...
  [1]kdb> md 0xffffff80c8e2b280 1
  0xffffff80c8e2b280 0000000000000200 000000008235e000   ...
  [1]kdb> md0 0xffffff80c8e2b280 1
  0xffffff80c8e2b280 0000000000000200 000000008235e000   ...

This doesn't seem like particularly useful behavior and adds a bunch
of complexity to the arg parsing. Remove it.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 kernel/debug/kdb/kdb_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index c013b014a7d3..700b4e355545 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1611,11 +1611,6 @@ static int kdb_md(int argc, const char **argv)
 
 	if (isdigit(argv[0][2])) {
 		bytesperword = (int)(argv[0][2] - '0');
-		if (bytesperword == 0) {
-			bytesperword = last_bytesperword;
-			if (bytesperword == 0)
-				bytesperword = 4;
-		}
 		last_bytesperword = bytesperword;
 		repeat = mdcount * 16 / bytesperword;
 		if (!argv[0][3])
-- 
2.45.2.627.g7a2c4fd464-goog
Re: [PATCH 06/13] kdb: Remove "mdW" and "mdWcN" handling of "W" == 0
Posted by Daniel Thompson 1 year, 6 months ago
On Mon, Jun 17, 2024 at 05:34:40PM -0700, Douglas Anderson wrote:
> The "mdW" and "mdWcN" generally lets the user control more carefully
> what word size we display memory in and exactly how many words should
> be displayed. Specifically, "md4" says to display memory w/ 4
> bytes-per word and "md4c6" says to display 6 words of memory w/
> 4-bytes-per word.
>
> The kdb "md" implementation has a special rule for when "W" is 0. In
> this case:
> * If you run with "W" == 0 and you've never run a kdb "md" command
>   this reboot then it will pick 4 bytes-per-word, ignoring the normal
>   default from the environment.
> * If you run with "W" == 0 and you've run a kdb "md" command this
>   reboot then it will pick up the bytes per word of the last command.
>
> As an example:
>   [1]kdb> md2 0xffffff80c8e2b280 1
>   0xffffff80c8e2b280 0200 0000 0000 0000 e000 8235 0000 0000   ...
>   [1]kdb> md0 0xffffff80c8e2b280 1
>   0xffffff80c8e2b280 0200 0000 0000 0000 e000 8235 0000 0000   ...
>   [1]kdb> md 0xffffff80c8e2b280 1
>   0xffffff80c8e2b280 0000000000000200 000000008235e000   ...
>   [1]kdb> md0 0xffffff80c8e2b280 1
>   0xffffff80c8e2b280 0000000000000200 000000008235e000   ...
>
> This doesn't seem like particularly useful behavior and adds a bunch
> of complexity to the arg parsing. Remove it.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
>  kernel/debug/kdb/kdb_main.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index c013b014a7d3..700b4e355545 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1611,11 +1611,6 @@ static int kdb_md(int argc, const char **argv)
>
>  	if (isdigit(argv[0][2])) {
>  		bytesperword = (int)(argv[0][2] - '0');
> -		if (bytesperword == 0) {
> -			bytesperword = last_bytesperword;
> -			if (bytesperword == 0)
> -				bytesperword = 4;
> -		}
>  		last_bytesperword = bytesperword;
>  		repeat = mdcount * 16 / bytesperword;

Isn't this now a divide-by-zero?


Daniel.
Re: [PATCH 06/13] kdb: Remove "mdW" and "mdWcN" handling of "W" == 0
Posted by Doug Anderson 1 year, 6 months ago
Hi,

On Tue, Jun 18, 2024 at 4:38 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Mon, Jun 17, 2024 at 05:34:40PM -0700, Douglas Anderson wrote:
> > The "mdW" and "mdWcN" generally lets the user control more carefully
> > what word size we display memory in and exactly how many words should
> > be displayed. Specifically, "md4" says to display memory w/ 4
> > bytes-per word and "md4c6" says to display 6 words of memory w/
> > 4-bytes-per word.
> >
> > The kdb "md" implementation has a special rule for when "W" is 0. In
> > this case:
> > * If you run with "W" == 0 and you've never run a kdb "md" command
> >   this reboot then it will pick 4 bytes-per-word, ignoring the normal
> >   default from the environment.
> > * If you run with "W" == 0 and you've run a kdb "md" command this
> >   reboot then it will pick up the bytes per word of the last command.
> >
> > As an example:
> >   [1]kdb> md2 0xffffff80c8e2b280 1
> >   0xffffff80c8e2b280 0200 0000 0000 0000 e000 8235 0000 0000   ...
> >   [1]kdb> md0 0xffffff80c8e2b280 1
> >   0xffffff80c8e2b280 0200 0000 0000 0000 e000 8235 0000 0000   ...
> >   [1]kdb> md 0xffffff80c8e2b280 1
> >   0xffffff80c8e2b280 0000000000000200 000000008235e000   ...
> >   [1]kdb> md0 0xffffff80c8e2b280 1
> >   0xffffff80c8e2b280 0000000000000200 000000008235e000   ...
> >
> > This doesn't seem like particularly useful behavior and adds a bunch
> > of complexity to the arg parsing. Remove it.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  kernel/debug/kdb/kdb_main.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index c013b014a7d3..700b4e355545 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -1611,11 +1611,6 @@ static int kdb_md(int argc, const char **argv)
> >
> >       if (isdigit(argv[0][2])) {
> >               bytesperword = (int)(argv[0][2] - '0');
> > -             if (bytesperword == 0) {
> > -                     bytesperword = last_bytesperword;
> > -                     if (bytesperword == 0)
> > -                             bytesperword = 4;
> > -             }
> >               last_bytesperword = bytesperword;
> >               repeat = mdcount * 16 / bytesperword;
>
> Isn't this now a divide-by-zero?

Dang, you're right. It goes away in a later patch, though, since we
stop re-calculating everything until the end when things are
validated. I'll plan to reorder this patch to be after the patch
("kdb: In kdb_md() make `repeat` and `mdcount` calculations more
obvious").