[PATCH] monitor_ops regnames array

[PATCH] monitor_ops regnames array

[Grace Sainsbury]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

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.
I only changed the rom files that are compiled for the m68k, but the
array is still there.

ok to commit?

grace

2002-06-25  Grace Sainsbury  <graces@redhat.com>
	    
	    * monitor.h: Add the function regname to monitor_ops
	    structure. This way NUM_REGS does not have to be a
	    constant.
	    * monitor.c (monitor_fetch_register): Added support for
	    regname
	    function. The function is called if the array regnames is
	    NULL.
	    (monitor_store_register): Same.
	    * cpu32bug-rom.c (cpu32bug_regname):  Add
	    function. Replaces regnames array.
	    (init_cpu32bug_cmds): set cpu32bug_cmds.regnames to NULL,
	    cpu32bug_cmds.regname to point to new function.
	    * abug-rom.c (abug_regname): Same as above.
	    (init_abug_cmds): Same.
	    * dbug-rom.c (dbug_regname): Same as above.
	    (init_dbug_cmds): Same.
	    * remote-est.c (est_regname): Same. 
	    (init_est_cmds): Same.
	    * rom68k-rom.c (rom68k_regname): Same. 
	    (init_rom68k_cmds): Same.

[-- Attachment #2: newpatch.patch --]
[-- Type: text/plain, Size: 8725 bytes --]

Index: abug-rom.c
===================================================================
RCS file: /cvs/src/src/gdb/abug-rom.c,v
retrieving revision 1.5
diff -u -r1.5 abug-rom.c
--- abug-rom.c	6 Mar 2001 08:21:05 -0000	1.5
+++ abug-rom.c	25 Jun 2002 20:30:07 -0000
@@ -76,12 +76,22 @@
  * registers either. So, typing "info reg sp" becomes an "A7".
  */
 
-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",
+  };
+
+  if ((index >= sizeof (regnames)) || (index < 0)
+      || index >= NUM_REGS)
+    return NULL;
+  else
+    return regnames[index];
+}
 
 /*
  * Define the monitor command strings. Since these are passed directly
@@ -141,7 +151,8 @@
   abug_cmds.cmd_end = NULL;	/* optional command terminator */
   abug_cmds.target = &abug_ops;	/* target operations */
   abug_cmds.stopbits = SERIAL_1_STOPBITS;	/* number of stop bits */
-  abug_cmds.regnames = abug_regnames;	/* registers names */
+  abug_cmds.regnames = NULL;	/* registers names */
+  abug_cmds.regname = abug_regname;
   abug_cmds.magic = MONITOR_OPS_MAGIC;	/* magic */
 };
 
Index: cpu32bug-rom.c
===================================================================
RCS file: /cvs/src/src/gdb/cpu32bug-rom.c,v
retrieving revision 1.5
diff -u -r1.5 cpu32bug-rom.c
--- cpu32bug-rom.c	6 Mar 2001 08:21:06 -0000	1.5
+++ cpu32bug-rom.c	25 Jun 2002 20:30:07 -0000
@@ -74,12 +74,22 @@
  * registers either. So, typing "info reg sp" becomes an "A7".
  */
 
-static char *cpu32bug_regnames[NUM_REGS] =
+static char *
+cpu32bug_regname (int index)
 {
-  "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
-  "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
-  "SR", "PC",
-};
+  static char *regnames[] =
+  {
+    "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
+    "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
+    "SR", "PC"
+  };
+
+  if ((index >= sizeof (regnames)) || (index < 0)
+      || index >= NUM_REGS)
+    return NULL;
+  else
+    return regnames[index];
+}
 
 /*
  * Define the monitor command strings. Since these are passed directly
@@ -139,7 +149,8 @@
   cpu32bug_cmds.cmd_end = NULL;	/* optional command terminator */
   cpu32bug_cmds.target = &cpu32bug_ops;		/* target operations */
   cpu32bug_cmds.stopbits = SERIAL_1_STOPBITS;	/* number of stop bits */
-  cpu32bug_cmds.regnames = cpu32bug_regnames;	/* registers names */
+  cpu32bug_cmds.regnames = NULL;	/* registers names */
+  cpu32bug_cmds.regname = cpu32bug_regname;
   cpu32bug_cmds.magic = MONITOR_OPS_MAGIC;	/* magic */
 };				/* init_cpu32bug_cmds */
 
Index: dbug-rom.c
===================================================================
RCS file: /cvs/src/src/gdb/dbug-rom.c,v
retrieving revision 1.5
diff -u -r1.5 dbug-rom.c
--- dbug-rom.c	6 Mar 2001 08:21:06 -0000	1.5
+++ dbug-rom.c	25 Jun 2002 20:30:07 -0000
@@ -76,13 +76,24 @@
    different names than GDB does, and don't support all the registers
    either. So, typing "info reg sp" becomes an "A7". */
 
-static char *dbug_regnames[NUM_REGS] =
+static char *
+dbug_regname (int index)
 {
-  "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
-  "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
-  "SR", "PC"
-  /* no float registers */
-};
+  static char *regnames[] =
+  {
+    "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
+    "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
+    "SR", "PC"
+    /* no float registers */
+  };
+
+  if ((index >= sizeof (regnames)) || (index < 0)
+      || index >= NUM_REGS)
+    return NULL;
+  else
+    return regnames[index];
+}
+
 static struct target_ops dbug_ops;
 static struct monitor_ops dbug_cmds;
 
