This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3] Make sure GDB uses a valid shell when starting the inferior and to perform the "shell" command
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Doug Evans <xdje42 at gmail dot com>, GDB Patches <gdb-patches at sourceware dot org>, Eli Zaretskii <eliz at gnu dot org>
- Date: Wed, 29 Jul 2015 15:20:59 -0400
- Subject: Re: [PATCH v3] Make sure GDB uses a valid shell when starting the inferior and to perform the "shell" command
- Authentication-results: sourceware.org; auth=none
- References: <1437761993-18758-1-git-send-email-sergiodj at redhat dot com> <1437869674-7880-1-git-send-email-sergiodj at redhat dot com> <CAP9bCMTpZUUdRJW8-V2PTFFgfuN2FrSdX=+koPcZ_aner5CL6A at mail dot gmail dot com> <874mkq4t58 dot fsf at redhat dot com> <CAP9bCMR19LV8DzukTHyCsaGj5uA+vPn8an0C8jHAd7xfqa+tog at mail dot gmail dot com> <55B80C0A dot 5020205 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/