This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH] remote protocol cleanups
- From: Nathan Sidwell <nathan at codesourcery dot com>
- To: Daniel Jacobowitz <drow at false dot org>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Tue, 08 Mar 2005 11:09:39 +0000
- Subject: Re: [PATCH] remote protocol cleanups
- Organization: Codesourcery LLC
- References: <4209F9CE.4030405@codesourcery.com> <20050307224517.GB28725@nevyn.them.org>
Daniel Jacobowitz wrote:
On Wed, Feb 09, 2005 at 11:53:50AM +0000, Nathan Sidwell wrote:
2) remote_write_bytes attempts to honour get_memory_write_packet_size's
limit, but fails in two ways when that is a very small number. a) it can
end up with a negative byte count. b) it deducts the number of chars in
the *maximal* length count, not the number of chars in the actual length
count. This can result in packets sending 1 or 2 bytes fewer than they
are limited to.
Mostly OK, but one question. I don't get the "max (todo, 1)". If no
bytes fit, aren't we hosed? It seems like an error condition; we
shouldn't be violating the size limit.
It appears the minimum size setting is somewhat arbitrary, and incorrect.
/* NOTE: 16 is just chosen at random. */
#ifndef MIN_REMOTE_PACKET_SIZE
#define MIN_REMOTE_PACKET_SIZE 16
#endif
the minumum packet size is actually
7 - strlen ("$M,:#NN")
8 - hexnumlen (memaddr) (32 bit host)
1 - hexnumlen (len)
2 - data
= 18 characters
This patch fixes that size calculation. ok?
nathan
--
Nathan Sidwell :: http://www.codesourcery.com :: CodeSourcery LLC
nathan@codesourcery.com :: http://www.planetfall.pwp.blueyonder.co.uk
2005-03-08 Nathan Sidwell <nathan@codesourcery.com>
* remote.c (MIN_REMOTE_PACKET_SIZE): Set to 20.
(remote_fetch_registers): Allow uppercase hex when resyncing.
(remote_write_bytes): Only call get_memory_write_packet_size once.
Robustify packet size calculation for very small packets. Check
that at least one byte will be written.
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.176
diff -c -3 -p -r1.176 remote.c
*** remote.c 4 Mar 2005 17:52:53 -0000 1.176
--- remote.c 8 Mar 2005 11:03:32 -0000
*************** get_memory_packet_size (struct memory_pa
*** 413,421 ****
#ifndef MAX_REMOTE_PACKET_SIZE
#define MAX_REMOTE_PACKET_SIZE 16384
#endif
! /* NOTE: 16 is just chosen at random. */
#ifndef MIN_REMOTE_PACKET_SIZE
! #define MIN_REMOTE_PACKET_SIZE 16
#endif
long what_they_get;
if (config->fixed_p)
--- 413,421 ----
#ifndef MAX_REMOTE_PACKET_SIZE
#define MAX_REMOTE_PACKET_SIZE 16384
#endif
! /* NOTE: 20 ensures we can write at least one byte. */
#ifndef MIN_REMOTE_PACKET_SIZE
! #define MIN_REMOTE_PACKET_SIZE 20
#endif
long what_they_get;
if (config->fixed_p)
*************** remote_fetch_registers (int regnum)
*** 3332,3337 ****
--- 3332,3338 ----
in the buffer is not a hex character, assume that has happened
and try to fetch another packet to read. */
while ((buf[0] < '0' || buf[0] > '9')
+ && (buf[0] < 'A' || buf[0] > 'F')
&& (buf[0] < 'a' || buf[0] > 'f')
&& buf[0] != 'x') /* New: unavailable register value. */
{
*************** remote_write_bytes (CORE_ADDR memaddr, c
*** 3659,3674 ****
/* Verify that the target can support a binary download. */
check_binary_download (memaddr);
/* Compute the size, and then allocate space for the largest
! possible packet. Include space for an extra trailing NULL. */
! sizeof_buf = get_memory_write_packet_size () + 1;
buf = alloca (sizeof_buf);
/* Compute the size of the actual payload by subtracting out the
! packet header and footer overhead: "$M<memaddr>,<len>:...#nn". */
! payload_size = (get_memory_write_packet_size () - (strlen ("$M,:#NN")
! + hexnumlen (memaddr)
! + hexnumlen (len)));
/* Construct the packet header: "[MX]<memaddr>,<len>:". */
--- 3660,3677 ----
/* Verify that the target can support a binary download. */
check_binary_download (memaddr);
+ payload_size = get_memory_write_packet_size ();
+
/* Compute the size, and then allocate space for the largest
! possible packet. Include space for an extra trailing NUL. */
! sizeof_buf = payload_size + 1;
buf = alloca (sizeof_buf);
/* Compute the size of the actual payload by subtracting out the
! packet header and footer overhead: "$M<memaddr>,<len>:...#nn".
! */
! payload_size -= strlen ("$M,:#NN");
! payload_size -= hexnumlen (memaddr);
/* Construct the packet header: "[MX]<memaddr>,<len>:". */
*************** remote_write_bytes (CORE_ADDR memaddr, c
*** 3681,3691 ****
--- 3684,3698 ----
*p++ = 'X';
/* Best guess at number of bytes that will fit. */
todo = min (len, payload_size);
+ payload_size -= hexnumlen (todo);
+ todo = min (todo, payload_size);
break;
case PACKET_DISABLE:
*p++ = 'M';
/* Num bytes that will fit. */
todo = min (len, payload_size / 2);
+ payload_size -= hexnumlen (todo);
+ todo = min (todo, payload_size / 2);
break;
case PACKET_SUPPORT_UNKNOWN:
internal_error (__FILE__, __LINE__,
*************** remote_write_bytes (CORE_ADDR memaddr, c
*** 3693,3698 ****
--- 3700,3708 ----
default:
internal_error (__FILE__, __LINE__, _("bad switch"));
}
+ if (todo <= 0)
+ internal_error (__FILE__, __LINE__,
+ _("minumum packet size too small to write data"));
/* Append "<memaddr>". */
memaddr = remote_address_masked (memaddr);