[RFC] Eliminate use of deprecated_register_bytes() from remote.c

[RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Kevin Buettner]

This one seems almost obvious, but I'd like someone else to look it
over before I check it in...

	* remote.c (init_remote_state): Eliminate use of
	deprecated_register_bytes().

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.184
diff -u -p -r1.184 remote.c
--- remote.c	15 Apr 2005 21:16:09 -0000	1.184
+++ remote.c	15 Apr 2005 21:25:00 -0000
@@ -248,10 +248,7 @@ init_remote_state (struct gdbarch *gdbar
   int regnum;
   struct remote_state *rs = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct remote_state);
 
-  if (deprecated_register_bytes () != 0)
-    rs->sizeof_g_packet = deprecated_register_bytes ();
-  else
-    rs->sizeof_g_packet = 0;
+  rs->sizeof_g_packet = 0;
 
   /* Assume a 1:1 regnum<->pnum table.  */
   rs->regs = GDBARCH_OBSTACK_CALLOC (gdbarch, NUM_REGS + NUM_PSEUDO_REGS,
@@ -266,8 +263,7 @@ init_remote_state (struct gdbarch *gdbar
       /* ...name = REGISTER_NAME (regnum); */
 
       /* Compute packet size by accumulating the size of all registers.  */
-      if (deprecated_register_bytes () == 0)
-        rs->sizeof_g_packet += register_size (current_gdbarch, regnum);
+      rs->sizeof_g_packet += register_size (current_gdbarch, regnum);
     }
 
   /* Default maximum number of characters in a packet body. Many

Re: [RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Daniel Jacobowitz]

On Fri, Apr 15, 2005 at 02:32:45PM -0700, Kevin Buettner wrote:
> This one seems almost obvious, but I'd like someone else to look it
> over before I check it in...
> 
> 	* remote.c (init_remote_state): Eliminate use of
> 	deprecated_register_bytes().

I'm pretty sure you need an if (regnum < NUM_REGS) on the second piece.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Kevin Buettner]

On Fri, 15 Apr 2005 17:37:58 -0400
Daniel Jacobowitz <drow@false.org> wrote:

> On Fri, Apr 15, 2005 at 02:32:45PM -0700, Kevin Buettner wrote:
> > This one seems almost obvious, but I'd like someone else to look it
> > over before I check it in...
> > 
> > 	* remote.c (init_remote_state): Eliminate use of
> > 	deprecated_register_bytes().
> 
> I'm pretty sure you need an if (regnum < NUM_REGS) on the second piece.

I agree that that's the right thing to do, but I'm not convinced that
will give us behavior equivalent to what we have now.  Consider the
following comment/code from regcache.c:

  /* FIXME: cagney/2002-05-22: Should only need to allocate space for
     the raw registers.  Unfortunately some code still accesses the
     register array directly using the global registers[].  Until that
     code has been purged, play safe and over allocating the register
     buffer.  Ulgh!  */
  descr->sizeof_raw_registers = descr->sizeof_cooked_registers;

The ``sizeof_raw_registers'' field is the value returned by
deprecated_register_bytes().  So, it seems to me that we're presently
setting rs->sizeof_g_packet to a value that's potentially larger than
need be.

I can think of no reason to continue to over-allocate space for g/G
packets in remote.c so I'm happy with this patch instead (which adds
the test that you wanted):

	* remote.c (init_remote_state): Eliminate use of
	deprecated_register_bytes().

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.184
diff -u -p -r1.184 remote.c
--- remote.c	15 Apr 2005 21:16:09 -0000	1.184
+++ remote.c	15 Apr 2005 22:53:38 -0000
@@ -248,10 +248,7 @@ init_remote_state (struct gdbarch *gdbar
   int regnum;
   struct remote_state *rs = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct remote_state);
 
-  if (deprecated_register_bytes () != 0)
-    rs->sizeof_g_packet = deprecated_register_bytes ();
-  else
-    rs->sizeof_g_packet = 0;
+  rs->sizeof_g_packet = 0;
 
   /* Assume a 1:1 regnum<->pnum table.  */
   rs->regs = GDBARCH_OBSTACK_CALLOC (gdbarch, NUM_REGS + NUM_PSEUDO_REGS,
@@ -266,8 +263,8 @@ init_remote_state (struct gdbarch *gdbar
       /* ...name = REGISTER_NAME (regnum); */
 
       /* Compute packet size by accumulating the size of all registers.  */
-      if (deprecated_register_bytes () == 0)
-        rs->sizeof_g_packet += register_size (current_gdbarch, regnum);
+      if (regnum < NUM_REGS)
+	rs->sizeof_g_packet += register_size (current_gdbarch, regnum);
     }
 
   /* Default maximum number of characters in a packet body. Many

Re: [RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Daniel Jacobowitz]

On Fri, Apr 15, 2005 at 04:12:12PM -0700, Kevin Buettner wrote:
> On Fri, 15 Apr 2005 17:37:58 -0400
> Daniel Jacobowitz <drow@false.org> wrote:
> 
> > On Fri, Apr 15, 2005 at 02:32:45PM -0700, Kevin Buettner wrote:
> > > This one seems almost obvious, but I'd like someone else to look it
> > > over before I check it in...
> > > 
> > > 	* remote.c (init_remote_state): Eliminate use of
> > > 	deprecated_register_bytes().
> > 
> > I'm pretty sure you need an if (regnum < NUM_REGS) on the second piece.
> 
> I agree that that's the right thing to do, but I'm not convinced that
> will give us behavior equivalent to what we have now.  Consider the
> following comment/code from regcache.c:
> 
>   /* FIXME: cagney/2002-05-22: Should only need to allocate space for
>      the raw registers.  Unfortunately some code still accesses the
>      register array directly using the global registers[].  Until that
>      code has been purged, play safe and over allocating the register
>      buffer.  Ulgh!  */
>   descr->sizeof_raw_registers = descr->sizeof_cooked_registers;
> 
> The ``sizeof_raw_registers'' field is the value returned by
> deprecated_register_bytes().  So, it seems to me that we're presently
> setting rs->sizeof_g_packet to a value that's potentially larger than
> need be.

You are definitely right.  That's an Ulgh indeed.

> I can think of no reason to continue to over-allocate space for g/G
> packets in remote.c so I'm happy with this patch instead (which adds
> the test that you wanted):
> 
> 	* remote.c (init_remote_state): Eliminate use of
> 	deprecated_register_bytes().

This is fine with me.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Kevin Buettner]

On Wed, 27 Apr 2005 11:40:18 -0400
Daniel Jacobowitz <drow@false.org> wrote:

> > I can think of no reason to continue to over-allocate space for g/G
> > packets in remote.c so I'm happy with this patch instead (which adds
> > the test that you wanted):
> > 
> > 	* remote.c (init_remote_state): Eliminate use of
> > 	deprecated_register_bytes().
> 
> This is fine with me.

Okay, committed.  Thanks for reviewing this.

Kevin

Re: [RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Michael Snyder]

Kevin Buettner wrote:

> I can think of no reason to continue to over-allocate space for g/G
> packets in remote.c 

What if the target returns a longer string?

Re: [RFC] Eliminate use of deprecated_register_bytes() from remote.c

[Kevin Buettner]

On Fri, 29 Apr 2005 15:59:21 -0700
Michael Snyder <msnyder@redhat.com> wrote:

> Kevin Buettner wrote:
> 
> > I can think of no reason to continue to over-allocate space for g/G
> > packets in remote.c 
> 
> What if the target returns a longer string?

GDB will print a "Remote packet too long" message.

Do you know of any stubs which return a "g" packet containing pseudo
registers?

Kevin