Allow for quoted program arguments (and executable).

In loadServiceRecord(), the 'command' setting was parsed and unquoted
resulting in a single string. Now, the beginning and end of each part
(i.e. the executable and each argument) are recorded so they can be
stored as part of the service record, and nul terminators are stored
behind each part.

diff --git a/load_service.cc b/load_service.cc
index ce9df910a6a20245ea6f2318ecb30d70b396caa4..95fd5b0791188eb6d293ee953d3687403ea1ed75 100644 (file)
--- a/load_service.cc
+++ b/load_service.cc
@@ -63,7 +63,7 @@ static string read_setting_name(string_iterator & i, string_iterator end)
 //    part_positions -  list of <int,int> to which the position of each setting value
 //                      part will be added as [start,end). May be null.
 static string read_setting_value(string_iterator & i, string_iterator end,
-        std::list<std::pair<int,int>> * part_positions = nullptr)
+        std::list<std::pair<unsigned,unsigned>> * part_positions = nullptr)
 {
     using std::locale;
     using std::isspace;
@@ -152,16 +152,40 @@ static string read_setting_value(string_iterator & i, string_iterator end,
         }
         ++i;
     }
-    
+
+    // Got to end:
+    if (part_positions != nullptr) {
+        part_positions->emplace_back(part_start, rval.length());
+    }
+
     return rval;
 }
 
 
+// Given a string and a list of pairs of (start,end) indices for each argument in that string,
+// store a null terminator for the argument. Return a `char *` array pointing at the beginning
+// of each argument. (The returned array is invalidated if the string is later modified).
+static const char ** separate_args(string &s, std::list<std::pair<unsigned,unsigned>> &arg_indices)
+{
+    const char * cstr = s.c_str();
+    const char ** r = new const char *[arg_indices.size()];
+    int i = 0;
+    for (auto index_pair : arg_indices) {
+        r[i] = cstr + index_pair.first;
+        if (index_pair.second < s.length()) {
+            s[index_pair.second] = 0;
+        }
+        i++;
+    }
+    return r;
+}
+
 // Find a service record, or load it from file. If the service has
 // dependencies, load those also.
 //
 // Might throw a ServiceLoadExc exception if a dependency cycle is found or if another
