[PATCH] Fix qRcmd error code parsing

[PATCH] Fix qRcmd error code parsing

[Luis Machado]

Someone at IRC spotted a bug in qRcmd handling. This looks like an oversight
or it is that way for historical reasons.

The code in gdb/remote.c:remote_target::rcmd uses isdigit instead of
isxdigit. One could argue that we are expecting decimal numbers, but further
below we use fromhex ().

Update the function to use isxdigit instead and also update the documentation.

I see there are lots of other cases of undocumented number format for error
messages, mostly described as NN instead of nn. For now I'll just update
this particular function.
---
 gdb/doc/gdb.texinfo | 3 ++-
 gdb/remote.c        | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 729f9d79a93..607b680b3c0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -42076,7 +42076,8 @@ A command response with no output.
 @item @var{OUTPUT}
 A command response with the hex encoded output string @var{OUTPUT}.
 @item E @var{NN}
-Indicate a badly formed request.
+Indicate a badly formed request.  The error number @var{NN} is given as
+hex digits.
 @item @w{}
 An empty reply indicates that @samp{qRcmd} is not recognized.
 @end table
diff --git a/gdb/remote.c b/gdb/remote.c
index bc308bd77f8..fe3a73bc6e0 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11564,7 +11564,7 @@ remote_target::rcmd (const char *command, struct ui_file *outbuf)
       if (strcmp (buf, "OK") == 0)
 	break;
       if (strlen (buf) == 3 && buf[0] == 'E'
-	  && isdigit (buf[1]) && isdigit (buf[2]))
+	  && isxdigit (buf[1]) && isxdigit (buf[2]))
 	{
 	  error (_("Protocol error with Rcmd"));
 	}
-- 
2.25.1

Re: [PATCH] Fix qRcmd error code parsing

[Eli Zaretskii]

> Date: Thu, 31 Mar 2022 16:59:04 +0100
> From: Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
> 
> Someone at IRC spotted a bug in qRcmd handling. This looks like an oversight
> or it is that way for historical reasons.
> 
> The code in gdb/remote.c:remote_target::rcmd uses isdigit instead of
> isxdigit. One could argue that we are expecting decimal numbers, but further
> below we use fromhex ().
> 
> Update the function to use isxdigit instead and also update the documentation.
> 
> I see there are lots of other cases of undocumented number format for error
> messages, mostly described as NN instead of nn. For now I'll just update
> this particular function.
> ---
>  gdb/doc/gdb.texinfo | 3 ++-
>  gdb/remote.c        | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

The documentation part is fine, thanks.

Re: [PATCH] Fix qRcmd error code parsing

[Tom Tromey]

>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

Luis> The code in gdb/remote.c:remote_target::rcmd uses isdigit instead of
Luis> isxdigit. One could argue that we are expecting decimal numbers, but further
Luis> below we use fromhex ().

Luis> Update the function to use isxdigit instead and also update the documentation.

This is ok.  Thank you.

Luis> I see there are lots of other cases of undocumented number format for error
Luis> messages, mostly described as NN instead of nn. For now I'll just update
Luis> this particular function.

Errors aren't very well defined in the RSP, unfortunately.
There's also an extended error type used in like one packet.

Tom

Re: [PATCH] Fix qRcmd error code parsing

[Luis Machado]

On 4/4/22 20:16, Tom Tromey wrote:
>>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Luis> The code in gdb/remote.c:remote_target::rcmd uses isdigit instead of
> Luis> isxdigit. One could argue that we are expecting decimal numbers, but further
> Luis> below we use fromhex ().
> 
> Luis> Update the function to use isxdigit instead and also update the documentation.
> 
> This is ok.  Thank you.
> 

Pushed now. Thanks!