From 3ec9a9f0799cdb0c69ed671a57c0dbd5937cbd52 Mon Sep 17 00:00:00 2001
From: Davin McCall <davmac@davmac.org>
Date: Tue, 17 Nov 2015 19:25:37 +0000
Subject: [PATCH] Logging and comment fixes

---
 dinit.cc   | 24 +++++++++---------------
 service.cc | 10 ++++------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/dinit.cc b/dinit.cc
index 51e56b3..ec4c682 100644
--- a/dinit.cc
+++ b/dinit.cc
@@ -15,7 +15,6 @@
 #include "dinit-log.h"
 
 /* TODO: prevent services from respawning too quickly */
-/* TODO: detect/guard against dependency cycles */
 /* TODO: optional automatic restart of services */
 
 /*
@@ -192,12 +191,10 @@ int main(int argc, char **argv)
             service_set->startService(*i);
         }
         catch (ServiceNotFound &snf) {
-            log(LogLevel::ERROR, "Could not find service description: " + snf.serviceName);
-            // TODO catch bad_alloc
+            log(LogLevel::ERROR, "Could not find service description: ", snf.serviceName);
         }
         catch (ServiceLoadExc &sle) {
-            log(LogLevel::ERROR, "Problem loading service description: " + sle.serviceName);
-            // TODO catch bad_alloc
+            log(LogLevel::ERROR, "Problem loading service description: ", sle.serviceName);
         }
     }
     
@@ -206,6 +203,7 @@ int main(int argc, char **argv)
     // Process events until all services have terminated.
     while (service_set->count_active_services() != 0) {
         ev_loop(loop, EVLOOP_ONESHOT);
+        log(LogLevel::ERROR, std::string("Got event, active service count now = ") + std::to_string(service_set->count_active_services())); // DAV
     }
     
     if (am_system_init) {
@@ -222,6 +220,7 @@ int main(int argc, char **argv)
         cout << endl;
     }
     
+    log(LogLevel::ERROR, "No more services"); // DAV
     
     if (am_system_init) {
         if (reboot) {
@@ -301,14 +300,12 @@ void open_control_socket(struct ev_loop *loop)
 
         int sockfd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
         if (sockfd == -1) {
-            log(LogLevel::ERROR, std::string("Error creating control socket: ") + strerror(errno));
-            // TODO catch/prevent bad_alloc
+            log(LogLevel::ERROR, "Error creating control socket: ", strerror(errno));
             return;
         }
 
         if (bind(sockfd, (struct sockaddr *) &name, namelen) == -1) {
-            log(LogLevel::ERROR, std::string("Error binding control socket: ") + strerror(errno));
-            // TODO catch/prevent bad_alloc
+            log(LogLevel::ERROR, "Error binding control socket: ", strerror(errno));
             close(sockfd);
             return;
         }
@@ -316,15 +313,13 @@ void open_control_socket(struct ev_loop *loop)
         // No connections can be made until we listen, so it is fine to change the permissions now
         // (and anyway there is no way to atomically create the socket and set permissions):
         if (chmod(saddrname, S_IRUSR | S_IWUSR) == -1) {
-            log(LogLevel::ERROR, std::string("Error setting control socket permissions: ") + strerror(errno));
-            // TODO catch/prevent bad_alloc
+            log(LogLevel::ERROR, "Error setting control socket permissions: ", strerror(errno));
             close(sockfd);
             return;
         }
 
         if (listen(sockfd, 10) == -1) {
-            log(LogLevel::ERROR, std::string("Error listening on control socket: ") + strerror(errno));
-            // TODO catch/prevent bad_alloc
+            log(LogLevel::ERROR, "Error listening on control socket: ", strerror(errno));
             close(sockfd);
             return;
         }
@@ -350,8 +345,7 @@ static void sigquit_cb(struct ev_loop *loop, ev_signal *w, int revents)
     // unlinked. In that case the kernel holds the binary open, so that it can't be
     // properly removed.
     execl("/sbin/shutdown", "/sbin/shutdown", (char *) 0);
-    log(LogLevel::ERROR, std::string("Error executing /sbin/shutdown: ") + strerror(errno));
-    // TODO catch/prevent bad_alloc
+    log(LogLevel::ERROR, "Error executing /sbin/shutdown: ", strerror(errno));
 }
 
 /* handle SIGTERM - stop all services */
diff --git a/service.cc b/service.cc
index 78dbe57..feb9935 100644
--- a/service.cc
+++ b/service.cc
@@ -101,9 +101,9 @@ void ServiceRecord::process_child_callback(struct ev_loop *loop, ev_child *w, in
                 sr->stopped();
             }
             else {
-                // TODO
-                // ??? failed to stop!
-                // For now just pretend we stopped, so that any dependencies
+                // ??? failed to stop! Let's log it as info:
+                log(LogLevel::INFO, "service ", sr->service_name, " stop command failed with exit code ", w->rstatus);
+                // Just assume that we stopped, so that any dependencies
                 // can be stopped:
                 sr->stopped();
             }
@@ -276,8 +276,6 @@ bool ServiceRecord::start_ps_process() noexcept
     }
 }
 
-// TODO this can currently throw std::bad_alloc, fix that (in the worst case,
-//      return failure instead).
 bool ServiceRecord::start_ps_process(const std::vector<std::string> &pargs) noexcept
 {
     // In general, you can't tell whether fork/exec is successful. We use a pipe to communicate
@@ -378,7 +376,7 @@ bool ServiceRecord::start_ps_process(const std::vector<std::string> &pargs) noex
         }
     }
     catch (std::bad_alloc &bad_alloc_exc) {
-        // TODO log error
+        log(LogLevel::ERROR, "Out of memory");
         return false;
     }
 }
-- 
2.25.1