Re: Displaced stepping 0002: refactor and create some copy helpers

["Ulrich Weigand" <uweigand@de.ibm.com>]

Yao Qi wrote:

> The patch continues to refactor code, in order to
>  1) make copy functions separated for ARM and Thumb,

I think it makes sense to split the copy functions into the part that actually
decodes the instruction and installs the target instruction (which depends on
ARM vs. Thumb), and the part that sets up registers and installs the cleanup etc. 
(which only depends on the semantics of the instruction, not its encoding).

For symmetry, I'd then prefer if *all* copy functions were split up that way
(in the end, you'll probably need all of them anyway, since just about all
ARM instructions have an alternate encoding as Thumb/Thumb-2).

>  2) define some copy helper functions for some ARM and Thumb-2 instructions.

These look really awkward to me, what with the union and all those function
pointers ...   It seems preferable to me to keep instruction decoding
between ARM and Thumb fully separate, even if this means we have a small
amount of duplication.

> -copy_unmodified (struct gdbarch *gdbarch, uint32_t insn,
> -		 const char *iname, struct displaced_step_closure *dsc)
> +arm_copy_unmodified (uint32_t insn, const char *iname,
> +		     struct displaced_step_closure *dsc)

Again for symmetry reasons, it's probably better if *all* copy_ functions
get the gdbarch, even if they currently don't need it.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com