[PATCH] Remove variable in get_startup_shell non-static

[PATCH] Remove variable in get_startup_shell non-static

[Tom Tromey]

I noticed that a variable in get_startup_shell is "static".  However,
I couldn't see any reason it ought to be, so this removes the
"static".

gdb/ChangeLog
2018-09-13  Tom Tromey  <tom@tromey.com>

	* nat/fork-inferior.c (get_startup_shell): Remove "static".
---
 gdb/ChangeLog           | 4 ++++
 gdb/nat/fork-inferior.c | 4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index ea71aad25f7..40cd05a0f8f 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -272,9 +272,7 @@ execv_argv::init_for_shell (const char *exec_file,
 static const char *
 get_startup_shell ()
 {
-  static const char *ret;
-
-  ret = getenv ("SHELL");
+  const char *ret = getenv ("SHELL");
   if (ret == NULL)
     ret = SHELL_FILE;
 
-- 
2.17.1

Re: [PATCH] Remove variable in get_startup_shell non-static

[Sergio Durigan Junior]

On Thursday, September 13 2018, Tom Tromey wrote:

> I noticed that a variable in get_startup_shell is "static".  However,
> I couldn't see any reason it ought to be, so this removes the
> "static".

IIRC this code was added by a patch of mine.  I think your patch makes
sense.  Perhaps, since the SHELL can't change once you start GDB, this
function could be made a bit smarter and check whether "ret" has been
initialized or not (in order to call "getenv" just once), but really,
that's not necessary at all, and your patch is totally fine.

Thanks,

> gdb/ChangeLog
> 2018-09-13  Tom Tromey  <tom@tromey.com>
>
> 	* nat/fork-inferior.c (get_startup_shell): Remove "static".
> ---
>  gdb/ChangeLog           | 4 ++++
>  gdb/nat/fork-inferior.c | 4 +---
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index ea71aad25f7..40cd05a0f8f 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -272,9 +272,7 @@ execv_argv::init_for_shell (const char *exec_file,
>  static const char *
>  get_startup_shell ()
>  {
> -  static const char *ret;
> -
> -  ret = getenv ("SHELL");
> +  const char *ret = getenv ("SHELL");
>    if (ret == NULL)
>      ret = SHELL_FILE;
>  
> -- 
> 2.17.1

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Remove variable in get_startup_shell non-static

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> IIRC this code was added by a patch of mine.  I think your patch makes
Sergio> sense.  Perhaps, since the SHELL can't change once you start GDB, this
Sergio> function could be made a bit smarter and check whether "ret" has been
Sergio> initialized or not (in order to call "getenv" just once), but really,
Sergio> that's not necessary at all, and your patch is totally fine.

Thanks, I'm going to check it in.

Tom

Re: [PATCH] Remove variable in get_startup_shell non-static

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> IIRC this code was added by a patch of mine.  I think your patch makes
Sergio> sense.  Perhaps, since the SHELL can't change once you start GDB, this
Sergio> function could be made a bit smarter and check whether "ret" has been
Sergio> initialized or not (in order to call "getenv" just once), but really,
Sergio> that's not necessary at all, and your patch is totally fine.

I poked a little at the macOS shell / SIP thing.  One possible solution
is copying the shell, and in this situation it's convenient if
get_startup_shell does not cache.  I already checked this in, but I
thought I'd mention it anyway in case someone was tempted to change it.

Tom