[myserver-commit] [2945] Code refactoring for the `Process' class.

[Giuseppe Scrivano]

Revision: 2945
          http://svn.sv.gnu.org/viewvc/?view=rev&root=myserver&revision=2945
Author:   gscrivano
Date:     2008-11-07 20:26:26 +0000 (Fri, 07 Nov 2008)

Log Message:
-----------
Code refactoring for the `Process' class. The `Process::exec()' function 
doesn't support a timeout anymore, it wouldn't work with the `waitpid' syscall 
without dirty hacks.

Modified Paths:
--------------
    trunk/myserver/include/base/process/process.h
    trunk/myserver/src/base/process/process.cpp

Modified: trunk/myserver/include/base/process/process.h
===================================================================
--- trunk/myserver/include/base/process/process.h       2008-11-07 20:22:14 UTC 
(rev 2944)
+++ trunk/myserver/include/base/process/process.h       2008-11-07 20:26:26 UTC 
(rev 2945)
@@ -61,8 +61,7 @@
        static void forkChild();
 #endif
        static void initialize();
-  int execAndWait (StartProcInfo *spi, u_long timeout = 0xFFFFFFFF);
-  int exec (StartProcInfo *spi, bool waitEnd = false, u_long timeout = 
0xFFFFFFFF);
+  int exec (StartProcInfo *spi, bool waitEnd = false);
   int terminateProcess();
   int isProcessAlive();
   static int setuid(u_long);

Modified: trunk/myserver/src/base/process/process.cpp
===================================================================
--- trunk/myserver/src/base/process/process.cpp 2008-11-07 20:22:14 UTC (rev 
2944)
+++ trunk/myserver/src/base/process/process.cpp 2008-11-07 20:26:26 UTC (rev 
2945)
@@ -61,19 +61,19 @@
   int len = additionalArgs.length ();
   int start = 0;
 
-  while((arg[start] == ' ') && (start < len))
+  while ((arg[start] == ' ') && (start < len))
     start++;
 
-  for(int i = start; i < len; i++)
+  for (int i = start; i < len; i++)
     {
-      if(arg[i] == '"')
+      if (arg[i] == '"')
         {
           start = i++;
-          while(additionalArgs[++i] != '"' && i < len);
-          if(i == len)
+          while (additionalArgs[++i] != '"' && i < len);
+          if (i == len)
             exit(1);
 
-          if(count < 100)
+          if (count < 100)
             {
               args[count++] = &(additionalArgs[start + 1]);
               start = i + 1;
@@ -82,11 +82,11 @@
           else
             break;
         }
-      else if(arg[i] == ' ')
+      else if (arg[i] == ' ')
         {
-          if(i - start <= 1)
+          if (i - start <= 1)
             continue;
-          if(count < 100)
+          if (count < 100)
             {
               args[count++] = &(additionalArgs[start]);
               additionalArgs[i] = '\0';
@@ -100,7 +100,7 @@
             break;
         }
     }
-  if(count < 100 && len != start)
+  if (count < 100 && len != start)
     {
       args[count++] = &(additionalArgs[start]);
     }
@@ -122,7 +122,7 @@
   int index = 0;
  
 
-  if(envString != NULL)
+  if (envString != NULL)
     {
       while(*(envString + i) != '\0')
         {
@@ -150,17 +150,11 @@
  *\return -1 on failure.
  *\return the new process identifier on success.
  */
-int Process::exec(StartProcInfo* spi, bool waitEnd, u_long timeout)
+int Process::exec(StartProcInfo* spi, bool waitEnd)
 {
   int ret = 0;
-#ifdef NOT_WIN
-  u_long count;
-#endif
   pid = 0;
 #ifdef WIN32
-  /*!
-   *Set the standard output values for the CGI process.
-   */
   STARTUPINFO si;
   PROCESS_INFORMATION pi;
   const char *cwd;
@@ -170,16 +164,19 @@
   si.hStdOutput = (HANDLE)spi->stdOut;
   si.hStdError = (HANDLE)spi->stdError;
   si.dwFlags = STARTF_USESHOWWINDOW;
-  if(si.hStdInput || si.hStdOutput ||si.hStdError)
+  if (si.hStdInput || si.hStdOutput ||si.hStdError)
     si.dwFlags |= STARTF_USESTDHANDLES;
   si.wShowWindow = SW_HIDE;
 
   cwd = spi->cwd.length() ? (char*)spi->cwd.c_str() : 0;
   ZeroMemory( &pi, sizeof(pi) );
+
   ret = CreateProcess(NULL, (char*)spi->cmdLine.c_str(), NULL, NULL, TRUE,0,
                       spi->envString, cwd, &si, &pi);
-  if(!ret)
+
+  if (!ret)
     return (-1);
+
   pid = (u_long)pi.hProcess;
 
   if (waitEnd)
@@ -187,31 +184,15 @@
       /*
        *Wait until the process stops its execution.
        */
-      ret = WaitForSingleObject(pi.hProcess, timeout);
+      ret = WaitForSingleObject (pi.hProcess, INFINITE);
 
-      if(ret == WAIT_FAILED)
+      if (ret == WAIT_FAILED)
         return (u_long)(-1);
 
-      ret = CloseHandle( pi.hProcess );
-
-      if(!ret)
-        return (u_long)(-1);
-
-      ret = CloseHandle( pi.hThread );
-
-      if(!ret)
-        return (u_long)(-1);
-
       return 0;
-  }
+    }
   else
     {
-      ret = CreateProcess(NULL, (char*)spi->cmdLine.c_str(), NULL, NULL, TRUE, 
0,
-                          spi->envString, cwd, &si, &pi);
-
-      if(!ret)
-        return (-1);
-
       pid = (*((int*)&pi.hProcess));
 
       return pid;
@@ -219,92 +200,66 @@
 #endif
 
 #ifdef NOT_WIN
-  pid = fork();
+  pid = fork ();
 
-  if(pid < 0) // a bad thing happened
+  if (pid < 0) // a bad thing happened
     return 0;
 
-  if(pid == 0) // child
-  {
-    // Set env vars
-    const char *envp[100];
-    const char *args[100];
-
-    if (generateArgList (args, spi->cmd.c_str(), spi->arg))
-      exit (1);
-
-    if (generateEnvString (envp, (char*) spi->envString))
-      exit (1);
-
-    // change to working dir
-    if(spi->cwd.length())
+  if (pid == 0) // child
     {
-      ret = chdir((const char*)(spi->cwd.c_str()));
-      if(ret == -1)
-        exit(1);
-    }
-     // If stdOut is -1, pipe to /dev/null
-    if((long)spi->stdOut == -1)
-      spi->stdOut = (FileHandle)open("/dev/null", O_WRONLY);
-    // map stdio to files
-    ret = close(0); // close stdin
-    if(ret == -1)
-      exit (1);
-    ret = dup2(spi->stdIn, 0);
-    if(ret == -1)
-      exit (1);
-    ret = close(spi->stdIn);
-    if(ret == -1)
-      exit (1);
-    ret = close(1); // close stdout
-    if(ret == -1)
-      exit (1);
+      // Set env vars
+      const char *envp[100];
+      const char *args[100];
+      
+      if (generateArgList (args, spi->cmd.c_str (), spi->arg))
+        exit (1);
+      
+      if (generateEnvString (envp, (char*) spi->envString))
+        exit (1);
+      
+      // change to working dir
+      if (spi->cwd.length ())
+        {
+          ret = chdir ((const char*)(spi->cwd.c_str()));
+          if (ret == -1)
+            exit (1);
+        }
+
+      // If stdOut is -1, pipe to /dev/null
+      if ((long)spi->stdOut == -1)
+        spi->stdOut = (FileHandle)open ("/dev/null", O_WRONLY);
+      // map stdio to files
+      ret = close(0); // close stdin
+      if (ret == -1)
+        exit (1);
+      ret = dup2(spi->stdIn, 0);
+      if (ret == -1)
+        exit (1);
+      ret = close(spi->stdIn);
+      if (ret == -1)
+        exit (1);
+      ret = close (1); // close stdout
+      if (ret == -1)
+        exit (1);
     ret = dup2(spi->stdOut, 1);
-    if(ret == -1)
+    if (ret == -1)
       exit (1);
-    ret = close(spi->stdOut);
-    if(ret == -1)
-      exit(1);
+    ret = close (spi->stdOut);
+    if (ret == -1)
+      exit (1);
     //close(2); // close stderr
     //dup2((int)spi->stdError, 2);
     // Run the script
-    ret = execve((const char*)args[0], 
+    ret = execve ((const char*)args[0], 
                  (char* const*)args, (char* const*) envp);
-    exit(0);
+    exit (0);
 
   }
 
   if (waitEnd)
-  {
-    //XXX: Fix polling.
-    count = 0;
-    for( ;  ; count++)
-      {
-        if(count >= timeout / 100)
-          {
-            kill(pid, SIGKILL);
-            ret = -1;
-            break;
-          }
-        ret = waitpid(pid, NULL, WNOHANG);
-        if(ret == -1)
-          {
-            if(errno == ECHILD)
-              return 0;
-            else
-              return (-1);
-          }
-        else if(ret != 0)
-          {
-            break;
-          }
-        Thread::wait(100);
-      }
-
-    if(ret == -1)
-      return (-1);
-    return 0;
-  }
+    {
+      return waitpid(pid, NULL, 0);
+    }
   else
     return pid;
 
@@ -317,16 +272,16 @@
  *Return a nonzero value if the process is still alive. A return value of zero
  *means the process is a zombie.
  */
-int Process::isProcessAlive()
+int Process::isProcessAlive ()
 {
-  if(pid == 0)
+  if (pid == 0)
     return 0;
 #ifdef WIN32
   u_long ec;
-  int ret = GetExitCodeProcess(*((HANDLE*)&pid), &ec);
-  if(ret == 0)
+  int ret = GetExitCodeProcess (*((HANDLE*)&pid), &ec);
+  if (ret == 0)
     return 0;
-  if(ec == STILL_ACTIVE)
+  if (ec == STILL_ACTIVE)
     return 1;
   return 0;
 #endif
@@ -337,10 +292,10 @@
   int ret;
   do
   {
-    ret = waitpid(pid, &status, WNOHANG);
+    ret = waitpid (pid, &status, WNOHANG);
   }
-  while(!ret && errno == EINTR);
-  if(!ret)
+  while (!ret && errno == EINTR);
+  if (!ret)
     return 1;
   return 0;
 #else
@@ -348,14 +303,14 @@
   int ret;
   do
   {
-    ret = kill(pid, 0);
+    ret = kill (pid, 0);
   }
-  while(!ret && errno == EINTR);
-  if(ret == 0)
+  while (!ret && errno == EINTR);
+  if (ret == 0)
     return 1;
 
   /*! Waitpid it to free the resource.  */
-  waitpid(pid, &status, WNOHANG | WUNTRACED);
+  waitpid (pid, &status, WNOHANG | WUNTRACED);
 
   return 0;
 #endif
@@ -364,62 +319,48 @@
   return 0;
 }
 
-/*!
- *Execute a process and wait for its execution to be completed or the timeout, 
- *whatever happens before.
- *
- *\param spi The new process information.
- *\param timeout Maximum amount of time to wait for.
- *\return -1 on fails.
- *\return 0 on success.
- */
-int Process::execAndWait (StartProcInfo *spi, u_long timeout)
-{
-  return exec(spi, true, timeout);
-}
-
 #ifdef HAVE_PTHREAD
 
 /*!
  *Called in the parent before do a fork.
  */
-void Process::forkPrepare()
+void Process::forkPrepare ()
 {
-  forkMutex.lock();
+  forkMutex.lock ();
 }
 
 /*!
  *Called in the parent after the fork.
  */
-void Process::forkParent()
+void Process::forkParent ()
 {
-  forkMutex.unlock();
+  forkMutex.unlock ();
 }
 
 /*!
  *Called in the child process after the fork.
  */
-void Process::forkChild()
+void Process::forkChild ()
 {
-  forkMutex.unlock();  
+  forkMutex.unlock ();  
 }
 #endif
 
 /*!
  *Initialize the static process data.
  */
-void Process::initialize()
+void Process::initialize ()
 {
 #ifdef HAVE_PTHREAD
-  forkMutex.init();
-  pthread_atfork(forkPrepare, forkParent, forkChild);
+  forkMutex.init ();
+  pthread_atfork (forkPrepare, forkParent, forkChild);
 #endif
 }
 
 /*!
  *Create the object.
  */
-Process::Process()
+Process::Process ()
 {
   pid = 0;
 }
@@ -427,7 +368,7 @@
 /*!
  *Destroy the object.
  */
-Process::~Process()
+Process::~Process ()
 {
 
 }
@@ -437,18 +378,18 @@
  *Return 0 on success.
  *Return nonzero on fails.
  */
-int Process::terminateProcess()
+int Process::terminateProcess ()
 {
   int ret;
-  if(pid == 0)
+  if (pid == 0)
     return 0;
 #ifdef WIN32
-  ret = TerminateProcess(*((HANDLE*)&pid),0);
+  ret = TerminateProcess (*((HANDLE*)&pid),0);
   pid = 0;
   return (!ret);
 #endif
 #ifdef NOT_WIN
-  ret = kill((pid_t)pid, SIGKILL);
+  ret = kill ((pid_t)pid, SIGKILL);
   pid = 0;
   return ret;
 #endif
@@ -457,10 +398,10 @@
 /*!
  *Set the user identity for the process. Returns 0 on success.
  */
-int Process::setuid(u_long uid)
+int Process::setuid (u_long uid)
 {
 #ifdef NOT_WIN
-  return ::setuid(uid);
+  return ::setuid (uid);
 #endif
   return 0;
 }
@@ -468,10 +409,10 @@
 /*!
  *Set the group identity for the process. Returns 0 on success.
  */
-int Process::setgid(u_long gid)
+int Process::setgid (u_long gid)
 {
 #ifdef NOT_WIN
-  return ::setgid(gid);
+  return ::setgid (gid);
 #endif
   return 0;
 }
@@ -479,7 +420,7 @@
 /*!
  *Set the additional groups list for the process.
  */
-int Process::setAdditionalGroups(u_long len, u_long *groups)
+int Process::setAdditionalGroups (u_long len, u_long *groups)
 {
 #ifdef NOT_WIN
 
@@ -488,17 +429,17 @@
   int ret;
   gid_t *gids = new gid_t[len];
   
-  for(i = 0; i < len; i++)
-    if(groups)
+  for (i = 0; i < len; i++)
+    if (groups)
       gids[i] = (gid_t)groups[i];
     else
       gids[i] = (gid_t)0;
 
-  ret = setgroups((size_t)0, gids) == -1;
+  ret = setgroups ((size_t)0, gids) == -1;
 
   delete [] gids;
 
-  if(errno == EPERM && len == 0)
+  if (errno == EPERM && len == 0)
     return 0;
   return ret;