[Lldb-commits] [PATCH] D34945: Adding Support for Error Strings in Remote Packets

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D34945#798808, @ravitheja wrote:

> With this patch, I see the extra packet to query from server is probably unnecessary. I mean the error code is still there and it should be sufficient for the client to detect an error. As long as it does not mistake it for something else it should not be a problem ? .


Well, if we don't care about newer lldb-server being able to communicate with older clients than it's theoretically superfluous. I'm not sure whether we care about this scenario, but I was kinda expecting we do it this way, because that's how it's done elsewhere (e.g. QListThreadsInStopReply). Let's see if others have anything to say about this (might take a bit of time as it's holiday in the US now).



================
Comment at: docs/lldb-gdb-remote.txt:137
+//
+//  E<error code>;"Error Message"
+//
----------------
The quotes here are misleading -- after reading this I would expect that the string is sent quoted. Best be explicit about the encoding of the error string.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h:64
 
+  PacketResult SendErrorResponse(Status &error);
+
----------------
ravitheja wrote:
> labath wrote:
> > Why the reference?
> how about const reference ?
much better


https://reviews.llvm.org/D34945