Re: [PATCH] monitor_ops regnames array

[Andrew Cagney <ac131313@cygnus.com>]

> 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