This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] small change for better error reporting in remote.c
On Friday 31 July 2009 21:31:13, Michael Snyder wrote:
> drow@false.org wrote:
> > On Wed, Jul 29, 2009 at 04:59:36PM -0700, Michael Snyder wrote:
> >> @@ -5189,7 +5192,10 @@ store_registers_using_G (const struct re
> >> /* remote_prepare_to_store insures that rsa->sizeof_g_packet gets
> >> updated. */
> >> bin2hex (regs, p, rsa->sizeof_g_packet);
> >> - remote_send (&rs->buf, &rs->buf_size);
> >> + putpkt (rs->buf);
> >> + getpkt (&rs->buf, &rs->buf_size, 0);
> >> + if (rs->buf[0] == 'E')
> >> + error (_("Could not write registers"));
Can you still include the remote error string in this message?
Otherwise, this loses the "feature" of being able to send
more verbose error down the pipe. E.g., notice:
static enum packet_result
packet_check_result (const char *buf)
{
if (buf[0] != '\0')
{
/* The stub recognized the packet request. Check that the
operation succeeded. */
if (buf[0] == 'E'
&& isxdigit (buf[1]) && isxdigit (buf[2])
&& buf[3] == '\0')
/* "Enn" - definitly an error. */
return PACKET_ERROR;
/* Always treat "E." as an error. This will be used for
more verbose error messages, such as E.memtypes. */
if (buf[0] == 'E' && buf[1] == '.')
return PACKET_ERROR;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/* The packet may or may not be OK. Just assume it is. */
return PACKET_OK;
}
else
/* The stub does not support the packet. */
return PACKET_UNKNOWN;
}
> >> }
> >>
> >> /* Store register REGNUM, or all registers if REGNUM == -1, from the contents
> >
> > Can't you use packet_ok like elsewhere? Otherwise OK.
>
> You may not realize it, but you're asking for a much bigger change.
> There is no struct packet_config for the 'G' packet. I'll have to
> change set_registers_using_G from void to int, so that it can
> return failure if the 'G' packet is unsupported.
>
> But I'll begin working on it, unles I hear "never mind"
> from you... ;-)
Heh, not sure how useful it would be to have an optional G
currently. Daniel was probably thinking of packet_check_result.
Maybe you could pick up, integrate and finish this patch
for PR9665?
--
Pedro Alves
---
gdb/remote.c | 64 ++++++++++++++++++++++++-----------------------------------
1 file changed, 27 insertions(+), 37 deletions(-)
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2009-07-31 21:48:21.000000000 +0100
+++ src/gdb/remote.c 2009-07-31 21:49:03.000000000 +0100
@@ -107,8 +107,6 @@ static void extended_remote_mourn (struc
static void remote_mourn_1 (struct target_ops *);
-static void remote_send (char **buf, long *sizeof_buf_p);
-
static int readchar (int timeout);
static void remote_kill (struct target_ops *ops);
@@ -2423,11 +2421,12 @@ get_offsets (void)
getpkt (&rs->buf, &rs->buf_size, 0);
buf = rs->buf;
- if (buf[0] == '\000')
- return; /* Return silently. Stub doesn't support
- this command. */
- if (buf[0] == 'E')
+ switch (packet_check_result (buf))
{
+ case PACKET_UNKNOWN:
+ /* Return silently. Stub doesn't support this command. */
+ return;
+ case PACKET_ERROR:
warning (_("Remote failure reply: %s"), buf);
return;
}
@@ -4873,7 +4872,9 @@ fetch_register_using_p (struct regcache
*p++ = 'p';
p += hexnumstr (p, reg->pnum);
*p++ = '\0';
- remote_send (&rs->buf, &rs->buf_size);
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
buf = rs->buf;
@@ -4921,7 +4922,12 @@ send_g_packet (void)
char *regs;
sprintf (rs->buf, "g");
- remote_send (&rs->buf, &rs->buf_size);
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+
+ if (packet_check_result (rs->buf) == PACKET_ERROR)
+ error (_("Remote failure reply: %s"), rs->buf);
/* We can get out of synch in various cases. If the first character
in the buffer is not a hex character, assume that has happened
@@ -5141,7 +5147,9 @@ store_register_using_P (const struct reg
p = buf + strlen (buf);
regcache_raw_collect (regcache, reg->regnum, regp);
bin2hex (regp, p, register_size (gdbarch, reg->regnum));
- remote_send (&rs->buf, &rs->buf_size);
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_P]))
{
@@ -5189,7 +5197,10 @@ store_registers_using_G (const struct re
/* remote_prepare_to_store insures that rsa->sizeof_g_packet gets
updated. */
bin2hex (regs, p, rsa->sizeof_g_packet);
- remote_send (&rs->buf, &rs->buf_size);
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+ if (packet_check_result (rs->buf) == PACKET_ERROR)
+ error (_("Remote failure reply: %s"), rs->buf);
}
/* Store register REGNUM, or all registers if REGNUM == -1, from the contents
@@ -5600,7 +5611,7 @@ remote_write_bytes_aux (const char *head
putpkt_binary (rs->buf, (int) (p - rs->buf));
getpkt (&rs->buf, &rs->buf_size, 0);
- if (rs->buf[0] == 'E')
+ if (packet_check_result (rs->buf) == PACKET_ERROR)
{
/* There is no correspondance between what the remote protocol
uses for errors and errno codes. We would like a cleaner way
@@ -5702,9 +5713,7 @@ remote_read_bytes (CORE_ADDR memaddr, gd
putpkt (rs->buf);
getpkt (&rs->buf, &rs->buf_size, 0);
- if (rs->buf[0] == 'E'
- && isxdigit (rs->buf[1]) && isxdigit (rs->buf[2])
- && rs->buf[3] == '\0')
+ if (packet_check_result (rs->buf) == PACKET_ERROR)
{
/* There is no correspondance between what the remote
protocol uses for errors and errno codes. We would like
@@ -5935,22 +5944,6 @@ readchar (int timeout)
return ch;
}
-/* Send the command in *BUF to the remote machine, and read the reply
- into *BUF. Report an error if we get an error reply. Resize
- *BUF using xrealloc if necessary to hold the result, and update
- *SIZEOF_BUF. */
-
-static void
-remote_send (char **buf,
- long *sizeof_buf)
-{
- putpkt (*buf);
- getpkt (buf, sizeof_buf, 0);
-
- if ((*buf)[0] == 'E')
- error (_("Remote failure reply: %s"), *buf);
-}
-
/* Return a pointer to an xmalloc'ed string representing an escaped
version of BUF, of len N. E.g. \n is converted to \\n, \t to \\t,
etc. The caller is responsible for releasing the returned
@@ -6879,7 +6872,7 @@ remote_remove_breakpoint (struct gdbarch
putpkt (rs->buf);
getpkt (&rs->buf, &rs->buf_size, 0);
- return (rs->buf[0] == 'E');
+ return (packet_check_result (rs->buf) == PACKET_ERROR);
}
return memory_remove_breakpoint (gdbarch, bp_tgt);
@@ -7186,7 +7179,7 @@ compare_sections_command (char *args, in
host_crc = crc32 ((unsigned char *) sectdata, size, 0xffffffff);
getpkt (&rs->buf, &rs->buf_size, 0);
- if (rs->buf[0] == 'E')
+ if (packet_check_result (rs->buf) == PACKET_ERROR)
error (_("target memory fault, section %s, range %s -- %s"), sectname,
paddress (target_gdbarch, lma),
paddress (target_gdbarch, lma + size));
@@ -7645,11 +7638,8 @@ remote_rcmd (char *command,
}
if (strcmp (buf, "OK") == 0)
break;
- if (strlen (buf) == 3 && buf[0] == 'E'
- && isdigit (buf[1]) && isdigit (buf[2]))
- {
- error (_("Protocol error with Rcmd"));
- }
+ if (packet_check_result (buf) == PACKET_ERROR)
+ error (_("Protocol error with Rcmd"));
for (p = buf; p[0] != '\0' && p[1] != '\0'; p += 2)
{
char c = (fromhex (p[0]) << 4) + fromhex (p[1]);