Re: [PATCH 03/12] Document "E.MESSAGE" RSP errors

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves <pedro@palves.net> writes:

> [Adding Eli.]
>
> AFAICT, this is the only patch left in the series that needs an OK.

Thanks for updating the docs.  I'm happy with this.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> Any comments on the updated documentation change below?
>
> Pedro Alves
>
> On 2024-04-22 20:04, Pedro Alves wrote:
>> On 2024-04-22 09:50, Andrew Burgess wrote:
>> 
>>>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>>>> index 57260a5b2fa..d6184d52841 100644
>>>> --- a/gdb/doc/gdb.texinfo
>>>> +++ b/gdb/doc/gdb.texinfo
>>>> @@ -42557,6 +42557,10 @@ number.  In almost all cases, the protocol does not specify the
>>>>  meaning of the error numbers; @value{GDBN} usually ignores the
>>>>  numbers, or displays them to the user without further interpretation.
>>>>  
>>>> +@item @samp{E.@var{message}}
>>>> +An error has occurred; @var{message} is the textual error message,
>>>> +encoded in @sc{ascii}.
>>>
>>> I think we should document here that the 'qRcmd' and 'm' packets don't
>>> accept this reply format.
>>>
>>> Alexandra Hájková has a patch series which has not been posted yet which
>>> will extend these packets in a backward compatible way so that E.message
>>> can be used, but until that arrives we should probably document this
>>> limitation.
>> 
>> Done, see updated patch below.  (See the bottom of the new commit log.)
>> 
>> Not sure when we'd want to make GDB print the error for the 'm' packet,
>> though.  Currently it just returns TARGET_XFER_E_IO up to the caller.
>> 
>> I noticed a few packets were documenting an "E.errtext" response,
>> which I now removed in this version.  Then "E.errtext" seemed more
>> descriptive than "E.message", so I switched to that too.
>> 
>> 
>> From 36ef5ca67f515039bfa68d2fb2515f181629a24f Mon Sep 17 00:00:00 2001
>> From: Pedro Alves <pedro@palves.net>
>> Date: Thu, 18 Apr 2024 20:22:36 +0100
>> Subject: [PATCH] Document "E.MESSAGE" RSP errors
>> 
>> For many years, GDB has accepted a "E.MESSAGE" error response, in
>> addition to "E NN".  For many packets, GDB strips the "E." before
>> giving the error message to the user.  For others, GDB does not strip
>> the "E.", but still understands that it is an error, as it starts with
>> "E", and either prints the whole string, or ignores it and just
>> mentions an error occured (same as for "E NN").
>> 
>> This has been the case for as long as I remember.  Now that I check, I
>> see that it's been there since 2006 (commit a76d924dffcb, also here:
>> https://sourceware.org/pipermail/gdb-patches/2006-September/047286.html).
>> All along, I actually thought it was documented.  Turns out it wasn't.
>> 
>> This commit documents it, in the new "Standard Replies" section, near
>> where we document "E NN".
>> 
>> The original version of this 3-patch documentation series was a single
>> CodeSourcery patch that documented the textual error as
>> "E.NAME.MESSAGE", with MESSAGE being 8-bit binary encoded.  But I
>> think the ship has sailed for that.  GDBserver has been sending error
>> messages with more than one "." for a long while, and with no binary
>> encoding.  Still, I've preserved the "Co-Authored-By" list of the
>> original larger patch.
>> 
>> The 'qRcmd' and 'm' commands are exceptions and do not accept this
>> reply format.  The top of the "Standard Replies" section already says:
>> 
>>   "All commands support these, except as noted in the individual
>>   command descriptions."
>> 
>> So this adds a note to the description of 'qRcmd' and 'm', explicitly
>> stating that they do not support this error reply format.
>> 
>> Change-Id: Ie4fee3d00d82ede39e439bf162e8cb7485532fd8
>> Co-Authored-By: Jim Blandy <jimb@codesourcery.com>
>> Co-Authored-By: Mike Wrighton <mike_wrighton@mentor.com>
>> Co-Authored-By: Nathan Sidwell <nathan@codesourcery.com>
>> Co-Authored-By: Hafiz Abid Qadeer <abidh@codesourcery.com>
>> ---
>>  gdb/doc/gdb.texinfo | 23 +++++++++++++----------
>>  1 file changed, 13 insertions(+), 10 deletions(-)
>> 
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 9781ce76226..f55ada29e9c 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -42559,6 +42559,11 @@ number.  In almost all cases, the protocol does not specify the
>>  meaning of the error numbers; @value{GDBN} usually ignores the
>>  numbers, or displays them to the user without further interpretation.
>>  
>> +@anchor{textual error reply}
>> +@item @samp{E.@var{errtext}}
>> +An error has occurred; @var{errtext} is the textual error message,
>> +encoded in @sc{ascii}.
>> +
>>  @end table
>>  
>>  @node Packets
>> @@ -42885,6 +42890,10 @@ The reply may contain fewer addressable memory units than requested if the
>>  server was able to read only part of the region of memory.
>>  @end table
>>  
>> +Unlike most packets, this packet does not support
>> +@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
>> +error reply}).
>> +
>>  @item M @var{addr},@var{length}:@var{XX@dots{}}
>>  @cindex @samp{M} packet
>>  Write @var{length} addressable memory units starting at address @var{addr}
>> @@ -44399,6 +44408,10 @@ A command response with no output.
>>  A command response with the hex encoded output string @var{OUTPUT}.
>>  @end table
>>  
>> +Unlike most packets, this packet does not support
>> +@samp{E.@var{errtext}}-style textual error replies (@pxref{textual
>> +error reply}).
>> +
>>  (Note that the @code{qRcmd} packet's name is separated from the
>>  command by a @samp{,}, not a @samp{:}, contrary to the naming
>>  conventions above.  Please don't use this packet as a model for new
>> @@ -45455,8 +45468,6 @@ Reply:
>>  @table @samp
>>  @item OK
>>  Branch tracing has been enabled.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>>  @end table
>>  
>>  @item Qbtrace:pt
>> @@ -45466,8 +45477,6 @@ Reply:
>>  @table @samp
>>  @item OK
>>  Branch tracing has been enabled.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>>  @end table
>>  
>>  @item Qbtrace:off
>> @@ -45477,8 +45486,6 @@ Reply:
>>  @table @samp
>>  @item OK
>>  Branch tracing has been disabled.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>>  @end table
>>  
>>  @item Qbtrace-conf:bts:size=@var{value}
>> @@ -45489,8 +45496,6 @@ Reply:
>>  @table @samp
>>  @item OK
>>  The ring buffer size has been set.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>>  @end table
>>  
>>  @item Qbtrace-conf:pt:size=@var{value}
>> @@ -45501,8 +45506,6 @@ Reply:
>>  @table @samp
>>  @item OK
>>  The ring buffer size has been set.
>> -@item E.errtext
>> -A badly formed request or an error was encountered.
>>  @end table
>>  
>>  @end table
>> 
>> base-commit: 0e6747d2a638693ad2f20e7929c8364913c87279
>> prerequisite-patch-id: 69a7241ff790954e07942cd212971bcfaa8f3a08
>> prerequisite-patch-id: b398b6e111b68e06adccda7d9e016ebe39b5fd35