Re: [RFC] Patch implementing EXECUTE_COMMAND_LINE

[Steve Kargl <sgk at troutmask dot apl dot washington dot edu>]

On Mon, May 04, 2009 at 12:07:18AM +0200, FX wrote:
> 
> As you can see from my few emails today, I had a bit of free time and  
> needed something that would take me away from my day-to-day  
> programming, so I worked on gfortran a bit :)
> 
> Here's one patch that isn't in final state yet, but I probably won't  
> have time to submit it formally before a few weeks. It implements  
> EXECUTE_COMMAND_LINE, a new F2008 intrinsic that should replace the  
> old SYSTEM extension in user code. My patch comes with a complete  
> implementation, but no testcase and no doc. I tested it manually and  
> it seems to work OK. I'd be glad to have people build it on different  
> systems, though, at it uses some UNIX-specific stuff that I'm never  
> too sure to get right on the first time.
> 
> So, if someone has time (although I know it's unlikely) and want to  
> fly with it, please go ahead. Otherwise, I'll submit it formally when  
> I get time. In the meantime, any testing you can do (and report here)  
> will be of help.
> 

I think there may be problems with the asynchronous path.  Here's my 
thoughts on the implementation.


+
+  /* Flush all I/O units before executing the command.  */
+  flush_all_units();

Missing space before ().

+#if defined(HAVE_FORK)
+  if (!wait)
+    {
+      /* Asynchronous execution.  */
+      pid_t pid;
+
+      *cmdstat = 0;
+      if ((pid = fork()) < 0)
+	*cmdstat = EXEC_SYSTEMFAILED;
+      else if (pid == 0)
+	{
+	  /* Child process.  */
+	  int res = system (cmd);
+	  _exit (WIFEXITED(res) ? WEXITSTATUS(res) : res);
+	}
+    }
+  else

Are you sure that this is asynchronous?  FreeBSD's manpage
indicates that system(3) is synchronous. 

man 3 system

     The system() function hands the argument string to the command inter-
     preter sh(1).  The calling process waits for the shell to finish execut-
     ing the command, ignoring SIGINT and SIGQUIT, and blocking SIGCHLD.

In a piece of code I wrote a long time ago (before I started to
use OpenMPI), I would fork np processes and not wait for the children.
The code is (note, nu is the total number of processes that I 
will eventual run).

            /*
            *  Fork a child process to run scat.
            */
            if (i + k <= nu) {
                switch((child_pid = fork())) {
                    case -1:
                        err(1, "Problem creating child process");
                        break; 
                    case  0:
                        execlp("scat", "scat", NULL);
                        err(1, "Child process returned");
                        break;
                    default:
                        /* Parent won't wait for the the child to return. */
                        if (k == np - 1 || i + k == nu) {
                            do {
                                wait_pid = wait(&status);
                                if (wait_pid == -1)
                                    err(1, "Abnormal child status");
                            } while (wait_pid != child_pid);
                        }
                }

                if (!WIFEXITED(status) || WIFSIGNALED(status))
                    err(1, "Abnormal termination of child");

            }

+#endif
+    {
+      /* Synchronous execution.  */
+      int res = system (cmd);
+
+      if (!wait)
+	*cmdstat = -2;
+      else if (res == -1)
+	*cmdstat = EXEC_SYSTEMFAILED;
+      else
+	{
+	  *cmdstat = 0;
+#if defined(WEXITSTATUS) && defined(WIFEXITED)
+	  *exitstat = WIFEXITED(res) ? WEXITSTATUS(res) : res;
+#else
+	  *exitstat = res;
+#endif
+	}
+    }

The synchronous execution is probably OK.  But, it might be possible to
combine both into something like the switch statement I have above, e.g.,


            /*
            *  Fork a child process.
            */
            status = 0;
            switch((child_pid = fork())) {
                    case -1:
                        err(1, "Problem creating child process");
                        break; 
                    case  0:
                        execlp("scat", "scat", NULL);
                        err(1, "Child process returned");
                        break;
                    default:
                        /* Should the parent wait or not for the child. */
                        if (!wait_for_child)
                            do {
                                wait_pid = wait(&status);
                                if (wait_pid == -1)
                                    err(1, "Abnormal child status");
                            } while (wait_pid != child_pid);
                        }
             }

             if (!WIFEXITED(status) || WIFSIGNALED(status))
                 err(1, "Abnormal termination of child");

Note, this changes your wait variable to wait_for_child because wait()
is a system call.

-- 
Steve