[01/13] New regcache_raw_get_signed

  This patch adds a new regcache api regcache_raw_get_signed.

gdb:

2016-11-14  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (regcache_raw_get_signed): New function.
	* regcache.h (regcache_raw_get_signed): Declare.
---
 gdb/regcache.c | 13 +++++++++++++
 gdb/regcache.h |  3 +++
 2 files changed, 16 insertions(+)
  

On 11/14/2016 11:42 AM, Yao Qi wrote:
> This patch adds a new regcache api regcache_raw_get_signed.
>
> gdb:
>
> 2016-11-14  Yao Qi  <yao.qi@linaro.org>
>
> 	* regcache.c (regcache_raw_get_signed): New function.
> 	* regcache.h (regcache_raw_get_signed): Declare.
> ---
>  gdb/regcache.c | 13 +++++++++++++
>  gdb/regcache.h |  3 +++
>  2 files changed, 16 insertions(+)
>
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index a5c90a6..1fcf933 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -742,6 +742,19 @@ regcache_raw_write_unsigned (struct regcache *regcache, int regnum,
>    regcache_raw_write (regcache, regnum, buf);
>  }
>
> +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?

> +  LONGEST value;
> +  enum register_status status;
> +
> +  status = regcache_raw_read_signed (regcache, regnum, &value);
> +  if (status == REG_UNAVAILABLE)
> +    throw_error (NOT_AVAILABLE_ERROR,
> +		 _("Register %d is not available"), regnum);
> +  return value;
> +}
> +
>  enum register_status
>  regcache_cooked_read (struct regcache *regcache, int regnum, gdb_byte *buf)
>  {
> 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?
  

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.

> >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.  */
  

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
  

On Tue, Nov 22, 2016 at 1:01 PM, Luis Machado <lgustavo@codesourcery.com> wrote:
>> How about this comment ?
>>
>> /* Return the register's value in signed or throw if it's not available.
>> */
>>
>
> Looks good to me.
>

OK, the comment is added then.  I pushed this series in.