Re: [PATCH v3] Make sure GDB uses a valid shell when starting the inferior and to perform the "shell" command

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

On Tuesday, July 28 2015, Pedro Alves wrote:

> I have to say that I'm a bit puzzled at the necessity of
> performing any validity check upfront.

The original idea was not only to check if the $SHELL is valid, but
mostly make GDB react to this by using another shell instead (/bin/sh in
this case).

> The proposed commit log says:
>
>> 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>
>
> But, all that confusion stems from the bogus error, which was meanwhile
> fixed by:
>
>  https://sourceware.org/ml/gdb-patches/2015-07/msg00705.html
>
> With that in place, the original error log would look like:
>
> 220-exec-run
> &"Cannot exec /sbin/nologin
> -c exec /mips/proj/build-compiler/upstream-testing/mipsswbrd048/GDB-testing/debian-mips-m64/build/gdb/testsuite/outputs/gdb.mi/mi-watch/mi-watch
> .\n"
>
> which should have made the problem obvious.  I'd hazard a guess
> that even if that was:
>
>  Cannot exec /opt/whatever/bin/someshell -c exec /mips/proj/build-compiler/upstream-testing/mipsswbrd048/GDB-testing/debian-mips-m64/build/gdb/testsuite/outputs/gdb.mi/mi-watch/mi-watch
>
> then the first think you'd do is try running that manually, and figure
> out quickly what is wrong.

Sure, the new error message makes things easier to figure out indeed.
And on a second thought I also agree that using the fork mechanism to
just be able to issue a warning to the user seems too much/too
dangerous.

To be completely honest I still prefer the first version of the patch,
which made GDB react when $SHELL was set to something dubious like
/sbin/nologin and choose a proper shell to start the inferior instead.
However, I understand that this patch has caused a lot of bikeshed
already, and has been "degraded" to "just warn the user" (which, as you
pointed out, is indeed not very necessary anymore), so TBH I don't have
the energy/time to keep hacking on it now.

I appreciate all the feedback received!

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/