Re: [PATCH 07/13] kdb: Tweak "repeat" handling code for "mdW" and "mdWcN"

[Daniel Thompson <daniel.thompson@linaro.org>]

On Mon, Jun 17, 2024 at 05:34:41PM -0700, Douglas Anderson wrote:
> In general, "md"-style commands are meant to be "repeated". This is a
> feature of kdb and "md"-style commands get it enabled because they
> have the flag KDB_REPEAT_NO_ARGS. What this means is that if you type
> "md4c2 0xffffff808ef05400" and then keep hitting return on the "kdb>"
> prompt that you'll read more and more memory. For instance:
>   [5]kdb> md4c2 0xffffff808ef05400
>   0xffffff808ef05400 00000204 00000000                     ........
>   [5]kdb>
>   0xffffff808ef05408 8235e000 00000000                     ..5.....
>   [5]kdb>
>   0xffffff808ef05410 00000003 00000001                     ........
>
> As a side effect of the way kdb works is implemented, you can get the
> same behavior as the above by typing the command again with no
> arguments. Though it seems unlikely anyone would do this it shouldn't
> really hurt:
>   [5]kdb> md4c2 0xffffff808ef05400
>   0xffffff808ef05400 00000204 00000000                     ........
>   [5]kdb> md4c2
>   0xffffff808ef05408 8235e000 00000000                     ..5.....
>   [5]kdb> md4c2
>   0xffffff808ef05410 00000003 00000001                     ........
>
> In general supporting "repeat" should be easy. If argc is 0 then we
> just copy the results of the arg parsing from the last time, making
> sure that the address has been updated. This is all handled nicely in
> the "if (argc == 0)" clause in kdb_md().
>
> Oddly, the "mdW" and "mdWcN" code seems to update "last_bytesperword"
> and "last_repeat", which doesn't seem like it should be necessary. It
> appears that this code is needed to make this use case work, though
> it's a bit unclear if this is truly an important feature to support:
>   [1]kdb> md2c3 0xffffff80c8e2b280
>   0xffffff80c8e2b280 0200 0000 0000                            ...
>   [1]kdb> md2c4
>   0xffffff80c8e2b286 0000 e000 8235 0000                       ...
>
> In order to abstract the code better, remove the code updating
> "last_bytesperword" and "last_repeat" from the "mdW" and "mdWcN"
> handling. This breaks the above case where the user tweaked "argv[0]"
> and then tried to somehow leverage the "repeat" code to do something
> smart, but that feels like it was a misfeature anyway.

I'm not too keen on "successfully" doing the wrong thing.

In that light I'd view this as a feature that is arguably simpler to
implement than it is to error check *not* implementing it. In other
words by the time you add error checking to the argc == 0 path to
spot mismatches then you are better off honouring the user request
rather then telling them they got it wrong.


Daniel.


PS I have never done so but I also wondered if it is reasonable to use
   this feature to manually decompose structures. For example:

     md1c1 structure_pointer; md1c7; md8c1; md8c1; md2c2