Re: [PATCH] gdb/remote: fix qRcmd error handling

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

>>> The patch itself seems reasonable to me, but at the same time, isn't the
>>> behavior you describe also fine?
>>> 
>>> The docs describe "OK", other "O" output packets, the "E" response, and
>>> the empty response.  It seems to me that any other response could
>>> reasonably be treated as an error.
>
> Andrew> The behaviour I'm restoring is documented, see the 'OUTPUT' entry in the
> Andrew> 'Reply:' list:
>
> Andrew>        'OUTPUT'
> Andrew>             A command response with the hex encoded output string OUTPUT.
>
> I thought your patch was concerned with changing the output of responses
> other than 'E' or 'O'... did I misunderstand that?

I don't think so.  I guess I didn't explain myself very well.

After GDB sends a qRcmd packet to the remote, the remote can reply with
zero or more 'O' packets.  This patch is not about these replies; they
are still working fine.

The remote can also reply with an 'E' packet.  This patch is not about
these error reply packets; they are still working fine.

The remote can also reply with an empty packet indicating the qRcmd is
not supported.  This patch is not about this type of reply; this is
still working fine.

The remote can also reply with a packet containing the string "OK", this
is different from a general 'O' packet, this indicates that the remote
is done handling the qRcmd and so GDB is done too.  This patch is not
about this reply; this is still working fine.

The remote can also reply with a random hex encoded string, of the kind
that would appear after the 'O' in an 'O' packet, but without the adding
the leading 'O' character.  When GDB gets one of these packets GDB
decodes the string and prints it, after which the qRcmd is done.  This
patch is about this behaviour only.

This feature was broken in commit 3623271997a5c0d, after this the hex
encoded string would be treated as an error.  In fact, what happens is
that GDB calls packet_result::err_msg() which will then throw an
assertion error because the packet status is PACKET_OK, not
PACKET_ERROR.

Thanks,
Andrew