Re: [RFA] Preliminary work in fork_inferior

[Tristan Gingold <gingold at adacore dot com>]

On Sep 16, 2011, at 3:19 PM, Pedro Alves wrote:

> Thanks, this is okay.
> 
> On Friday 16 September 2011 13:28:59, Tristan Gingold wrote:
>> +      /* 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));
> 
> I wouldn't mind merging the "Cannot exec" bits of both branches as well,
> always printing the whole argv.

Ok, so more cleanup.  Something like that ?

Tristan.

diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index bb173e7..5dbf1f7 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
@@ -141,6 +139,8 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env,
   static char **argv;
   const char *inferior_io_terminal = get_inferior_io_terminal ();
   struct inferior *inf;
+  int i;
+  int save_errno;
 
   /* If no exec file handed to us, get it from the exec-file command
      -- with a good, common error message if none is specified.  */
@@ -162,16 +162,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 +170,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 +258,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,44 +359,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>".  */
-      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);
-
-	  i = 1;
-	  while (argv[i] != NULL)
-	    {
-	      if (i != 1)
-		fprintf_unfiltered (gdb_stderr, " ");
-	      fprintf_unfiltered (gdb_stderr, "%s", argv[i]);
-	      i++;
-	    }
-	  fprintf_unfiltered (gdb_stderr, ".\n");
-	  gdb_flush (gdb_stderr);
-	  _exit (0177);
-	}
+      execvp (argv[0], argv);
+
+      /* If we get here, it's an error.  */
+      save_errno = errno;
+      fprintf_unfiltered (gdb_stderr, "Cannot exec %s", exec_file);
+      for (i = 1; argv[i] != NULL; i++)
+	fprintf_unfiltered (gdb_stderr, " %s", argv[i]);
+      fprintf_unfiltered (gdb_stderr, ".\n");
+      fprintf_unfiltered (gdb_stderr, "Error: %s\n",
+			  safe_strerror (save_errno));
+      gdb_flush (gdb_stderr);
+      _exit (0177);
     }
 
   /* Restore our environment in case a vforked child clob'd it.  */