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 <eliz at gnu dot org>]

> 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