Re: [patch] gdbserver regcache fetch all regs

[Aleksandar Ristovski <aristovski@qnx.com>]

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

Daniel Jacobowitz wrote:
> On Fri, Jun 19, 2009 at 08:35:13AM -0700, Doug Evans wrote:
>> Hi.  At first I thought "Yikes!". :-)
>> But it turns out that all fetch_registers routines treat 0 and -1 equivalently.
>> I wouldn't hold up this patch (it's fine with me fwiw), though I would
>> change the ChangeLog entry to something like: "Use -1 instead of 0 to
>> fetch all registers." since passing 0 does actually fetch all
>> registers.
> 
> Agreed on all counts.
> 
>> It would be good to remove this oddity and stop the conflation of 0
>> and -1, but I don't know what would break.
>> We could run the testsuite and see what happens as a start.
>> Does anyone know the history behind this?
> 
> Pretty sure nothing will break - since nothing else calls these
> functions.  I could be mistaken though.
> 

Ok, this would be a more complete patch (with cleanups in 
spots where -1 and 0 are used interchangeably).

If you think it would be more appropriate to first commit 
regcache.c change only, and then the cleanup, let me know.




ChangeLog:

* linux-low.c (usr_fetch_inferior_registers): Remove check 
for regno 0.
* proc-service.c (ps_lgetregs): Pass -1 to fetch all registers.
* regcache.c (get_regcache): Likewise.
* spu-low.c (spu_fetch_registers): Remove 0 to -1 conversion.
* win32-low.c (child_fetch_inferior_registers): Remove check 
for regno 0.


-- 
Aleksandar Ristovski
QNX Software Systems


ChangeLog:

* linux-low.c (usr_fetch_inferior_registers): Remove check 
for regno 0.
* proc-service.c (ps_lgetregs): Pass -1 to fetch all registers.
* regcache.c (get_regcache): Likewise.
* spu-low.c (spu_fetch_registers): Remove 0 to -1 conversion.
* win32-low.c (child_fetch_inferior_registers): Remove check 
for regno 0.

[-- Attachment #2: gdbserver-regcache.c-fetchallregs-20090619.diff --]
[-- Type: text/x-patch, Size: 2842 bytes --]

Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.105
diff -u -p -r1.105 linux-low.c
--- linux-low.c	24 May 2009 17:44:19 -0000	1.105
+++ linux-low.c	19 Jun 2009 15:56:05 -0000
@@ -2054,7 +2054,7 @@ error_exit:;
 static void
 usr_fetch_inferior_registers (int regno)
 {
-  if (regno == -1 || regno == 0)
+  if (regno == -1)
     for (regno = 0; regno < the_low_target.num_regs; regno++)
       fetch_register (regno);
   else
Index: proc-service.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/proc-service.c,v
retrieving revision 1.12
diff -u -p -r1.12 proc-service.c
--- proc-service.c	1 Apr 2009 22:50:24 -0000	1.12
+++ proc-service.c	19 Jun 2009 15:56:05 -0000
@@ -110,7 +110,7 @@ ps_lgetregs (gdb_ps_prochandle_t ph, lwp
   save_inferior = current_inferior;
   current_inferior = reg_inferior;
 
-  the_target->fetch_registers (0);
+  the_target->fetch_registers (-1);
   gregset_info()->fill_function (gregset);
 
   current_inferior = save_inferior;
Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/regcache.c,v
retrieving revision 1.19
diff -u -p -r1.19 regcache.c
--- regcache.c	22 Mar 2009 23:57:10 -0000	1.19
+++ regcache.c	19 Jun 2009 15:56:05 -0000
@@ -53,7 +53,7 @@ get_regcache (struct thread_info *inf, i
   /* FIXME - fetch registers for INF */
   if (fetch && regcache->registers_valid == 0)
     {
-      fetch_inferior_registers (0);
+      fetch_inferior_registers (-1);
       regcache->registers_valid = 1;
     }
 
Index: spu-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/spu-low.c,v
retrieving revision 1.24
diff -u -p -r1.24 spu-low.c
--- spu-low.c	3 Apr 2009 14:38:39 -0000	1.24
+++ spu-low.c	19 Jun 2009 15:56:06 -0000
@@ -471,10 +471,6 @@ spu_fetch_registers (int regno)
   int fd;
   CORE_ADDR addr;
 
-  /* ??? Some callers use 0 to mean all registers.  */
-  if (regno == 0)
-    regno = -1;
-
   /* We must be stopped on a spu_run system call.  */
   if (!parse_spufs_run (&fd, &addr))
     return;
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.35
diff -u -p -r1.35 win32-low.c
--- win32-low.c	1 Apr 2009 22:50:24 -0000	1.35
+++ win32-low.c	19 Jun 2009 15:56:06 -0000
@@ -331,7 +331,7 @@ child_fetch_inferior_registers (int r)
 {
   int regno;
   win32_thread_info *th = thread_rec (current_inferior_ptid (), TRUE);
-  if (r == -1 || r == 0 || r > NUM_REGS)
+  if (r == -1 || r > NUM_REGS)
     child_fetch_inferior_registers (NUM_REGS);
   else
     for (regno = 0; regno < r; regno++)