Patchwork Fix type of startup_with_shell in gdbserver

login
register
mail settings
Submitter Tom Tromey
Date Oct. 1, 2019, 1:31 p.m.
Message ID <20191001133127.4249-1-tromey@adacore.com>
Download mbox | patch
Permalink /patch/34763/
State New
Headers show

Comments

Tom Tromey - Oct. 1, 2019, 1:31 p.m.
startup_with_shell was changed to be of "bool" type, but I noticed
that the definition in gdbserver disagreed.  This disagreement caused
some regressions on a big-endian machine.

This patch removes the redundant declaration and definition of
startup_with_shell and ensures that such clashes will be diagnosed.

I didn't know the best place to put the declaration and definition.
fork-inferior did not seem best, because it is conditionally compiled,
so I chose a generic spot instead.

gdb/ChangeLog
2019-10-01  Tom Tromey  <tromey@adacore.com>

	* gdbsupport/common-utils.c (startup_with_shell): Define.
	* infcmd.c (startup_with_shell): Don't define.
	* nat/fork-inferior.h (startup_with_shell): Don't declare.
	* gdbsupport/common-utils.h (startup_with_shell): Declare.
	* inferior.h (startup_with_shell): Don't declare.

gdb/gdbserver/ChangeLog
2019-10-01  Tom Tromey  <tromey@adacore.com>

	* server.c (startup_with_shell): Don't define.
---
 gdb/ChangeLog                 |  8 ++++++++
 gdb/gdbserver/ChangeLog       |  4 ++++
 gdb/gdbserver/server.c        |  6 ------
 gdb/gdbsupport/common-utils.c |  4 ++++
 gdb/gdbsupport/common-utils.h | 20 ++++++++++++++++++++
 gdb/infcmd.c                  |  4 ----
 gdb/inferior.h                | 19 -------------------
 gdb/nat/fork-inferior.h       | 20 --------------------
 8 files changed, 36 insertions(+), 49 deletions(-)
Pedro Alves - Oct. 1, 2019, 5:08 p.m.
On 10/1/19 2:31 PM, Tom Tromey wrote:
> startup_with_shell was changed to be of "bool" type, but I noticed
> that the definition in gdbserver disagreed.  This disagreement caused
> some regressions on a big-endian machine.
> 
> This patch removes the redundant declaration and definition of
> startup_with_shell and ensures that such clashes will be diagnosed.
> 
> I didn't know the best place to put the declaration and definition.
> fork-inferior did not seem best, because it is conditionally compiled,
> so I chose a generic spot instead.
> 
> gdb/ChangeLog
> 2019-10-01  Tom Tromey  <tromey@adacore.com>
> 
> 	* gdbsupport/common-utils.c (startup_with_shell): Define.
> 	* infcmd.c (startup_with_shell): Don't define.
> 	* nat/fork-inferior.h (startup_with_shell): Don't declare.
> 	* gdbsupport/common-utils.h (startup_with_shell): Declare.
> 	* inferior.h (startup_with_shell): Don't declare.
> 
> gdb/gdbserver/ChangeLog
> 2019-10-01  Tom Tromey  <tromey@adacore.com>
> 
> 	* server.c (startup_with_shell): Don't define.

I think common-inferior.{h,c} would be a better place.  It's where
get_exec_wrapper is, another common startun/run option, for example.
common-inferior.c doesn't exist yet, but there's no reason it can't exist.

Thanks,
Pedro Alves
Tom Tromey - Oct. 1, 2019, 6:07 p.m.
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I think common-inferior.{h,c} would be a better place.  It's where
Pedro> get_exec_wrapper is, another common startun/run option, for example.
Pedro> common-inferior.c doesn't exist yet, but there's no reason it can't exist.

Good idea -- I looked for something like this but I somehow missed it.
I sent v2 with this change.

Tom

Patch

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 67e8e3e54de..8ee551828dc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -67,12 +67,6 @@  char *current_directory;
 
 static gdb_environ our_environ;
 
-/* Start the inferior using a shell.  */
-
-/* We always try to start the inferior using a shell.  */
-
-int startup_with_shell = 1;
-
 int server_waiting;
 
 static int extended_protocol;
