Re: [ping 2] [RFA][PATCH v4 0/5] Add TDB regset support

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

Andreas Arnez wrote:

> Would it help to merge the iterator function with the logic of
> ppc_regset_from_core_section(), like outlined in the patch below?
> 
> I see the following advantages:
> 
> - One less gdbarch function needed: gdbarch_regset_from_core_section
>   becomes obsolete.
> 
> - The regset description is more "self-contained": it now includes the
>   name, description, and size.
> 
> - Even less code than the previous version, all condensed in a single
>   function.

In general, I agree it would be nice to merge the information from
core_regset_section into struct regset itself, for the reasons you
mention.

However, one reason to keep them distinct in the past has been that
many platforms do not actually provide a core_regset_section, only
a struct regset; in particular, the sizes of the regsets are not
there.  These are the platforms that fall back to sizeof (gregset)
etc. for native code and do not support cross-corefile debugging
and/or generation.

It has always been a goal to provide core_regset_section info for
all platforms and get rid of the various legacy methods, but so
far this has not been done.  As usual, one of the problem is that
you'd need to test on those platforms, which may be hard to do.


Your patch below doesn't show the common code changes, so I'm not
sure how you planned to handle legacy platforms.  I guess it might
be possible to detect them using zero markers in those fields ...


As an aside, I'm wondering:

> +  res = cb (tdep->wordsize == 4 ?
> +	    &ppc32_linux_gregset : &ppc64_linux_gregset,
> +	    cb_data);
> +  if (!res)
> +    res = cb (&ppc32_linux_fpregset, cb_data);
> +  if (!res && have_altivec)
> +    res = cb (&ppc32_linux_vrregset, cb_data);
> +  if (!res && have_vsx)
> +    cb (&ppc32_linux_vsxregset, cb_data);

Why does the callback need to return a flag that has to be handled
by the caller?   If there is indeed a requirement for treating
error conditions specially, couldn't the callback store error data
in the cb_data and handle it on subsequent calls?

This would make the gdbarch implementations in the targets yet
easier and simpler to write, something along the lines of:

  if (tdep->wordsize == 4)
    cb (&ppc32_linux_gregset, cb_data);
  else
    cb (&ppc64_linux_gregset, cb_data);
  cb (&ppc32_linux_fpregset, cb_data);
  if (have_altivec)
    cb (&ppc32_linux_vrregset, cb_data);
  if (have_vsx)
    cb (&ppc32_linux_vsxregset, cb_data);

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com