Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)

Eli Zaretskii
2012-10-18T17:30:35Z

> From: Giuseppe MONTALTO <giuseppe.montalto@st.com>
> Cc: Tom Tromey <tromey@redhat.com>, "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> Date: Thu, 18 Oct 2012 18:16:34 +0200
>
> Please, find attached a new patch with all the requested changes (also includes
> The additons to NEWS and gdb.texinfo that I previously posted in a separate patch).

Thanks.

> + ** New optional parameter added to the "-data-write-memory-bytes" command,
> + to allow pattern filling of memory areas.

Please name the new parameter. Otherwise, this part is OK.

> +@item @var{count}
> +Optional argument indicating the number of bytes to be written. If @var{count} is bigger than @var{contents}' length, pattern filling occurs.

What exactly does "pattern filling" mean? This should be explained.
Looking at the code, I think you mean

If @var{count} is greater than @var{contents}' length, @value{GDBN}
will repeatedly write @var{contents} until it fills @var{count}
bytes.

The patch for the manual is OK with these changes.

> for (i = 0; i < len; ++i)
> {
> int x;
> sscanf (cdata + i * 2, "%02x", &x);
> - data[i] = (gdb_byte) x;
> + databuf[i] = (gdb_byte) x;
> + }

Sorry for chiming in late wrt the code parts, but:

. you don't test the return value of sscanf, is that a good idea?

. won't it be better to use strtoul here? that would avoid the need
to cast to gdb_byte, AFAIU