diff --git a/gdb/gdbsupport/common-utils.c b/gdb/gdbsupport/common-utils.c
index d1059de0b33..030539fa85e 100644
--- a/gdb/gdbsupport/common-utils.c
+++ b/gdb/gdbsupport/common-utils.c
@@ -22,6 +22,10 @@ 
 #include "host-defs.h"
 #include <ctype.h>
 
+/* See common-utils.h.  */
+
+bool startup_with_shell = true;
+
 void *
 xzalloc (size_t size)
 {
diff --git a/gdb/gdbsupport/common-utils.h b/gdb/gdbsupport/common-utils.h
index a5312cb0c49..df8455fbf0b 100644
--- a/gdb/gdbsupport/common-utils.h
+++ b/gdb/gdbsupport/common-utils.h
@@ -188,4 +188,24 @@  in_inclusive_range (T value, T low, T high)
 extern ULONGEST align_up (ULONGEST v, int n);
 extern ULONGEST align_down (ULONGEST v, int n);
 
+/* Whether to start up the debuggee under a shell.
+
+   If startup-with-shell is set, GDB's "run" will attempt to start up
+   the debuggee under a shell.  This also happens when using GDBserver
+   under extended remote mode.
+
+   This is in order for argument-expansion to occur.  E.g.,
+
+   (gdb) run *
+
+   The "*" gets expanded by the shell into a list of files.
+
+   While this is a nice feature, it may be handy to bypass the shell
+   in some cases.  To disable this feature, do "set startup-with-shell
+   false".
+
+   The catch-exec traps expected during start-up will be one more if
+   the target is started up with a shell.  */
+extern bool startup_with_shell;
+
 #endif /* COMMON_COMMON_UTILS_H */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index dc82ef043fe..4d42f75b63a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -103,10 +103,6 @@  enum stop_stack_kind stop_stack_dummy;
 
 int stopped_by_random_signal;
 
-/* See inferior.h.  */
-
-bool startup_with_shell = true;
-
 
 /* Accessor routines.  */
 
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 3a64a7cfeae..43f0417e629 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -227,25 +227,6 @@  extern struct value *get_return_value (struct value *function,
 extern void prepare_execution_command (struct target_ops *target,
 				       int background);
 
-/* Whether to start up the debuggee under a shell.
-
-   If startup-with-shell is set, GDB's "run" will attempt to start up
-   the debuggee under a shell.
-
-   This is in order for argument-expansion to occur.  E.g.,
-
-   (gdb) run *
-
-   The "*" gets expanded by the shell into a list of files.
-
-   While this is a nice feature, it may be handy to bypass the shell
-   in some cases.  To disable this feature, do "set startup-with-shell
-   false".
-
-   The catch-exec traps expected during start-up will be one more if
-   the target is started up with a shell.  */
-extern bool startup_with_shell;
-
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
 extern enum stop_stack_kind stop_stack_dummy;
diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
index 065496c3827..0a76ff82d31 100644
--- a/gdb/nat/fork-inferior.h
+++ b/gdb/nat/fork-inferior.h
@@ -54,26 +54,6 @@  extern ptid_t startup_inferior (pid_t pid, int ntraps,
 				struct target_waitstatus *mystatus,
 				ptid_t *myptid);
 
-/* Whether to start up the debuggee under a shell.
-
-   If startup-with-shell is set, GDB's "run" will attempt to start up
-   the debuggee under a shell.  This also happens when using GDBserver
-   under extended remote mode.
-
-   This is in order for argument-expansion to occur.  E.g.,
-
-   (gdb) run *
-
-   The "*" gets expanded by the shell into a list of files.
-
-   While this is a nice feature, it may be handy to bypass the shell
-   in some cases.  To disable this feature, do "set startup-with-shell
-   false".
-
-   The catch-exec traps expected during start-up will be one more if
-   the target is started up with a shell.  */
-extern bool startup_with_shell;
-
 /* Perform any necessary tasks before a fork/vfork takes place.  ARGS
    is a string containing all the arguments received by the inferior.
    This function is mainly used by fork_inferior.  */