-// problem occurs (I/O error, service description not found etc).
+// problem occurs (I/O error, service description not found etc). Throws std::bad_alloc
+// if a memory allocation failure occurs.
 ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
 {
     using std::string;
@@ -188,6 +212,9 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
     service_filename += name;
     
     string command;
+    const char ** commands = nullptr;
+    int num_args = 0;
+
     int service_type = SVC_PROCESS;
     std::list<ServiceRecord *> depends_on;
     std::list<ServiceRecord *> depends_soft;
@@ -230,7 +257,10 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
             i = skipws(++i, end);
             
             if (setting == "command") {
-                command = read_setting_value(i, end);
+                std::list<std::pair<unsigned,unsigned>> indices;
+                command = read_setting_value(i, end, &indices);
+                commands = separate_args(command, indices);
+                num_args = indices.size();
             }
             else if (setting == "depends-on") {
                 string dependency_name = read_setting_value(i, end);
@@ -277,7 +307,7 @@ ServiceRecord * ServiceSet::loadServiceRecord(const char * name)
         if (*iter == rval) {
             // We've found the dummy record
             delete rval;
-            rval = new ServiceRecord(this, string(name), service_type, command,
+            rval = new ServiceRecord(this, string(name), service_type, command, commands, num_args,
                     & depends_on, & depends_soft);
             rval->setLogfile(logfile);
             rval->setAutoRestart(auto_restart);

diff --git a/service.cc b/service.cc
index 7d95ccb92a9e85016cc7fd1ad69ca9717027bd58..9216bd07a17fa47dc90a06522974cc322c7244b3 100644 (file)
--- a/service.cc
+++ b/service.cc
@@ -8,15 +8,6 @@
 #include <fcntl.h>
 #include <unistd.h>
 
-// Tokenize a string, allow quoting
-// TODO doesn't yet allow quoting...
-static std::vector<std::string> tokenize(std::string arg)
-{
-    // TODO rewrite to be more efficient.
-    using namespace std;
-    istringstream iss(arg);
-    return vector<string>(istream_iterator<string>(iss), istream_iterator<string>());
-}
 
 // Find the requested service by name
 static ServiceRecord * findService(const std::list<ServiceRecord *> & records,
@@ -182,7 +173,6 @@ void ServiceRecord::start()
             }
         }
     }
-    
 
     if (! all_deps_started) {
         // The dependencies will notify this service once they've started.
@@ -316,19 +306,18 @@ bool ServiceRecord::start_ps_process(const std::vector<std::string> &pargs)
           dup2(1, 2);
         }
         
-        // Tokenize the command, and add additional arguments from pargs:
-        vector<string> progAndArgs = tokenize(program_name);
-        progAndArgs.insert(progAndArgs.end(), pargs.begin(), pargs.end());
-       
-        const char * pname = progAndArgs[0].c_str();
-        const char ** args = new const char *[progAndArgs.size() + 1];
-        
-        for (std::vector<std::string>::size_type i = 0; i < progAndArgs.size(); i++) {
-            args[i] = progAndArgs[i].c_str();
+        const char ** args = new const char *[num_args + pargs.size() + 1];
+        int i;
+        for (i = 0; i < num_args; i++) {
+            args[i] = exec_arg_parts[i];
+        }
+        for (auto progarg : pargs) {
+            args[i] = progarg.c_str();
+            i++;
         }
-        args[progAndArgs.size()] = nullptr;
+        args[i] = nullptr;
         
-        execvp(pname, const_cast<char **>(args));
+        execvp(exec_arg_parts[0], const_cast<char **>(args));
         
         // If we got here, the exec failed:        
         int exec_status = errno;

diff --git a/service.h b/service.h
index ee2833c75dac1f08c4f7ff01330463bba3cc595a..a8c34da30579d217dd290274f2b0cf1e32e68b73 100644 (file)
--- a/service.h
+++ b/service.h
@@ -121,7 +121,11 @@ class ServiceRecord
     bool force_stop; // true if the service must actually stop. This is the
                      // case if for example the process dies; the service,
                      // and all its dependencies, MUST be stopped.
-    string program_name;  /* executable program or script */
+
+    string program_name;          /* storage for program/script and arguments */
+    const char **exec_arg_parts;  /* pointer to each argument/part of the program_name */
+    int num_args;                 /* number of argumrnets (including program) */
+
     string logfile; /* log file name, empty string specifies /dev/null */
     bool auto_restart; /* whether to restart this (process) if it dies unexpectedly */
     
@@ -199,16 +203,19 @@ class ServiceRecord
         service_type = SVC_DUMMY;
     }
     
-    ServiceRecord(ServiceSet *set, string name, int service_type, string command,
-            sr_list * pdepends_on, sr_list * pdepends_soft)
+    ServiceRecord(ServiceSet *set, string name, int service_type, string command, const char ** commands,
+            int num_argsx, sr_list * pdepends_on, sr_list * pdepends_soft)
         : service_state(SVC_STOPPED), desired_state(SVC_STOPPED), force_stop(false), auto_restart(false)
     {
         service_set = set;
         service_name = name;
         this->service_type = service_type;
-        program_name = command;
         this->depends_on = std::move(*pdepends_on);
 
+        program_name = command;
+        exec_arg_parts = commands;
+        num_args = num_argsx;
+
         for (sr_iter i = pdepends_on->begin(); i != pdepends_on->end(); ++i) {
             (*i)->dependents.push_back(this);
         }
@@ -222,6 +229,8 @@ class ServiceRecord
         }
     }
     
+    // TODO write a destructor
+
     // Set logfile, should be done before service is started
     void setLogfile(string logfile)
     {