Minor refactoring, additional comments

src/dinit-log.cc

@@ -14,27 +14,24 @@
 // Dinit logging subsystem.
 //
 // Note that most actual functions for logging messages are found in the header, dinit-log.h.
-//
-// We have two separate log "streams": one for the console/stdout, one for the syslog facility (or log
-// file). Both have a circular buffer. Log messages are appended to the circular buffer (for a syslog
-// stream, the messages are prepended with a syslog priority indicator). Both streams start out inactive
-// (release = true in buffered_log_stream), which means they will buffer messages but not write them.
-//
-// The console log stream needs to be able to release the console, if a service is waiting to acquire it.
-// This is accomplished by calling flush_for_release() which then completes the output of the current
-// message (if any) and then assigns the console to a waiting service.
+// See documentation there also.
 
 extern eventloop_t event_loop;
 extern bool external_log_open;
 
-static bool log_current_line[2];  // Whether the current line is being logged (for console, main log)
-static bool log_format_syslog[2] = { false, true };
+static bool log_current_line[DLOG_NUM];  // Whether the current line is being logged (for console, main log)
+static bool log_format_syslog[DLOG_NUM] = { true, false };
+static_assert(DLOG_NUM == 2, "number of log streams has changed");
+static_assert(DLOG_MAIN == 0, "main log index has changed");
 
 static service_set *services = nullptr;  // Reference to service set
 
-loglevel_t log_level[2] = { loglevel_t::NOTICE, loglevel_t::WARN };
+loglevel_t log_level[DLOG_NUM] = { loglevel_t::NOTICE, loglevel_t::WARN };
+static_assert(DLOG_NUM == 2, "number of log streams has changed");
+
 bool console_service_status = true;  // show service status messages to console?
 
+
 dasynq::time_val release_time; // time the log was released
 
 using rearm = dasynq::rearm;
@@ -122,7 +119,7 @@ class buffered_log_stream : public eventloop_t::fd_watcher_impl<buffered_log_str
 
 // Two log streams:
 // (One for main log, one for console)
-buffered_log_stream log_stream[2];
+buffered_log_stream log_stream[DLOG_NUM];
 
 void buffered_log_stream::release_console()
 {
@@ -470,6 +467,7 @@ static void do_log_part(int idx, const char *arg) noexcept
             append(log_stream[idx], arg);
         }
         else {
+            // we have to discard the message
             log_stream[idx].rollback_msg();
             log_current_line[idx] = false;
             log_stream[idx].mark_discarded();
@@ -500,7 +498,7 @@ void log_msg_begin(loglevel_t lvl, const char *msg) noexcept
         }
     }
 
-    for (int i = 0; i < 2; i++) {
+    for (int i = 0; i < DLOG_NUM; i++) {
         do_log_part(i, "dinit: ");
         do_log_part(i, msg);
     }
@@ -516,7 +514,7 @@ void log_msg_part(const char *msg) noexcept
 // Complete a multi-part log message
 void log_msg_end(const char *msg) noexcept
 {
-    for (int i = 0; i < 2; i++) {
+    for (int i = 0; i < DLOG_NUM; i++) {
         do_log_part(i, msg);
         do_log_part(i, "\n");
         do_log_commit(i);

src/includes/dinit-log.h

@@ -10,6 +10,29 @@
 // It takes a list of items comprising a single log message, including strings (C/C++ style), and integers.
 // The loglevel argument determines if the message will actually be logged (according to the configured log
 // level of the log mechanisms).
+//
+// We have two separate log "streams": one for the console/stdout, one for the syslog facility (or log
+// file). Both have a circular buffer. Log messages are appended to the circular buffer (for a syslog
+// stream, the messages are prepended with a syslog priority indicator). Both streams start out inactive
+// (release = true in buffered_log_stream), which means they will buffer messages but not write them.
+//
+// Service start/stop messages for the console stream are formatted differently, with a "visual" flavour.
+// The console stream is treated as informational and in some circumstances messages will be discarded
+// from its buffer with no warning.
+//
+// If a stream buffer becomes full mid-message, the message is discarded and we mark the stream as "message
+// discarded". Once the message at the front of the buffer has been fully output we check for the mark and
+// if set, issue a message informing that log messages have been discarded, before resuming regular output
+// from the buffer. (Because the buffer is full, we can't store the "message discarded" message in it; we
+// temporarily switch to a "special buffer" which just contains the "message discarded" text. Current the
+// special buffer mechanism is used only for this purpose).
+//
+// The console log stream needs to be able to release the console, if a service is waiting to acquire it.
+// This is accomplished by calling flush_for_release() which then completes the output of the current
+// message (if any) and then assigns the console to a waiting service. Once the console is no longer used
+// by any service, the service_set will return it to the log; the time between release and re-acquire is
+// checked and, if it's too large, the entire buffer is discarded; this avoids suddenly and confusingly
+// displaying stale messages.
 
 #include <string>
 #include <cstdio>
@@ -28,6 +51,8 @@ enum class loglevel_t {
 constexpr static int DLOG_MAIN = 0; // main log facility
 constexpr static int DLOG_CONS = 1; // console
 
+constexpr static int DLOG_NUM = 2;
+
 // These are defined in dinit-log.cc:
 extern loglevel_t log_level[2];
 extern bool console_service_status;  // show service status messages to console?