Re: [PATCH] monitor_ops regnames array

Andrew Cagney
2002-06-26T03:11:21Z

> With multi-arch NUM_REGS is not a constant, but monitor.c was treating
> as a constant in determining the size of the numregs array. So I added
> a function to monitor_ops that returns the register names, and doesn't
> need NUM_REGS to be constant.

Outch.

> I only changed the rom files that are compiled for the m68k, but the
> array is still there.

Ok, I think having a functional interface will be better any way.

> ok to commit?

Yes, after the below tweaks:

> -static char *abug_regnames[NUM_REGS] =
> +static char *
> +abug_regname (int index)
> {
> - "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
> - "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
> - "PC",
> -};
> + static char *regnames[] =
> + {
> + "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
> + "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
> + "PC",
> + };

I think the below should be:

if (index >= (sizeof (regnames) / sizeof (regnames[0]))
|| ...

> + if ((index >= sizeof (regnames)) || (index < 0)
> + || index >= NUM_REGS)
> + return NULL;
> + else
> + return regnames[index];

> Index: monitor.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/monitor.c,v
> retrieving revision 1.33
> diff -u -r1.33 monitor.c
> --- monitor.c 18 Apr 2002 18:09:03 -0000 1.33
> +++ monitor.c 25 Jun 2002 20:30:08 -0000
> @@ -1183,7 +1183,10 @@
> zerobuf = alloca (MAX_REGISTER_RAW_SIZE);
> memset (zerobuf, 0, MAX_REGISTER_RAW_SIZE);
>
> - name = current_monitor->regnames[regno];
> + if (current_monitor->regnames)

Suggest testing regname first vis:

if (current_monitor->regname != NULL)

since that is the prefered interface. If, for some [strange] reason,
both were present I think the function should be prefered.

> + name = current_monitor->regnames[regno];
> + else
> + name = current_monitor->regname (regno);

> Index: monitor.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/monitor.h,v
> retrieving revision 1.8
> diff -u -r1.8 monitor.h
> --- monitor.h 21 Oct 2001 19:20:30 -0000 1.8
> +++ monitor.h 25 Jun 2002 20:30:08 -0000
> @@ -116,6 +116,7 @@
> struct target_ops *target; /* target operations */
> int stopbits; /* number of stop bits */

Suggest a comment mentioning that ``regnames'' is ``depreacated'' and
create a change-request [bug report] indicating that it should be zapped.

> char **regnames; /* array of register names in ascii */
> + char *(*regname) (int index); /* function for dynamic regname array */

Is it possible to have (*regname)() return a ``const char *''? The
fuctions would need updating.

(One day, GDB will be compiled with -Wwrite-strings and the above would
upset that).

enjoy,
Andrew