Re: [PATCH v3 1/2] Add new GDB command "maint print user-registers"

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

On Wed, Dec 10 2014, Doug Evans wrote:

> Nit: I realize the rest of the file uses "nr" but
> it's a horrible name.  I hadn't read user-regs.c
> in awhile, and was reading your patch absent that context.
> I read "nr" and think "number of registers",
> and that's the only thing that comes to mind
> as a possible interpretation.
>
> I'm not asking you to change any other uses,
> but can I ask that "nr" here be named "regnum" or some such.

Yes, I agree that "nr" is a bad name.  I will rename the variable to
"regnum" instead.

> In the column title you could use "Num" or some such.

Hm, that would be inconsistent with "maint print registers", which uses
"Nr" in the title as well.  IMHO all user-visible interfaces should use
the same title for this column.  Maybe we can change that in a separate
patch.

> Also, I think the loop would be more readable thusly:
>
> +  for (reg = regs->first; reg != NULL; reg = reg->next, regnum++)
>
> I have a slight preference for this instead:
>
> +  for (reg = regs->first; reg != NULL; reg = reg->next, ++regnum)
>
> but the rest of the file uses post-inc, so whatever.

Right, this reduces code and improves clarity.  Will change as
suggested.

Based on your suggestions, I will adjust the patch as indicated below.


-- >8 --
Subject: Style improvements for maintenance_print_user_registers

diff --git a/gdb/user-regs.c b/gdb/user-regs.c
index 84ecf3a..b70dd45 100644
--- a/gdb/user-regs.c
+++ b/gdb/user-regs.c
@@ -223,21 +223,18 @@ maintenance_print_user_registers (char *args, int from_tty)
   struct gdbarch *gdbarch;
   struct gdb_user_regs *regs;
   struct user_reg *reg;
-  int nr;
+  int regnum;
 
   if (!target_has_registers)
     error (_("The program has no registers now."));
 
   gdbarch = get_frame_arch (get_selected_frame (NULL));
   regs = gdbarch_data (gdbarch, user_regs_data);
-  nr = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
+  regnum = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
 
   fprintf_unfiltered (gdb_stdout, " Nr  Name\n");
-  for (reg = regs->first; reg != NULL; reg = reg->next)
-    {
-      fprintf_unfiltered (gdb_stdout, "%3d  %s\n", nr, reg->name);
-      nr++;
-    }
+  for (reg = regs->first; reg != NULL; reg = reg->next, ++regnum)
+    fprintf_unfiltered (gdb_stdout, "%3d  %s\n", regnum, reg->name);
 }
 
 extern initialize_file_ftype _initialize_user_regs; /* -Wmissing-prototypes */