Re: Documenting E. packet. (was Re: [patch] Fix PR 13392 : check offset of JMP insn)

[Pedro Alves <palves@redhat.com>]

On 03/07/2012 08:41 PM, Philippe Waroquiers wrote:

> On Tue, 2012-03-06 at 13:47 -0800, Stan Shebs wrote:
> 
>>> Is there somewhere a description of what an E. packet is,
>>> and when this is allowed ?
>>>
> ...
>> It doesn't look like it's officially described in the manual.  The 
> ...
>>
>> It's conceivable that one could do something with GDB that interprets 
>> the error reply (whether NN or a string) and does something differently, 
>> but in practice, if the target side is reporting errors, things are 
>> going straight downhill and the user needs to decide what to do about 
>> it.  Also, if a particular result is common enough that GDB has code to 
>> cope with it, then maybe it's just another element of the protocol, not 
>> really an error. :-)
> 
> Effectively, having GDB trying to do something sophisticated depending
> on the E NN nr or E. string received from the stub looks difficult.
> 
> There is however already some kind of logic in GDB.
> E.g. in remote.c, the function trace_error handles differently E.,
> E1? and E2?.


Yeah, that's old legacy stuff.  :-(  We're probably safe in removing those
special E1 and E2 handlings (which all I know about is what's in the code)
by now.

See: packet_ok -> packet_check_result

      /* 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;

So all places that use the modern packet_ok explicitly handle this.

It's very lame that we print the leading '.'.  We should fix that.

> 
> There are other places where only E NN is considered as an error.
> E. is not considered as an error.
> Example: qRcmd.


I think CS/Maciej had a patch for this.

> When I did the Valgrind gdbserver, I had to return a readable error
> msg to the user but did not find a way to do that in the doc.
> I found the E. by reading the gdb sources and used it (believing
> I was doing a nasty thing. But now, I see it is just an officially
> accepted error packet, just not documented :).
> => it looks better to document this E. packet.
> 
> However, it is not clear to me what is exactly expected for
> this documentation/behaviour.
> 
> It looks better if at all places where an E NN is accepted by GDB,
> GDB would also accept an E. packet.


Yeah, though we'd a careful audit.  We should be careful in avoiding the
case of with newer stubs sending "E.xxx" to GDBs that haven't been updated,
and those GDBs not understanding it as an error packet.

> But that is not the current behaviour, so either remote.c is
> changed to consistently accept E NN and E. everywhere,
> or the protocol doc must match the current behaviour,
> and indicate for each packet if an E NN and/or an E. error is
> accepted.


In any case, the documentation should be updated.

-- 
Pedro Alves