From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19209 invoked by alias); 31 Jul 2009 21:00:39 -0000 Received: (qmail 19199 invoked by uid 22791); 31 Jul 2009 21:00:38 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_18,J_CHICKENPOX_72,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 31 Jul 2009 21:00:30 +0000 Received: (qmail 9887 invoked from network); 31 Jul 2009 21:00:27 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 31 Jul 2009 21:00:27 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] small change for better error reporting in remote.c Date: Fri, 31 Jul 2009 22:09:00 -0000 User-Agent: KMail/1.9.10 Cc: Michael Snyder References: <4A70E268.6010401@vmware.com> <20090730213019.GB3955@caradoc.them.org> <4A735491.4040006@vmware.com> In-Reply-To: <4A735491.4040006@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200907312155.35340.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-07/txt/msg00772.txt.bz2 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]);