Re: [PATCH] Warn the user when $SHELL is invalid

[Pedro Alves <palves@redhat.com>]

On 07/28/2015 08:58 PM, Sergio Durigan Junior wrote:
> After a lot of discussion (really, I did not know this patch would
> draw so much attention), this patch has now been simplified (mostly
> because of Doug's suggestions).
> 
> It is known that GDB needs a valid shell to start the inferior and to
> offer the "shell" command to the user.  This has recently been the
> cause of a problem on the MIPS buildslave, because $SHELL was set to
> /sbin/nologin and several tests were failing.  The thread is here:
> 
>   <https://sourceware.org/ml/gdb-patches/2015-07/msg00535.html>
> 
> In order to improve the error/warning messages emitted in this
> scenario, this patch proposes a new check to be performed before
> executing the inferior (or before executing a program using the
> "shell" command on GDB).  This new check tries to determine if the
> specified $SHELL is valid or not.  The default behavior has not been
> changed: if $SHELL is invalid, GDB will still try to execute a program
> using it, which will lead to an error eventually.  However, now GDB
> will also display a message to the user saying that $SHELL is invalid.
> This should help when determining what went wrong.
> 
> The check for a valid shell is simple and has been proposed by Doug.
> We just fork and then exec a simple command:
> 
>   $SHELL -c 'exit 42'
> 
> If the return value is 42, then $SHELL is valid.  Otherwise we issue
> the warning.  It is obviously trivial to create a program that returns
> 42 and ignores its arguments, and it it obvious that this program
> would be considered a valid shell by GDB, but this should not cause
> any problem; the only "drawback" for the user is that she wouldn't get
> the warning message before the error.
> 
> OK to apply?

As mentioned in the other thread, myself, I'm not convinced of the
value of the extra fork/exit complexity for this.  IMO, a wider
potential bug surface for a not-so-clear benefit.  We currently get:

  $ SHELL=/nonexisting gdb /home/pedro/a.out
  (gdb) r
  Starting program: /home/pedro/a.out
  Cannot exec /nonexisting -c exec /home/pedro/a.out .
  Error: No such file or directory
  During startup program exited with code 127.
  (gdb)

If we're starting with a shell, then if the exec fails, it was
obviously because execing the shell failed.  I'd suggest simply trying
to make the error message clearer.  E.g.,:

  $ SHELL=/nonexisting gdb /home/pedro/a.out
  (gdb) r
  Starting program: /home/pedro/a.out
  "set startup-with-shell" is on, but failed to exec:
  /nonexisting -c exec /home/pedro/a.out
  Error: No such file or directory
  If set, the SHELL environment variable must point at a valid shell.
  SHELL is currently set to "/nonexisting".
  During startup program exited with code 127.
  (gdb)

Thanks,
Pedro Alves