Refactoring: split loading service from directory into seperate class

dirload_service_set, a new class, extends from service_set and overloads
service load functionality. service_set's standard load_service function
now simply errors out.

diff --git a/src/control.cc b/src/control.cc
index a700587dc92a93c8ab2b34f4b7d745cafd8a4980..2ce4ffee794db3034f3fd58c26edca4e8fa5f3fe 100644 (file)
--- a/src/control.cc
+++ b/src/control.cc
@@ -94,7 +94,7 @@ bool control_conn_t::processFindLoad(int pktType)
     if (pktType == DINIT_CP_LOADSERVICE) {
         // LOADSERVICE
         try {
-            record = services->loadService(serviceName);
+            record = services->load_service(serviceName.c_str());
         }
         catch (service_load_exc &slexc) {
             log(LogLevel::ERROR, "Could not load service ", slexc.serviceName, ": ", slexc.excDescription);

diff --git a/src/control.h b/src/control.h
index c2cf0e3c08a6b1a459bbd63b120b391a644f6972..be94fd89d4bb346597f3d20f391a3da606b55408 100644 (file)
--- a/src/control.h
+++ b/src/control.h
@@ -204,7 +204,8 @@ class control_conn_t : private service_listener
     }
     
     public:
-    control_conn_t(eventloop_t * loop, service_set * services_p, int fd) : loop(loop), services(services_p), chklen(0)
+    control_conn_t(eventloop_t * loop, service_set * services_p, int fd)
+            : loop(loop), services(services_p), chklen(0)
     {
         iob.registerWith(*loop, fd, IN_EVENTS);
         active_control_conns++;

diff --git a/src/dinit.cc b/src/dinit.cc
index bb9f6803bc948d935fdd25acceae5e1377b6dd4f..d7221ea1cef32b0a4b119a8d58bec5510238d5d7 100644 (file)
--- a/src/dinit.cc
+++ b/src/dinit.cc
@@ -57,7 +57,7 @@ void setup_external_log() noexcept;
 
 // Variables
 
-static service_set *services;
+static dirload_service_set *services;
 
 static bool am_system_init = false; // true if we are the system init process
 
@@ -301,13 +301,13 @@ static int dinit_main(int argc, char **argv)
 #endif
     
     /* start requested services */
-    services = new service_set(service_dir);
+    services = new dirload_service_set(service_dir);
     
     init_log(services);
     
     for (auto svc : services_to_start) {
         try {
-            services->startService(svc);
+            services->start_service(svc);
             // Note in general if we fail to start a service we don't need any special error handling,
             // since we either leave other services running or, if it was the only service, then no
             // services will be running and we will process normally (reboot if system process,
@@ -363,7 +363,7 @@ static int dinit_main(int argc, char **argv)
             // user has now exited the shell. We'll try and re-start the
             // boot process...
             try {
-                services->startService("boot");
+                services->start_service("boot");
                 goto event_loop; // yes, the "evil" goto
             }
             catch (...) {

diff --git a/src/load_service.cc b/src/load_service.cc
index 6d8ce3eb125d620d926850f0c5c99d4c8e1e65f0..496341da878eee19c5799b2e6c175c09f5ab107b 100644 (file)
--- a/src/load_service.cc
+++ b/src/load_service.cc
@@ -352,7 +352,7 @@ static void parse_timespec(const std::string &paramval, const std::string &servi
 // Might throw a ServiceLoadExc exception if a dependency cycle is found or if another
 // problem occurs (I/O error, service description not found etc). Throws std::bad_alloc
 // if a memory allocation failure occurs.
-service_record * service_set::loadServiceRecord(const char * name)
+service_record * dirload_service_set::load_service(const char * name)
 {
     using std::string;
     using std::ifstream;
@@ -418,7 +418,7 @@ service_record * service_set::loadServiceRecord(const char * name)
     
     // Add a dummy service record now to prevent infinite recursion in case of cyclic dependency
     rval = new service_record(this, string(name));
-    records.push_back(rval);
+    add_service(rval);
     
     try {
         // getline can set failbit if it reaches end-of-file, we don't want an exception in that case:
@@ -476,11 +476,11 @@ service_record * service_set::loadServiceRecord(const char * name)
                 }
                 else if (setting == "depends-on") {
                     string dependency_name = read_setting_value(i, end);
-                    depends_on.push_back(loadServiceRecord(dependency_name.c_str()));
+                    depends_on.push_back(load_service(dependency_name.c_str()));
                 }
                 else if (setting == "waits-for") {
                     string dependency_name = read_setting_value(i, end);
-                    depends_soft.push_back(loadServiceRecord(dependency_name.c_str()));
+                    depends_soft.push_back(load_service(dependency_name.c_str()));
                 }
                 else if (setting == "logfile") {
                     logfile = read_setting_value(i, end);

diff --git a/src/service.cc b/src/service.cc
index d85ec6c0fb343e3b6503556d8e44c7123b929733..21e8af43508a8a9900151978f3c01dfc2d9e0787 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -47,15 +47,6 @@ service_record * service_set::find_service(const std::string &name) noexcept
     return ::find_service(records, name.c_str());
 }
 
-void service_set::startService(const char *name)
-{
-    using namespace std;
-    service_record *record = loadServiceRecord(name);
-    
-    record->start();
-    processQueues(true);
-}
-
 void service_set::stopService(const std::string & name) noexcept
 {
     service_record *record = find_service(name);

diff --git a/src/service.h b/src/service.h
index de32c6f0d130bb746b5d667c40f9b8f96e3974ed..1828d52081b303907f03b55afcc1530602e000fd 100644 (file)
--- a/src/service.h
+++ b/src/service.h
@@ -428,8 +428,8 @@ class service_record
         service_name = name;
         record_type = service_type::DUMMY;
     }
-    
-    service_record(service_set *set, string name, service_type record_type_p, string &&command, std::list<std::pair<unsigned,unsigned>> &command_offsets,
+
+    service_record(service_set *set, string name, service_type record_type_p,
             sr_list * pdepends_on, sr_list * pdepends_soft)
         : service_record(set, name)
     {
@@ -438,9 +438,6 @@ class service_record
         this->record_type = record_type_p;
         this->depends_on = std::move(*pdepends_on);
 
-        program_name = std::move(command);
-        exec_arg_parts = separate_args(program_name, command_offsets);
-
         for (sr_iter i = depends_on.begin(); i != depends_on.end(); ++i) {
             (*i)->dependents.push_back(this);
         }
@@ -454,6 +451,14 @@ class service_record
         }
     }
     
+    service_record(service_set *set, string name, service_type record_type_p, string &&command, std::list<std::pair<unsigned,unsigned>> &command_offsets,
+            sr_list * pdepends_on, sr_list * pdepends_soft)
+        : service_record(set, name, record_type_p, pdepends_on, pdepends_soft)
+    {
+        program_name = std::move(command);
+        exec_arg_parts = separate_args(program_name, command_offsets);
+    }
+    
     virtual ~service_record() noexcept
     {
     }
@@ -757,9 +762,9 @@ inline auto extract_console_queue(service_record *sr) -> decltype(sr->console_qu
  */
 class service_set
 {
+    protected:
     int active_services;
     std::list<service_record *> records;
-    const char *service_dir;  // directory containing service descriptions
     bool restart_enabled; // whether automatic restart is enabled (allowed)
     
     shutdown_type_t shutdown_type = shutdown_type_t::CONTINUE;  // Shutdown type, if stopping
@@ -771,49 +776,60 @@ class service_set
     slist<service_record, extract_prop_queue> prop_queue;
     slist<service_record, extract_stop_queue> stop_queue;
     
-    // Private methods
-        
+    public:
+    service_set()
+    {
+        active_services = 0;
+        restart_enabled = true;
+    }
+    
+    // Start the specified service
+    void start_service(service_record *svc)
+    {
+        svc->start();
+        processQueues();
+    }
+
+    // Locate an existing service record.
+    service_record *find_service(const std::string &name) noexcept;
+
     // Load a service description, and dependencies, if there is no existing
     // record for the given name.
     // Throws:
     //   ServiceLoadException (or subclass) on problem with service description
     //   std::bad_alloc on out-of-memory condition
-    service_record *loadServiceRecord(const char *name);
-
-    // Public
-    
-    public:
-    service_set(const char *service_dir)
+    virtual service_record *load_service(const char *name)
     {
-        this->service_dir = service_dir;
-        active_services = 0;
-        restart_enabled = true;
+        auto r = find_service(name);
+        if (r == nullptr) {
+            throw service_not_found(name);
+        }
+        return r;
     }
-    
+
     // Start the service with the given name. The named service will begin
     // transition to the 'started' state.
     //
     // Throws a ServiceLoadException (or subclass) if the service description
     // cannot be loaded or is invalid;
     // Throws std::bad_alloc if out of memory.
-    void startService(const char *name);
-    
-    // Locate an existing service record.
-    service_record *find_service(const std::string &name) noexcept;
+    void start_service(const char *name)
+    {
+        using namespace std;
+        service_record *record = load_service(name);
+        service_set::start_service(record);
+    }
     
-    // Find a loaded service record, or load it if it is not loaded.
-    // Throws:
-    //   ServiceLoadException (or subclass) on problem with service description
-    //   std::bad_alloc on out-of-memory condition 
-    service_record *loadService(const std::string &name)
+    void add_service(service_record *svc)
     {
-        service_record *record = find_service(name);
-        if (record == nullptr) {
-            record = loadServiceRecord(name.c_str());
-        }
-        return record;
+        records.push_back(svc);
     }
     
+    void remove_service(service_record *svc)
+    {
+        std::remove(records.begin(), records.end(), svc);
+    }
+
     // Get the list of all loaded services.
     const std::list<service_record *> &listServices()
     {
@@ -937,4 +953,16 @@ class service_set
     }
 };
 
+class dirload_service_set : public service_set
+{
+    const char *service_dir;  // directory containing service descriptions
+
+    public:
+    dirload_service_set(const char *service_dir_p) : service_set(), service_dir(service_dir_p)
+    {
+    }
+
+    service_record *load_service(const char *name) override;
+};
+
 #endif