Re: [RFC] Add support for PPC Altivec registers in gcore

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

Carlos Eduardo Seo wrote:

> Here are the patches revised. Testsuite runs show no regressions in ppc 
> and i386.

Thanks for making those changes.

> 2008-05-14  Carlos Eduardo Seo  <cseo@linux.vnet.ibm.com>
> 
> 	* gdbarch.sh: Added new gdbarch struct
> 	core_regset_sections.
> 	* gdbarch.c: Refreshed.
> 	* gdbarch.h: Refreshed.
> 	* regset.h (core_regset_section): Declared.
> 	* linux-nat.c (linux_nat_do_thread_registers): Added
> 	support to the new gdbarch struct core_regset_sections.

This should read "Added support for ...", not "to ...".

> +  /* The loop below uses the new struct core_regset_section, which stores
> +     the supported section names and sizes for the core file. Note that
Two spaces after '.' here as well, please.

> +     note PRSTATUS needs to be treated specially. But the other notes are
> +     structurally the same, so they can benefit from the new struct.  */
> +  if (core_regset_p && sect_list != NULL)
> +    while (sect_list->sect_name != NULL)
> +      {
> +	/* .reg was already handled above.  */
> +	if (strcmp (sect_list->sect_name, ".reg") == 0)
> +	  {
> +	    sect_list++;
> +	    continue;
> +	  }
> +	regset = gdbarch_regset_from_core_section (gdbarch,
> +						   sect_list->sect_name,
> +						   sect_list->size);

Please add an assertion
      gdb_assert (regset && regset->collect_regset)
here, so we don't just crash on the call below if a gdbarch doesn't
provide those functions as it should ...

> +	gdb_regset = xmalloc (sect_list->size);
> +	regset->collect_regset (regset, regcache, -1,
> +				gdb_regset, sect_list->size);
> +	note_data = (char *) elfcore_write_register_note (obfd,
> +							  note_data,
> +							  note_size,
> +							  sect_list->sect_name,
> +							  gdb_regset,
> +							  sect_list->size);
> +	xfree (gdb_regset);
> +	sect_list++;
> +      }
> +
> +  /* For architectures that does not have the struct core_regset_section
> +     implemented, we use the old method. When all the architectures have
Two spaces after '.'.


With these final changes, the patch is OK.

Thanks,
Ulrich

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