Re: [PATCH V2 3/5] Use linux_get_siginfo_type_with_fields for x86.

[Pedro Alves <palves@redhat.com>]

On 12/17/2015 04:56 PM, Walfred Tedeschi wrote:
> Using new functions introduced for siginfo for i386/amd64.
> 
> 
> 2015-12-15  Walfred Tedeschi  <walfred.tedeschi@intel.com>
> 
> 	* linux-tdep.h (linux_get_siginfo_type_with_fields): Making
> 	function linux_get_siginfo_type_with_fields public.
> 	* linux-tdep.c (linux_get_siginfo_type_with_fields): Making
> 	function linux_get_siginfo_type_with_fields public.

Use imperative: "Use", "Make" (here and elsewhere, including subject
line of patch #1.  Drop periods in subject lines while at it).

s/public/extern/.


> 	* i386-linux-tdep.h (x86_get_siginfo_type_with_fields): New
> 	function.
> 	* amd64-linux-tdep.c (amd64_linux_init_abi_common): Add
> 	x86_get_siginfo_type_with_fields for the amd64 abi.

	(amd64_linux_init_abi_common): Register as gdbarch_get_siginfo_type
	method.

> 	* i386-linux-tdep.c (x86_get_siginfo_type_with_fields): New
> 	Function.

lowercase "function".

> 	(i386_linux_init_abi): Add new function at the i386 ABI
> 	initialization.

	(i386_linux_init_abi): Register as gdbarch_get_siginfo_type
	method.


> 
> ---
>  gdb/amd64-linux-tdep.c | 2 ++
>  gdb/i386-linux-tdep.c  | 8 ++++++++
>  gdb/i386-linux-tdep.h  | 3 +++
>  gdb/linux-tdep.c       | 2 +-
>  gdb/linux-tdep.h       | 5 +++++
>  5 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index ee21635..bde751f 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1838,6 +1838,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
>  
>    set_gdbarch_process_record (gdbarch, i386_process_record);
>    set_gdbarch_process_record_signal (gdbarch, amd64_linux_record_signal);
> +
> +  set_gdbarch_get_siginfo_type (gdbarch, x86_get_siginfo_type_with_fields);
>  }
>  
>  static void
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 86fe09e..766f193 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -656,6 +656,12 @@ i386_linux_supply_xstateregset (const struct regset *regset,
>    i387_supply_xsave (regcache, regnum, xstateregs);
>  }
>  
> +struct type *
> +x86_get_siginfo_type_with_fields (struct gdbarch *gdbarch)

"with_fields" is implementation detail.  I'd rather just:

struct type *
x86_get_siginfo_type (struct gdbarch *gdbarch)


> +{
> +  return linux_get_siginfo_type_with_fields (gdbarch, LINUX_SIGINFO_FIELD_ADDR_BND);
> +}
> +
>  /* Similar to i386_collect_fpregset, but use XSAVE extended state.  */
>  
>  static void
> @@ -994,6 +1000,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_I386);
>    set_gdbarch_get_syscall_number (gdbarch,
>                                    i386_linux_get_syscall_number);
> +
> +  set_gdbarch_get_siginfo_type (gdbarch, x86_get_siginfo_type_with_fields);
>  }
>  
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
> diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
> index 5ac08d3..f42bf7a 100644
> --- a/gdb/i386-linux-tdep.h
> +++ b/gdb/i386-linux-tdep.h
> @@ -72,4 +72,7 @@ extern struct target_desc *tdesc_i386_avx512_linux;
>  
>  extern int i386_linux_gregset_reg_offset[];
>  
> +/* Returns x86 siginfo type with extra fields.  */

/* Return the x86 siginfo type.  */

> +extern struct type *x86_get_siginfo_type_with_fields (struct gdbarch *gdbarch);
> +
>  #endif /* i386-linux-tdep.h */
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index a7e85c5..e41b6f4 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -246,7 +246,7 @@ get_linux_inferior_data (void)
>  /* This function is suitable for architectures that
>     extend/override the standard siginfo in an specific way.  */
>  

Update comment:

/* See linux-tdep.h.  */

> -static struct type *
> +struct type *
>  linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
>  				    linux_siginfo_extra_fields extra_fields)
>  {
> diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
> index c0c9d91..7592259 100644
> --- a/gdb/linux-tdep.h
> +++ b/gdb/linux-tdep.h
> @@ -33,6 +33,11 @@ enum linux_siginfo_extra_field_values
>  /* Defines a type for the values defined in linux_siginfo_extra_field_values.  */
>  DEF_ENUM_FLAGS_TYPE (enum linux_siginfo_extra_field_values, linux_siginfo_extra_fields);
>  
> +/* Return the siginfo type with additional fields.  */
> +struct type *
> +linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch,
> +				    linux_siginfo_extra_fields fields);
> +

/* Return a new siginfo type with additional fields specified by
   FIELDS.  */

struct type *linux_get_siginfo_type_with_fields
  (struct gdbarch *gdbarch,
   linux_siginfo_extra_fields fields);


>  typedef char *(*linux_collect_thread_registers_ftype) (const struct regcache *,
>  						       ptid_t,
>  						       bfd *, char *, int *,
> 


Thanks,
Pedro Alves