Re: [PATCH/committed] sim: rl78: clean up various warnings

Mike Frysinger via Gdb-patches
2021-05-05T18:53:05Z

On 05 May 2021 11:30, Tom Tromey wrote:
> >>>>> "Mike" == Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Mike> Fix a logic error when processing commands where p was reassigned
> Mike> to cmd and then has its leading whitespace scanned a 2nd time.
> Mike> Handle short reads with fread().
>
> Mike> - /* Find the extent of the command word. */
> Mike> - for (p = cmd; *p; p++)
> Mike> - if (isspace (*p))
> Mike> - break;
> Mike> -
>
> I'm not sure it makes sense to completely delete this.
> Perhaps instead just removing 'p = cmd' is correct?
>
> It looks to me that this code is trying to parse a command name followed
> by arguments. Like, ideally, "monitor trace on" should set cmd="trace"
> and args="on", but with the patch I don't think this will happen.

you're right that the code is more subtle than i thought. but it's still
broken with that tweak. it's broken today too. pretty sure this code has
always been wrong and crashes, either from writing to read-only memory, or
trying to free an invalid pointer. it basically needs to be gutted.

glancing at some other ports (m32c & rx), they have the same bugs, so looks
like it started with one bad form, and then copied around.

i think replacing it with buildargv is easiest.
-mike