[Lldb-commits] [PATCH] D79614: Fix error reporting for qLaunchSuccess, check for fix/enable it via qSupported

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D79614#2030692 <https://reviews.llvm.org/D79614#2030692>, @jasonmolenda wrote:

> Thanks for the feedback.
>
> In D79614#2029157 <https://reviews.llvm.org/D79614#2029157>, @labath wrote:
>
> > I think that "piggy-backing" on the `qSupported` packet for communicating protocol fixes is a good idea. However, I agree with Greg, that it does not seem like it's needed for this case. Fixing the problem purely on the debugserver side seems preferable, as it avoids adding compatibility code to lldb. It's true that it's a one-off error reporting mechanism, but it's already there, so it seems better to just support what's already implemented then try to support two mechanisms.
> >
> > For a long-term solution, I am wondering whether we need `qLaunchSuccess` at all? It seems like the packet is completely redundant in a world where we can return textual error messages to _any_ packet. What was the reason for introducing it in the first place? Could we just switch to using error replies to the `A` packet for communicating the launch errors (with some transition plan for supporting `qLaunchSuccess` for a while)?
>
>
> It's hard to tell what we were thinking (GREG :) back when we used the A packet to set the binary name / arguments /etc, and then the qLaunchSuccess packet to start the process.  Reading over the current era gdb Remote Serial Protocol docs, I think the vRun packet does the combination of these two things in one packet, which makes sense.


There was no "just return error strings" packet that I was aware of. The problem is identifying a correct error packet. Typically you can get some canned responses to packets:

- "" (empty) response means not supported
- "EXX" where XX is a hex code only
- "OK" if the packet was successful
- open ended output

How do error strings get returned when not using the EXX format? Are they safely prefixed with something? Do they just start with "E" and then a string? If so, how would you tell the difference between the response "Everyone has a string" and "Einvalid path error"? Or does this only apply to a packet that returns an error if some setting was enabled with a previous  "Q" packet?

> I tested a patch using Greg's suggestion of using the binary escape protocol for the error string and that does work, as expected, I'll land the patch like that.

Much easier, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79614/new/

https://reviews.llvm.org/D79614