Re: [RFC] patch to refactor ppc64 specific code from ppc-linux-tdep

[Pedro Alves <palves@redhat.com>]

On 01/26/2013 04:24 PM, Andreas Tobler wrote:
> Hi Pedro,

Hi Andreas.

>> > Also, GDB's convention is that the function name goes on the first
>> > column in function definitions only, not declarations, and that
>> > declarations in .h files get an explicit "extern".
> Done, not sure if I need the extern on ppc64-tdep.h too?

Yes.

> 2013-01-26  Andreas Tobler  <andreast@fgznet.ch>
> 
> 	* Makefile.in (ALL_TARGET_OBS): Add new file ppc64-tdep.o.
> 	(HFILES_NO_SRCDIR): Likewise.
> 	(ALLDEPFILES): Likewise.

 	* Makefile.in (ALL_TARGET_OBS): Add ppc64-tdep.o.
 	(HFILES_NO_SRCDIR): Add ppc64-tdep.h.
 	(ALLDEPFILES): Add ppc64-tdep.c.


> 	* configure.tgt: Add new file for powerpc-linux.

	* configure.tgt (powerpc-*-linux* | powerpc64-*-linux*): Add ppc64-tdep.o
        to gdb_target_obs.

> 	* ppc64-tdep.h: New file.
> 	* ppc64-tdep.c: New file.
> 	(insn_d, insn_ds, insn_xfx, ppc64_desc_entry_point): Move from
> 	ppc-linux-tdep.c to here.
> 	(PPC64_STANDARD_LINKAGE1_LEN, PPC64_STANDARD_LINKAGE2_LEN)
> 	(PPC64_STANDARD_LINKAGE2_LEN): Likewise and use ARRAY_SIZE macro.
> 	(ppc64_standard_linkage1_target, ppc64_standard_linkage2_target)
> 	(ppc64_standard_linkage3_target, ppc64_skip_trampoline_code): Move
> 	from ppc-linux-tdep.c to here.
> 	(ppc64_convert_from_func_ptr_addr): Rename it from
> 	ppc64_linux_convert_from_func_ptr_addr to
> 	ppc64_convert_from_func_ptr_addr and move it from ppc-linux-tdep.c to
> 	here.

Drop the "it"s.

> 	* rs6000-tdep.c:
> 	(read_insn): Move from ppc-linux-tdep.c to here.
> 	(insns_match_pattern, insn_d_field, insn_ds_field): Move
> 	from ppc-linux-tdep.c to here and rename them with the ppc_ prefix.
> 	* ppc-linux-tdep.c: Include ppc64-tdep.h.
> 	Removed above functions.


> 	(ppc_linux_init_abi): Rename
> 	ppc64_linux_convert_from_func_ptr_addr to
> 	ppc64_convert_from_func_ptr_addr.

 	(ppc_linux_init_abi): Adjust.


> diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
> index 9be9666..b268e90 100644
> --- a/gdb/ppc-tdep.h
> +++ b/gdb/ppc-tdep.h
> @@ -291,6 +291,23 @@ enum {
>    PPC_NUM_REGS
>  };
>  
> +/* An instruction to match.  */
> +
> +struct ppc_insn_pattern
> +{
> +  unsigned int mask;            /* mask the insn with this...  */
> +  unsigned int data;            /* ...and see if it matches this.  */
> +  int optional;                 /* If non-zero, this insn may be absent.  */
> +};
> +
> +extern int
> +ppc_insns_match_pattern (CORE_ADDR pc, struct ppc_insn_pattern *pattern,
> +			 unsigned int *insn);
> +extern CORE_ADDR
> +ppc_insn_d_field (unsigned int insn);
> +
> +extern CORE_ADDR
> +ppc_insn_ds_field (unsigned int insn);

Here, GDB's convention is that the function name goes on the first
column in function definitions only, not declarations.  So:

extern int ppc_insns_match_pattern (CORE_ADDR pc,
				    struct ppc_insn_pattern *pattern,
				    unsigned int *insn);

extern CORE_ADDR ppc_insn_d_field (unsigned int insn);

extern CORE_ADDR ppc_insn_ds_field (unsigned int insn);


This is so 'grep ^functionname" finds function definitions.

>  
>  /* Instruction size.  */
>  #define PPC_INSN_SIZE 4
> diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
> new file mode 100644
> index 0000000..cdf8f4d
> --- /dev/null
> +++ b/gdb/ppc64-tdep.c
> @@ -0,0 +1,364 @@
> +/* Common target-dependent code for ppc64 GDB, the GNU debugger.
> +
> +   Copyright (C) 2013 Free Software
> +   Foundation, Inc.

This is existing code, not new code.  Please retain the copyright
the copyright years of the file(s) the code has been copied from.

This is okay with these issues fixed.

Thanks again for doing this.

-- 
Pedro Alves