This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


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

pexecute.c cleanup


pexecute.c has been complaining about uninitialized variables and
variables clobbered by vfork for a very long time.  This patch
corrects both problems.

The uninitialized variables were initialized, but gcc is not sensible
enough to realize it.  This appears to be related to the loop
optimizer being +/- useless:

  for (i = 0; i < 4; i++) {
    var = value;
    /* ... */
    if (condition) break;
  }

  if (i == 4) return;

  /* use var here */

The loop is known to execute at least once, so var will not be used
before it is initialized, but gcc doesn't know that.  If you rewrite
as

  for (i = 0; i < 4; i++) {
    var = value;
    /* ... */
    if (condition) goto label;
  }
  return;

label:
  /* use var here */

then it understands.

The clobbered-by-vfork warnings were correct; it was not safe to use
vfork in pexecute, as it was structured.  (Note, for example, the use
of fprintf and exit in the child.)  I have chosen the simple solution
of using fork instead.  vfork+exec is not significantly faster than
fork+exec on modern systems, and not using it avoids ever so many
problems.

Bootstrapped i386-linux.  OK to install?

zw

	* pexecute.c: Don't use vfork. Restructure retry loop to avoid
	uninitialized-variable warnings.

===================================================================
Index: libiberty/pexecute.c
--- libiberty/pexecute.c	2000/07/27 01:48:33	1.19
+++ libiberty/pexecute.c	2000/08/02 20:28:53
@@ -46,19 +46,6 @@ extern int errno;
 #include <sys/wait.h>
 #endif
 
-#ifdef vfork /* Autoconf may define this to fork for us. */
-# define VFORK_STRING "fork"
-#else
-# define VFORK_STRING "vfork"
-#endif
-#ifdef HAVE_VFORK_H
-#include <vfork.h>
-#endif
-#ifdef VMS
-#define vfork() (decc$$alloc_vfork_blocks() >= 0 ? \
-               lib$get_current_invo_context(decc$$get_vfork_jmpbuf()) : -1)
-#endif /* VMS */
-
 #include "libiberty.h"
 
 /* stdin file number.  */
@@ -716,53 +703,20 @@ pexecute (program, argv, this_pname, tem
   sleep_interval = 1;
   for (retries = 0; retries < 4; retries++)
     {
-      pid = vfork ();
+      pid = fork ();
       if (pid >= 0)
-	break;
+	goto forked;
       sleep (sleep_interval);
       sleep_interval *= 2;
     }
-
-  switch (pid)
-    {
-    case -1:
-      {
-	*errmsg_fmt = VFORK_STRING;
-	*errmsg_arg = NULL;
-	return -1;
-      }
-
-    case 0: /* child */
-      /* Move the input and output pipes into place, if necessary.  */
-      if (input_desc != STDIN_FILE_NO)
-	{
-	  close (STDIN_FILE_NO);
-	  dup (input_desc);
-	  close (input_desc);
-	}
-      if (output_desc != STDOUT_FILE_NO)
-	{
-	  close (STDOUT_FILE_NO);
-	  dup (output_desc);
-	  close (output_desc);
-	}
-
-      /* Close the parent's descs that aren't wanted here.  */
-      if (last_pipe_input != STDIN_FILE_NO)
-	close (last_pipe_input);
 
-      /* Exec the program.  */
-      (*func) (program, argv);
-
-      /* Note: Calling fprintf and exit here doesn't seem right for vfork.  */
-      fprintf (stderr, "%s: ", this_pname);
-      fprintf (stderr, install_error_msg, program);
-      fprintf (stderr, ": %s\n", xstrerror (errno));
-      exit (-1);
-      /* NOTREACHED */
-      return 0;
+  *errmsg_fmt = "fork";
+  *errmsg_arg = NULL;
+  return -1;
 
-    default:
+ forked:
+  if (pid > 0)
+    {
       /* In the parent, after forking.
 	 Close the descriptors that we made for this child.  */
       if (input_desc != STDIN_FILE_NO)
@@ -773,6 +727,34 @@ pexecute (program, argv, this_pname, tem
       /* Return child's process number.  */
       return pid;
     }
+
+  /* In the child, after forking.
+     Move the input and output pipes into place, if necessary.  */
+  if (input_desc != STDIN_FILE_NO)
+    {
+      close (STDIN_FILE_NO);
+      dup (input_desc);
+      close (input_desc);
+    }
+  if (output_desc != STDOUT_FILE_NO)
+    {
+      close (STDOUT_FILE_NO);
+      dup (output_desc);
+      close (output_desc);
+    }
+
+  /* Close the parent's descs that aren't wanted here.  */
+  if (last_pipe_input != STDIN_FILE_NO)
+    close (last_pipe_input);
+
+      /* Exec the program.  */
+  (*func) (program, argv);
+
+  fprintf (stderr, "%s: ", this_pname);
+  fprintf (stderr, install_error_msg, program);
+  fprintf (stderr, ": %s\n", xstrerror (errno));
+  exit (-1);
+  /* NOTREACHED */
 }
 
 int

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