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.

>> >
>>> >>
>>> >>Or maybe better, just byte the bullet and make gdbserver use
>>> >>extract_unsigned_integer, like gdb.
>>> >>
>>> >>The problem with that is that gdbserver can't currently use 'enum bfd_endian',
>>> >>which IIRC, was already an issue in the get-next-pcs stuff.  I've attached a
>> >
>> >get-next-pcs stuff needs endianness in GDB side.  In GDBserver,
>> >endianness is not needed.
> The get-next-pcs stuff does have endianness bits, but it works
> around the lack of 'enum bfd_endian' by hacking it through an int instead:

IMO, it was a design issue to have endianness in get-next-pcs stuff,
because endianess is only used by GDB, so we shouldn't bring endianess
into get-next-pcs at all.  I pointed it out during the review
https://sourceware.org/ml/gdb-patches/2015-12/msg00040.html but I
didn't insist on this, because I didn't want this block the whole patch
set and I can fix this issue later.

In fact, I've already had a patch to remove byte_order and
byte_oder_for_code from get-next-pcs, but have no change to post it out
due to the recent bug fixing for 7.11 release.

>
> void
> arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
> 		       struct arm_get_next_pcs_ops *ops,
> 		       int byte_order,
> 		       int byte_order_for_code,
> 		       int has_thumb2_breakpoint,
> 		       struct regcache *regcache)
> {
>
> $ grep byte_order *
>
> arm-get-next-pcs.c:                    int byte_order,
> arm-get-next-pcs.c:                    int byte_order_for_code,
> arm-get-next-pcs.c:  self->byte_order = byte_order;
> arm-get-next-pcs.c:  self->byte_order_for_code = byte_order_for_code;
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  insn1 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
> arm-get-next-pcs.c:  insn2 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
> arm-get-next-pcs.c:      insn1 = self->ops->read_mem_uint (loc, 2,byte_order_for_code);
> arm-get-next-pcs.c:       insn2 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  insn = self->ops->read_mem_uint (loc, 4, byte_order_for_code);
> arm-get-next-pcs.c:      insn = self->ops->read_mem_uint (loc, 4, byte_order_for_code);
> arm-get-next-pcs.c:  int byte_order = self->byte_order;
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
> arm-get-next-pcs.c:           inst1 = self->ops->read_mem_uint (pc, 2,byte_order_for_code);
> arm-get-next-pcs.c:               inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
> arm-get-next-pcs.c:               inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
> arm-get-next-pcs.c:      nextpc = self->ops->read_mem_uint (sp + offset, 4, byte_order);
> arm-get-next-pcs.c:      inst2 = self->ops->read_mem_uint (pc + 2, 2, byte_order_for_code);
> arm-get-next-pcs.c:           nextpc = self->ops->read_mem_uint (addr + offset, 4, byte_order);
> arm-get-next-pcs.c:           = self->ops->read_mem_uint (base, 4, byte_order);
> arm-get-next-pcs.c:       length = 2 * self->ops->read_mem_uint (table + offset, 1, byte_order);
> arm-get-next-pcs.c:       length = 2 * self->ops->read_mem_uint (table + offset, 2, byte_order);
> arm-get-next-pcs.c:  int byte_order = self->byte_order;
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  this_instr = self->ops->read_mem_uint (pc, 4, byte_order_for_code);
> arm-get-next-pcs.c:                                                         4, byte_order);
> arm-get-next-pcs.c:                                                              4, byte_order);
> arm-get-next-pcs.h:  ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order);
> arm-get-next-pcs.h:  int byte_order;
> arm-get-next-pcs.h:  int byte_order_for_code;
> arm-get-next-pcs.h:                         int byte_order,
> arm-get-next-pcs.h:                         int byte_order_for_code,
>
>

All of them can be removed by the patch below,

-- 
Yao (齐尧)