Re: [PATCH 01/13] New regcache_raw_get_signed

[Luis Machado <lgustavo at codesourcery dot com>]

On 11/22/2016 03:28 AM, Yao Qi wrote:
> On Wed, Nov 16, 2016 at 09:09:40AM -0600, Luis Machado wrote:
> > > +LONGEST
> > > +regcache_raw_get_signed (struct regcache *regcache, int regnum)
> > > +{
> >
> > I don't quite like the name, but it is debatable whether it is
> > appropriate or not. :-)
> >
> > Making it shorter and to the point since it gets used so much?
>
> Hi Luis,
> [Sorry for the late response.  I was in a training last week, C++ 11
> training btw.]
>
> We've already had regcache_raw_get_unsigned (in common/common-regcache.h),
> so it is reasonable to add regcache_raw_get_signed.  The regcache API
> scheme is like regcache_{raw,cooked}_{read,write,get}_{signed,unsigned}.
> I can't find any redundant bits in the function name.

True. Thanks for clarifying this.

> > > diff --git a/gdb/regcache.h b/gdb/regcache.h
> > > index 1bb0ce0..19ea976 100644
> > > --- a/gdb/regcache.h
> > > +++ b/gdb/regcache.h
> > > @@ -66,6 +66,9 @@ extern void regcache_raw_write_signed (struct regcache *regcache,
> > > extern void regcache_raw_write_unsigned (struct regcache *regcache,
> > > 					 int regnum, ULONGEST val);
> > >
> > > +extern LONGEST regcache_raw_get_signed (struct regcache *regcache,
> > > +					int regnum);
> > > +
> > > /* Set a raw register's value in the regcache's buffer.  Unlike
> > >    regcache_raw_write, this is not write-through.  The intention is
> > >    allowing to change the buffer contents of a read-only regcache
> >
> > I understand this command just mimics what get_frame_register_signed
> > does, but is it worth having some documentation to make it clear
> > what this does and that it throws when a register is not available?
>
> How about this comment ?
>
> /* Return the register's value in signed or throw if it's not available.  */

Looks good to me.

Thanks,
Luis