Clean-up the forked process code a little.

git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@8536 a592a061-630c-0410-9148-cb99ea01b6c8

diff --git a/src/ChangeLog b/src/ChangeLog
index 6ac277306fac70398034ec298561ce4331fd01e2..3eaee3c7085056fe2518d8037b921af634c04fc8 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2004-03-26  Angus Leeming  <leeming@lyx.org>
+
+       * ispell.C (LaunchIspell::start):
+       * lyx_cb.C (AutoSaveBuffer::start):
+       invoke run(DontWait) rather than runNonBlocking().
 
 2004-03-26  Alfredo Braunstein  <abraunst@lyx.org>
 

diff --git a/src/ispell.C b/src/ispell.C
index 08de736d831aedb0491d357087913f720d84cebc..6a2aac96b497963bcf9fbe71ee6630f8ee906baa 100644 (file)
--- a/src/ispell.C
+++ b/src/ispell.C
@@ -74,7 +74,7 @@ private:
 int LaunchIspell::start()
 {
        command_ = lyxrc.isp_command;
-       return runNonBlocking();
+       return run(DontWait);
 }
 
 

diff --git a/src/lyx_cb.C b/src/lyx_cb.C
index 9009677032c6919e29a9c677a1e4b12d249e2381..213ff3967687d8eb375f36ec1a2f8c5236375742 100644 (file)
--- a/src/lyx_cb.C
+++ b/src/lyx_cb.C
@@ -242,7 +242,7 @@ private:
 int AutoSaveBuffer::start()
 {
        command_ = bformat(_("Auto-saving %1$s"), fname_);
-       return runNonBlocking();
+       return run(DontWait);
 }
 
 

diff --git a/src/support/ChangeLog b/src/support/ChangeLog
index d55058c51bd014cf15021664ea0e524ec4bc3e04..62b825b6612b5f27f870c695302eb84af249c322 100644 (file)
--- a/src/support/ChangeLog
+++ b/src/support/ChangeLog
@@ -1,3 +1,14 @@
+2004-03-26  Angus Leeming  <leeming@lyx.org>
+
+       * forkedcall.[Ch] (run): new function, replacing runBlocking,
+       runNonBlocking.
+
+       * forkedcall.C (generateChild): ensure that the code that splits
+       the command up into an array of words won't leak in the event of an
+       exception.
+
+       * forkedcontr.C: make it a little more robust.
+       
 2004-03-24  Angus Leeming  <leeming@lyx.org>
 
        * forkedcontr.[Ch]: get rid of the timer that we use to poll the list

diff --git a/src/support/forkedcall.C b/src/support/forkedcall.C
index e768d81f8a5baca65ff276707432ec1d2f0a2dfa..188019d2d285a22c64f15b41d28b4dc458f281e6 100644 (file)
--- a/src/support/forkedcall.C
+++ b/src/support/forkedcall.C
@@ -36,15 +36,17 @@
 #include <boost/bind.hpp>
 
 #include <cerrno>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <csignal>
 #include <cstdlib>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
+#include <vector>
 
-using std::string;
 using std::endl;
+using std::string;
+using std::vector;
 
 #ifndef CXX_GLOBAL_CSTD
 using std::strerror;
@@ -115,8 +117,8 @@ void ForkedProcess::emitSignal()
 }
 
 
-// Wait for child process to finish.
-int ForkedProcess::runBlocking()
+// Spawn the child process
+int ForkedProcess::run(Starttype type)
 {
        retval_  = 0;
        pid_ = generateChild();
@@ -125,25 +127,17 @@ int ForkedProcess::runBlocking()
                return retval_;
        }
 
-       retval_ = waitForChild();
-       return retval_;
-}
-
-
-// Do not wait for child process to finish.
-int ForkedProcess::runNonBlocking()
-{
-       retval_ = 0;
-       pid_ = generateChild();
-       if (pid_ <= 0) { // child or fork failed.
-               retval_ = 1;
-               return retval_;
+       switch (type) {
+       case Wait:
+               retval_ = waitForChild();
+               break;
+       case DontWait: {
+               // Integrate into the Controller
+               ForkedcallsController & contr = ForkedcallsController::get();
+               contr.addCall(*this);
+               break;
+       }
        }
-
-       // Non-blocking execution.
-       // Integrate into the Controller
-       ForkedcallsController & contr = ForkedcallsController::get();
-       contr.addCall(*this);
 
        return retval_;
 }
@@ -238,7 +232,7 @@ int Forkedcall::startscript(Starttype wait, string const & what)
 
        command_ = what;
        signal_.reset();
-       return runBlocking();
+       return run(Wait);
 }
 
 
