Re: [PATCH v4 3/3] Extend "set cwd" to work on gdbserver

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Friday, September 29 2017, Pedro Alves wrote:

> On 09/28/2017 05:10 AM, Sergio Durigan Junior wrote:
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -26,6 +26,14 @@
>>  
>>  * New features in the GDB remote stub, GDBserver
>>  
>> +  ** GDBserver is now able to set the inferior's current working
>> +     directory.
>
> That's confusing IMO, because it sounds like you're saying
> you can change the inferior's current working directory at
> any time, even on inferiors that are already running...
>
> I'd go for:
>
>   ** GDBserver is now able to start inferior processes with a
>      specified initial working directory.

OK, changed.

>> +
>> +     The user can set the desired working directory to be used by the
>> +     remote inferior on GDB, using the new "set cwd" command, which
>> +     will instruct GDB to tell GDBserver about this directory change
>> +     the next time an inferior is run.
>
> And instead of the above, say:
>
>      The user can set the desired working directory to be used from GDB
>      using the new "set cwd" command.
>
> Note "on" -> "from".  I think that conveys it better.
>
> The "which will instruct" part is distracting and unnecessary
> implementation detail, IMO.

Done.

>
>> +@item QSetWorkingDir:@r{[}@var{directory}@r{]}
>> +@anchor{QSetWorkingDir packet}
>> +@cindex set working directory, remote request
>> +@cindex @samp{QSetWorkingDir} packet
>> +This packet is used to inform the remote server of the intended
>> +current working directory for programs that are going to be executed.
>> +
>> +The packet is composed by @var{directory}, an hex encoded
>> +representation of the directory that the remote inferior will use as
>> +its current working directory.  If @var{directory} is an empty string,
>> +the remote server should reset the inferior's current working
>> +directory to its original, empty value.
>> +
>> +This packet is always transmitted when the inferior is run.  If the
>> +user has not explcitly specified a directory with the @kbd{set cwd}
>
> typo: explicitly

Fixed.

>> +command, then an empty packet will be sent to the remote server, which
>
> Not an empty packet, and empty directory.  Really-empty packets are 
> special in the remote protocol.
>
>> +will have no effect.  
>
> WDYM will have no effect, when just above you've explained that empty
> means reset ?  I think you should just remove this paragraph starting
> with "This packet is always transmitted".  That's implementation
> detail.  GDB could for example not resend the packet if the value
> didn't change between runs, no?

What I tried to say is that if the user doesn't make use of the "set
cwd" command in a GDB session, the packet will be transmitted anyway.  I
think I should have been clearer here.

I will remove the paragraph as requested.

>> Otherwise, the specified inferior's working
>> +directory will be transmitted (@pxref{Working Directory, ,Your
>> +Program's Working Directory}).
>
> This too should be tweaked accordingly.

Since the whole paragraph has been removed, then this comment doesn't
apply anymore, I assume.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/