@@ -135,7 +146,8 @@
   dbug_cmds.cmd_end = NULL;	/* optional command terminator */
   dbug_cmds.target = &dbug_ops;	/* target operations */
   dbug_cmds.stopbits = SERIAL_1_STOPBITS;	/* number of stop bits */
-  dbug_cmds.regnames = dbug_regnames;	/* registers names */
+  dbug_cmds.regnames = NULL;	/* registers names */
+  dbug_cmds.regname = dbug_regname;
   dbug_cmds.magic = MONITOR_OPS_MAGIC;	/* magic */
 }				/* init_debug_ops */
 
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)
+    name = current_monitor->regnames[regno];
+  else
+    name = current_monitor->regname (regno);
   monitor_debug ("MON fetchreg %d '%s'\n", regno, name ? name : "(null name)");
 
   if (!name || (*name == '\0'))
@@ -1336,7 +1339,10 @@
   char *name;
   ULONGEST val;
 
-  name = current_monitor->regnames[regno];
+  if (current_monitor->regnames)
+    name = current_monitor->regnames[regno];
+  else
+    name = current_monitor->regname (regno);
   if (!name || (*name == '\0'))
     {
       monitor_debug ("MON Cannot store unknown register\n");
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 */
     char **regnames;		/* array of register names in ascii */
+    char *(*regname) (int index); /* function for dynamic regname array */
     int num_breakpoints;	/* If set_break != NULL, number of supported
 				   breakpoints */
     int magic;			/* Check value */
Index: remote-est.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-est.c,v
retrieving revision 1.5
diff -u -r1.5 remote-est.c
--- remote-est.c	6 Mar 2001 08:21:12 -0000	1.5
+++ remote-est.c	25 Jun 2002 20:30:08 -0000
@@ -76,12 +76,23 @@
  * registers either. So, typing "info reg sp" becomes a "r30".
  */
 
-static char *est_regnames[NUM_REGS] =
+static char *
+est_regname (int index) 
 {
-  "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
-  "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
-  "SR", "PC",
-};
+  
+  static char *regnames[] =
+  {
+    "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
+    "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7",
+    "SR", "PC",
+  };
+  
+  if ((index >= sizeof (regnames)) || (index < 0)
+      || index >= NUM_REGS)
+    return NULL;
+  else
+    return regnames[index];
+}
 
 /*
  * Define the monitor command strings. Since these are passed directly
@@ -143,7 +154,8 @@
   est_cmds.cmd_end = NULL;	/* optional command terminator */
   est_cmds.target = &est_ops;	/* target operations */
   est_cmds.stopbits = SERIAL_1_STOPBITS;	/* number of stop bits */
-  est_cmds.regnames = est_regnames;	/* registers names */
+  est_cmds.regnames = NULL;
+  est_cmds.regname = est_regname; /*register names*/
   est_cmds.magic = MONITOR_OPS_MAGIC;	/* magic */
 }				/* init_est_cmds */
 
Index: rom68k-rom.c
===================================================================
RCS file: /cvs/src/src/gdb/rom68k-rom.c,v
retrieving revision 1.7
diff -u -r1.7 rom68k-rom.c
--- rom68k-rom.c	10 Sep 2001 23:54:16 -0000	1.7
+++ rom68k-rom.c	25 Jun 2002 20:30:08 -0000
@@ -157,11 +157,23 @@
    than does GDB, and don't necessarily support all the registers
    either. So, typing "info reg sp" becomes a "r30".  */
 
-static char *rom68k_regnames[NUM_REGS] =
+static char *
+rom68k_regname (int index) 
 {
-  "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
-  "A0", "A1", "A2", "A3", "A4", "A5", "A6", "ISP",
-  "SR", "PC"};
+
+  static char *regnames[] =
+  {
+    "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7",
+    "A0", "A1", "A2", "A3", "A4", "A5", "A6", "ISP",
+    "SR", "PC"
+  };
+  
+  if ((index >= sizeof (regnames)) || (index < 0)
+      || index >= NUM_REGS)
+    return NULL;
+  else
+    return regnames[index];
+}
 
 /* Define the monitor command strings. Since these are passed directly
    through to a printf style function, we may include formatting
@@ -220,7 +232,8 @@
   rom68k_cmds.cmd_end = ".\r";
   rom68k_cmds.target = &rom68k_ops;
   rom68k_cmds.stopbits = SERIAL_1_STOPBITS;
-  rom68k_cmds.regnames = rom68k_regnames;
+  rom68k_cmds.regnames = NULL;
+  rom68k_cmds.regname = rom68k_regname;
   rom68k_cmds.magic = MONITOR_OPS_MAGIC;
 }				/* init_rom68k_cmds */

Re: [PATCH] monitor_ops regnames array

[Andrew Cagney]

> 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

Re: [PATCH] monitor_ops regnames array

[Grace Sainsbury]

On Tue, Jun 25, 2002 at 11:10:42PM -0400, Andrew Cagney wrote:
> > 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:
> 

I made the changes and committed.

grace