Re: [PATCH] Introduce gdbarch_num_cooked_regs

[Kevin Buettner <kevinb@redhat.com>]

Hi Simon,

On Sun, 21 Oct 2018 15:01:33 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> From: Simon Marchi <simon.marchi@ericsson.com>
> 
> This was part of another patchset I'm working on, but I thought it could
> be a good cleanup on its own.
> 
> The expression
> 
>   gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch)
> 
> is used quite often to find the number of cooked registers (raw + pseudo
> registers).  This patch introduces gdbarch_num_cooked_regs, which does
> the equivalent.  It substantially reduces required wrapping in some
> places, so should improve readability.

LGTM, aside from a question and one formatting problem...

> diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
> index 1490ee28668a..b6e8f00a0ba1 100644
> --- a/gdb/m68hc11-tdep.c
> +++ b/gdb/m68hc11-tdep.c
> @@ -854,10 +854,7 @@ m68hc11_frame_unwind_cache (struct frame_info *this_frame,
>  
>    /* Adjust all the saved registers so that they contain addresses and not
>       offsets.  */
> -  for (i = 0;
> -       i < gdbarch_num_regs (gdbarch)
> -	   + gdbarch_num_pseudo_regs (gdbarch) - 1;
> -       i++)
> +  for (i = 0; i < gdbarch_num_cooked_regs (gdbarch); i++)
>      if (trad_frame_addr_p (info->saved_regs, i))
>        {
>          info->saved_regs[i].addr += this_base;

The " - 1" in the original expression was a mistake, right?  (I spent
a few minutes looking at the mc68hc11's pseudo register layout but can't
find a reason for subtracting one.)

> diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
> index 12382cddb357..9c34a070ae14 100644
> --- a/gdb/tui/tui-regs.c
> +++ b/gdb/tui/tui-regs.c
> @@ -206,10 +206,7 @@ tui_show_register_group (struct reggroup *group,
>  
>    /* See how many registers must be displayed.  */
>    nr_regs = 0;
> -  for (regnum = 0;
> -       regnum < gdbarch_num_regs (gdbarch)
> -		+ gdbarch_num_pseudo_regs (gdbarch);
> -       regnum++)
> +  for (regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); regnum++)
>      {
>        const char *name;
>  
> @@ -253,10 +250,7 @@ tui_show_register_group (struct reggroup *group,
>  
>        /* Now set the register names and values.  */
>        pos = 0;
> -      for (regnum = 0;
> -	   regnum < gdbarch_num_regs (gdbarch)
> -		    + gdbarch_num_pseudo_regs (gdbarch);
> -	   regnum++)
> +      for (regnum = 0;regnum < gdbarch_num_cooked_regs (gdbarch); regnum++)

Missing space between ; and "regnum".

Kevin