Re: [PATCH 6/6] Implement proper "startup-with-shell" support on gdbserver

Eli Zaretskii
2016-12-23T08:07:30Z

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: palves@redhat.com, Sergio Durigan Junior <sergiodj@redhat.com>
> Date: Thu, 22 Dec 2016 22:39:21 -0500
>
> gdb/doc/ChangeLog:
> 2016-12-22 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * gdb.texinfo (startup-with-shell): Add note mentioning that the
> setting is also used by remote targets.
> (set remote startup-shell): New item.

The text in parentheses should be the name of the node where you make
the change. If you want to refer to specific symbols within the node,
either mention them as part of the text or put them in <..>, like
this:

* gdb.texinfo (Starting) <startup-with-shell>: Add note ...

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a0de7d1..3250cae 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2174,6 +2174,10 @@ initialization file---such as @file{.cshrc} for C-shell,
> $@file{.zshenv} for the Z shell, or the file specified in the
> @samp{BASH_ENV} environment variable for BASH.
>
> +This setting is also used by @code{target extended-remote} to
> +determine whether the target system will start the inferior using the
> +shell (@pxref{set remote startup-shell}).

I would add here a qualification: only on Unix-like target systems.
(Is it true that this is a necessary condition for the applicability
of that feature?)

> +@item set remote startup-shell @var{shell}
> +@itemx show remote startup-shell
> +@anchor{set remote startup-shell}
> +@cindex startup shell, for remote target
> +Set the shell that is going to be used to start the inferior with
> +@code{target extended-remote}. This should be set to a valid shell on
> +the target system, and is only effective when
> +@code{startup-with-shell} is on. If it is not set, the target system
> +will use the shell pointed by the @code{SHELL} environment variable.

Three comments on this:

. What is a "valid shell" in this context? Is the DOS command.com a
valid shell, for example (probably not)? I think we should be
more explicit about which shells are acceptable.

. What exactly is the value supposed to be? Is it a full absolute
file name of the shell executable? We should say so explicitly,
IMO.

. Does it really make sense to require that both this option and
startup-with-shell be on? Why cannot the user set only this
option to achieve that effect, with some special value standing
for "use $SHELL"? Also, is it possible that a user will want
remote targets to use a shell, while avoiding that for native
debugging, in the same session?

A minor nit: environment variables should have the @env markup, not
@code.

I think this warrants a NEWS entry.

Thanks.