>
> So, I started reviewing this patch, and wrote a whole bunch of stuff.
> One thing I wrote was, you should really add a test for this that uses
> the existing gdbserver.
>
 I haven't added one because I wasn't sure this patch would be accepted.
I'll add one if we agree this work makes sense.

>
> So, I thought, I wonder what errors gdbserver currently sends back, so I
> took a look at the QSetWorkingDir handling in gdbserver/server.cc.
> Weird, I thought, it doesn't appear to send back any errors.
>
> So then I took a look at set_inferior_cwd in gdbserver/inferiors.cc, and
> all that it does is cache the requested directory.
>
> So, then I went to the docs again, and re-read what we say about
> QSetWorkingDir:
>
>   This packet is used to inform the remote server of the intended
>   current working directory for programs that are going to be executed.
>
> Note the use of the word 'intended'.  This packet does not try to change
> the directory NOW, it will try to change the directory LATER, when the
> inferior is actually executed.
>
> My current reading of the docs is that the QSetWorkingDir packet, is
> either not supported, or should never fail.
>
> If you specify an invalid directory then future attempts to start an
> inferior will fail, but the QSetWorkingDir packet itself shouldn't
> fail.
>

I think I misinterpreted this part. My understanding was the remote
gdbserver (or the stub) will change the directory and then it's weird
to not be able to send back errors. Your interpretation makes perfect
sense.

In this case, if this behaviour is the preferred way to handle the
QSetWOrkingDir,
I propose to change the documentation to make it better understandable.


> I guess we could decide that we want to extend QSetWorkingDir to allow
> for it to send back error packets, I'm not sure that's entirely a good
> idea though as that's not how the existing gdbserver works... but if we
> did want to do that then we should probably improve the documentation to
> explain that some gdbservers will cache the passed in directory and
> return an error later, while others might choose to immediately change
> directory and return an error now.
>
> Because I already wrote it, I've left all my original review comments
> inline below, but I'm not sure if you will want to roll a v3 or not.
>
> What is your preference and does anybody else have an opinion on
QSetWorkingDir
(and similar packets) behaviour?

Another confusing thing in remote protocol documentation (
https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets)
is the error handling. Various packets can reply with an error:

‘E NN’

Indicate a badly formed request. The error number NN is given as hex
digits.
But packet_check_result in remote.c just checks if it's an error and does
not handle the 'NN'. And I haven't found the meaning of 'NN'.

Thank you very much for the in-depth review.