[RFA] Preliminary work in fork_inferior

[Tristan Gingold <gingold@adacore.com>]

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;
-  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));
       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 *));
+      argv[0] = shell_file;
+      argv[1] = "-c";
+      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.  */