This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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)


Dear all,
I've performed all the required changes.

Incidentally, the attached patch contains the sscanf() version of the loop mentioned here below:

  for (i = 0; i < len; ++i)
    {
      int x;
      if (sscanf (cdata + i * 2, "%02x", &x) != 1)
        error (_("Invalid argument"));
      databuf[i] = (gdb_byte) x;
    }

While, on the other hand, using strtoul() instead of sscanf() will lead to something 
like this, or so I guess:

  for (i = 0; i < len; ++i)
      databuf[i] = strtoul (cdata + i * 2, NULL, 16);

The former still being my preferred one, I think it all comes down to personal taste, doesn't it?

Regards,
	Giuseppe

> -----Original Message-----
> From: Eli Zaretskii [mailto:eliz@gnu.org]
> Sent: Thursday, October 18, 2012 7:31 PM
> To: Giuseppe MONTALTO
> Cc: palves@redhat.com; tromey@redhat.com; gdb-
> patches@sourceware.org; Hafiz_Abid@mentor.com
> Subject: 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)
> 
> > 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

Attachment: patch-V.10.patch
Description: patch-V.10.patch


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]