@@ -247,38 +241,42 @@ int Forkedcall::startscript(string const & what, SignalTypePtr signal)
        command_ = what;
        signal_  = signal;
 
-       return runNonBlocking();
+       return run(DontWait);
 }
 
 
 // generate child in background
 int Forkedcall::generateChild()
 {
-       // Split command_ up into a char * array
-       int const MAX_ARGV = 255;
-       char *argv[MAX_ARGV];
-
-       string line = command_;
-       int index = 0;
-       for (; index < MAX_ARGV-1; ++index) {
-               string word;
-               line = split(line, word, ' ');
-               if (word.empty())
-                       break;
-
-               char * tmp = new char[word.length() + 1];
-               word.copy(tmp, word.length());
-               tmp[word.length()] = '\0';
-
-               argv[index] = tmp;
+       string line = trim(command_);
+       if (line.empty())
+               return 1;
+
+       // Split the input command up into an array of words stored
+       // in a contiguous block of memory.
+       char const * const c_str = line.c_str();
+       // Don't forget the terminating `\0' character.
+       vector<char> vec(c_str, c_str + line.size() + 1);
+       // Turn the string into an array of words, each terminated with '\0'.
+       std::replace(vec.begin(), vec.end(), ' ', '\0');
+
+       // Build an array of pointers to each word.
+       vector<char>::iterator vit = vec.begin();
+       vector<char>::iterator vend = vec.end();
+       vector<char *> argv;
+       char prev = '\0';
+       for (; vit != vend; ++vit) {
+               if (*vit != '\0' && prev == '\0')
+                       argv.push_back(&*vit);
+               prev = *vit;
        }
-       argv[index] = 0;
+       argv.push_back(0);
 
 #ifndef __EMX__
        pid_t const cpid = ::fork();
        if (cpid == 0) {
                // Child
-               execvp(argv[0], argv);
+               execvp(argv[0], &*argv.begin());
 
                // If something goes wrong, we end up here
                lyxerr << "execvp of \"" << command_ << "\" failed: "
@@ -287,7 +285,7 @@ int Forkedcall::generateChild()
        }
 #else
        pid_t const cpid = spawnvp(P_SESSION|P_DEFAULT|P_MINIMIZE|P_BACKGROUND,
-                                  argv[0], argv);
+                                  argv[0], &*argv.begin());
 #endif
 
        if (cpid < 0) {
@@ -295,13 +293,6 @@ int Forkedcall::generateChild()
                lyxerr << "Could not fork: " << strerror(errno) << endl;
        }
 
-       // Clean-up.
-       for (int i = 0; i < MAX_ARGV; ++i) {
-               if (argv[i] == 0)
-                       break;
-               delete [] argv[i];
-       }
-
        return cpid;
 }
 

diff --git a/src/support/forkedcall.h b/src/support/forkedcall.h
index abe1938b151e20ceaf37b4d35a685984452b1179..a3843e366ae47de223235a2c9ce62498a1af1d73 100644 (file)
--- a/src/support/forkedcall.h
+++ b/src/support/forkedcall.h
@@ -107,14 +107,10 @@ public:
        void kill(int tolerance = 5);
 
 protected:
-       /** Wait for child process to finish.
+       /** Spawn the child process.
         *  Returns returncode from child.
         */
-       int runBlocking();
-       /** Do not wait for child process to finish.
-        *  Returns returncode from child.
-        */
-       int runNonBlocking();
+       int run(Starttype type);
 
        /// Callback function
        SignalTypePtr signal_;

diff --git a/src/support/forkedcontr.C b/src/support/forkedcontr.C
index 9a35444df4340e57bb25edcfc71e86792fe2d9e4..a8074729a94c1d4cfd41893934f90979074851c6 100644 (file)
--- a/src/support/forkedcontr.C
+++ b/src/support/forkedcontr.C
@@ -163,6 +163,11 @@ extern "C"
 void child_handler(int)
 {
        ForkedcallsController & fcc = ForkedcallsController::get();
+
+       // Be safe
+       if (fcc.current_child+1 >= fcc.reaped_children.size())
+               return;
+
        ForkedcallsController::Data & store =
                fcc.reaped_children[++fcc.current_child];
        // Clean up the child process.
@@ -261,7 +266,9 @@ void ForkedcallsController::handleCompletedProcesses()
                }
 
                ListType::iterator it = find_pid(store.pid);
-               BOOST_ASSERT(it != forkedCalls.end());
+               if (it == forkedCalls.end())
+                       // Eg, child was run in blocking mode
+                       continue;
 
                ForkedProcess & child = *it->get();
                bool remove_it = false;