Re: [PATCH] Clear *VAL in regcache_raw_read_unsigned

[Pedro Alves <palves at redhat dot com>]

On 02/11/2016 04:59 PM, Yao Qi wrote:
> Hi Pedro,
> 
> On 11/02/16 15:51, Pedro Alves wrote:
>> I think it's only going to bite us back in the future.
>>
>>  From the perspective of potentially making it easier to share more
>> code between gdb and gdbserver, I'd prefer that.  Would you object it?
>>
> 
> No, I am not against code sharing between GDB and GDBserver in general,
> but sharing extract_unsigned_integer is not necessary for GDBserver.

It's not like like we'd be bringing in some big complicated piece of
code.  The routines in question are quite small.  It's about the
same code as the other solution with the switch.  Compare,

gdb's:

ULONGEST
extract_unsigned_integer (const gdb_byte *addr, int len,
                         enum bfd_endian byte_order)
{
  ULONGEST retval;
  const unsigned char *p;
  const unsigned char *startaddr = addr;
  const unsigned char *endaddr = startaddr + len;

  if (len > (int) sizeof (ULONGEST))
    error (_("\
That operation is not available on integers of more than %d bytes."),
          (int) sizeof (ULONGEST));

  /* Start at the most significant end of the integer, and work towards
     the least significant.  */
  retval = 0;
  if (byte_order == BFD_ENDIAN_BIG)
    {
      for (p = startaddr; p < endaddr; ++p)
       retval = (retval << 8) | *p;
    }
  else
    {
      for (p = endaddr - 1; p >= startaddr; --p)
       retval = (retval << 8) | *p;
    }
  return retval;
}

vs

gdbserver's, using a switch:

ULONGEST
extract_unsigned_integer (const gdb_byte *addr, int len)
{
  uint8_t u8;
  uint16_t u16;
  uint32_t u32;
  uint64_t u64;

  if (len > (int) sizeof (ULONGEST))
    error (_("That operation is not available on integers of more than"
            "%d bytes."),
          (int) sizeof (ULONGEST));

  switch (len)
  {
    case 1:
      memcpy (&u8, addr, len);
      return u8;
   case 2:
     memcpy (&u16, addr, len);
     return u16;
   case 4:
     memcpy (&u32, addr, len);
     return u32;
   case 8:
     memcpy (&u64, addr, len);
     return u64;
  }
}

I just don't see what the worry is.  OTOH, sharing the routines
would pave to way to share more, and have one less diverging detail
to worry about, avoiding proliferation of "bridging" interfaces like
regcache_raw_read_unsigned or the need to hook in more entry points
to get_next_pcs and whatever else we put in shared code areas.

Thanks,
Pedro Alves