Re: [PATCH v3 4/5] Implement "set cwd" command on GDB

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Friday, September 22 2017, Eli Zaretskii wrote:

>> +@kindex set cwd
>> +@cindex change inferior's working directory
>> +@item set cwd @r{[}@var{directory}@r{]}
>> +Set the inferior's working directory to @var{directory}.  If not
>> +given, @var{directory} uses @file{'~'}.
>
> I think we should document here what does "~" mean on MS-Windows,
> especially since, when HOME is not in the environment, Gnulib's glob
> module doesn't behave according to MS platform recommendations (which
> say not to create files directly below %HOMEDRIVE%%HOMEPATH%).

Sure, but just to be clear, this text was strongly based on another part
of the docs, which also mentions '~' without explaining further.

As I am totally out of the loop when it comes to Windows environments,
I'd appreciate a suggestion for the new text.

> More generally, I think we should say here that the argument is
> glob-expanded, because this is user-visible behavior (right?).  Also,
> how will TAB-completion react to input of this command? will it expand
> the input typed so far?

TAB-completion will not change anything; we don't have such a facility
on GDB AFAIK.  But it will complete the input.

>> +@kindex show cwd
>> +@cindex show inferior's working directory
>> +@item show cwd
>> +Show the inferior's working directory.  If no directory has been
>> +specified by @code{set cwd}, then the default inferior's working
>> +directory is the same as @value{GDBN}'s working directory.
>
> Does this show the original value typed by the user, or the expanded
> value?  E.g., if the user types "set cwd ~/foo", what will "show cwd"
> display?  If it shows the unexpanded form, does that mean the actual
> cwd will change if, say, HOME changes?

Pedro and I had a conversation about this specific topic yesterday, and
the decision was that the host should not do any path expansion on this
case.  Therefore, whatever the user sets with "set cwd" is not expanded
until the inferior starts, which means that it is the target who
performs the expansion.

> Should we store the cwd after tilde-expansion?

I don't think so.  Or at least I cannot see a reason to do that.

>> @@ -2461,6 +2462,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
>>    BOOL ret;
>>    DWORD flags = 0;
>>    const char *inferior_io_terminal = get_inferior_io_terminal ();
>> +  const char *inferior_cwd = get_inferior_cwd ();
>>  
>>    if (!exec_file)
>>      error (_("No executable specified, use `target exec'."));
>> @@ -2488,8 +2490,15 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
>>  	error (_("Error starting executable: %d"), errno);
>>        cygallargs = (wchar_t *) alloca (len * sizeof (wchar_t));
>>        mbstowcs (cygallargs, allargs, len);
>> +
>> +      len = mbstowcs (NULL, inferior_cwd, 0) + 1;
>> +      if (len == (size_t) -1)
>> +	error (_("Invalid cwd for inferior: %d"), errno);
>> +      infcwd = (wchar_t *) alloca (len * sizeof (wchar_t));
>> +      mbstowcs (infcwd, inferior_cwd, len);
>>  #else  /* !__USEWIDE */
>>        cygallargs = allargs;
>> +      infcwd = (cygwin_buf_t *) inferior_cwd;
>>  #endif
>>      }
>>    else
>> @@ -2574,7 +2583,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
>>  		       TRUE,	/* inherit handles */
>>  		       flags,	/* start flags */
>>  		       w32_env,	/* environment */
>> -		       NULL,	/* current directory */
>> +		       infcwd,	/* current directory */
>>  		       &si,
>>  		       &pi);
>>    if (w32_env)
>> @@ -2697,7 +2706,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
>>  			TRUE,	/* inherit handles */
>>  			flags,	/* start flags */
>>  			w32env,	/* environment */
>> -			NULL,	/* current directory */
>> +			inferior_cwd,	/* current directory */
>>  			&si,
>>  			&pi);
>>    if (tty != INVALID_HANDLE_VALUE)
>
> This seems to pass the unexpanded cwd directly to CreateProcess.  I
> don't think this will work on Windows, as this directory is not
> interpreted by any shell, so "~" will cause errors.  I think we should
> pass this via gdb_tilde_expand, like we do in the Unix case, and I
> also think we should mirror all the slashes in the result, just in
> case.

Hm, you're right.  I will call "gdb_tilde_expand" here.  I'm not sure
what you mean by "mirror all the slashes in the result".  Do you mean
"escape the slashes"?

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/