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

Sergio Durigan Junior
2015-07-29T19:21:03Z

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