This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Preliminary work in fork_inferior


On Fri, Sep 16, 2011 at 5:58 PM, Tristan Gingold <gingold@adacore.com> wrote:
> Hi,
>
> this patch is both a cleanup and a preliminary work for Lion.
>
> It fixes a few weirdness:
> - shell_command was always allocated even if not used
> - argv was xmalloc'ed but never free
> - gdb_flush/_exit sequence for exec failure was duplicated
>
> The preliminary work consists in calling execvp wether or not a shell is executed.
>
> No regressions on GNU/Linux i386
>
> Ok for trunk ?
>
> Tristan.
>
> 2011-09-07 ?Tristan Gingold ?<gingold@adacore.com>
>
> ? ? ? ?* fork-child.c (fork_inferior): Update comment.
> ? ? ? ?Use alloca instead of xmalloc for argv. ?Move
> ? ? ? ?len and shell_command declarations in the block where they are used.
> ? ? ? ?Only call execvp. ?Factorize some failure code.
>
> diff --git a/gdb/fork-child.c b/gdb/fork-child.c
> index bb173e7..e937aec 100644
> --- a/gdb/fork-child.c
> +++ b/gdb/fork-child.c
> @@ -126,9 +126,7 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> ? ? ? ? ? ? ? void (*pre_trace_fun) (void), char *shell_file_arg)
> ?{
> ? int pid;
> - ?char *shell_command;
> ? static char default_shell_file[] = SHELL_FILE;
I don't know whether this is relevant, but I think the SHELL_FILE
macro is defined as "/bin/sh" and this will not work for MinGW. Some
similar comments were popped up during the review of my patch for
adding pipe command in GDB. I'm relating this to that.
> - ?int len;
> ? /* Set debug_fork then attach to the child while it sleeps, to debug. ?*/
> ? static int debug_fork = 0;
> ? /* This is set to the result of setpgrp, which if vforked, will be visible
> @@ -162,16 +160,6 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> ? ? ? shell = 1;
> ? ? }
>
> - ?/* Multiplying the length of exec_file by 4 is to account for the
> - ? ? fact that it may expand when quoted; it is a worst-case number
> - ? ? based on every character being '. ?*/
> - ?len = 5 + 4 * strlen (exec_file) + 1 + strlen (allargs) + 1 + /*slop */ 12;
> - ?if (exec_wrapper)
> - ? ?len += strlen (exec_wrapper) + 1;
> -
> - ?shell_command = (char *) alloca (len);
> - ?shell_command[0] = '\0';
> -
> ? if (!shell)
> ? ? {
> ? ? ? /* We're going to call execvp. ?Create argument vector.
> @@ -180,18 +168,29 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> ? ? ? ? argument. ?*/
> ? ? ? int argc = (strlen (allargs) + 1) / 2 + 2;
>
> - ? ? ?argv = (char **) xmalloc (argc * sizeof (*argv));
> + ? ? ?argv = (char **) alloca (argc * sizeof (*argv));
alloca is not a portable function I guess.
> ? ? ? argv[0] = exec_file;
> ? ? ? breakup_args (allargs, &argv[1]);
> ? ? }
> ? else
> ? ? {
> ? ? ? /* We're going to call a shell. ?*/
> -
> + ? ? ?char *shell_command;
> + ? ? ?int len;
> ? ? ? char *p;
> ? ? ? int need_to_quote;
> ? ? ? const int escape_bang = escape_bang_in_quoted_argument (shell_file);
>
> + ? ? ?/* Multiplying the length of exec_file by 4 is to account for the
> + ? ? ? ? fact that it may expand when quoted; it is a worst-case number
> + ? ? ? ? based on every character being '. ?*/
> + ? ? ?len = 5 + 4 * strlen (exec_file) + 1 + strlen (allargs) + 1 + /*slop */ 12;
> + ? ? ?if (exec_wrapper)
> + ? ? ? ?len += strlen (exec_wrapper) + 1;
> +
> + ? ? ?shell_command = (char *) alloca (len);
> + ? ? ?shell_command[0] = '\0';
> +
> ? ? ? strcat (shell_command, "exec ");
>
> ? ? ? /* Add any exec wrapper. ?That may be a program name with arguments, so
> @@ -257,6 +256,16 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
>
> ? ? ? strcat (shell_command, " ");
> ? ? ? strcat (shell_command, allargs);
> +
> + ? ? ?/* If we decided above to start up with a shell, we exec the
> + ? ? ? ?shell, "-c" says to interpret the next arg as a shell command
> + ? ? ? ?to execute, and this command is "exec <target-program>
> + ? ? ? ?<args>". ?*/
> + ? ? ?argv = (char **) alloca (4 * sizeof (char *));
Same comment about alloca as above.
> + ? ? ?argv[0] = shell_file;
> + ? ? ?argv[1] = "-c";
For MinGW the -c switch will not work.
> + ? ? ?argv[2] = shell_command;
> + ? ? ?argv[3] = (char *) 0;
> ? ? }
>
> ? /* On some systems an exec will fail if the executable is open. ?*/
> @@ -348,29 +357,18 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> ? ? ? ? ?path to find $SHELL. ?Rich Pixley says so, and I agree. ?*/
> ? ? ? environ = env;
>
> - ? ? ?/* If we decided above to start up with a shell, we exec the
> - ? ? ? ?shell, "-c" says to interpret the next arg as a shell command
> - ? ? ? ?to execute, and this command is "exec <target-program>
> - ? ? ? ?<args>". ?*/
> + ? ? ?execvp (argv[0], argv);
> +
> + ? ? ?/* If we get here, it's an error. ?*/
> ? ? ? if (shell)
> ? ? ? ?{
> - ? ? ? ? execlp (shell_file, shell_file, "-c", shell_command, (char *) 0);
> -
> - ? ? ? ? /* If we get here, it's an error. ?*/
> ? ? ? ? ?fprintf_unfiltered (gdb_stderr, "Cannot exec %s: %s.\n", shell_file,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?safe_strerror (errno));
> - ? ? ? ? gdb_flush (gdb_stderr);
> - ? ? ? ? _exit (0177);
> ? ? ? ?}
> ? ? ? else
> ? ? ? ?{
> - ? ? ? ? /* Otherwise, we directly exec the target program with
> - ? ? ? ? ? ?execvp. ?*/
> ? ? ? ? ?int i;
>
> - ? ? ? ? execvp (exec_file, argv);
> -
> - ? ? ? ? /* If we get here, it's an error. ?*/
> ? ? ? ? ?safe_strerror (errno);
> ? ? ? ? ?fprintf_unfiltered (gdb_stderr, "Cannot exec %s ", exec_file);
>
> @@ -383,9 +381,9 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
> ? ? ? ? ? ? ?i++;
> ? ? ? ? ? ?}
> ? ? ? ? ?fprintf_unfiltered (gdb_stderr, ".\n");
> - ? ? ? ? gdb_flush (gdb_stderr);
> - ? ? ? ? _exit (0177);
> ? ? ? ?}
> + ? ? ?gdb_flush (gdb_stderr);
> + ? ? ?_exit (0177);
> ? ? }
>
> ? /* Restore our environment in case a vforked child clob'd it. ?*/
>

Regards,
Abhijit Halder


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]