Fix configuration loading logic

If configuration is bad, do not execute commands except those with the
PERMISSIVE_CONFIG property.

Flush log buffer immediately after clearing cf_limbo flag, in case there are no
further log messages that would cause the flush.  (Fixes bug that an unrecognised
command produced no log output.)

More block comments in log.c.

commandline.c

@@ -38,6 +38,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #include "conf.h"
 #include "rhizome.h"
 #include "strbuf.h"
+#include "strbuf_helpers.h"
 #include "str.h"
 #include "mdp_client.h"
 #include "cli.h"
@@ -194,13 +195,17 @@ int parseCommandLine(const char *argv0, int argc, const char *const *args)
   int result = cli_parse(argc, args, command_line_options);
   if (result != -1) {
     const struct command_line_option *option = &command_line_options[result];
-    if (option->flags & CLIFLAG_PERMISSIVE_CONFIG)
-      cf_reload_permissive();
-    else
-      cf_reload();
+    // Do not run the command if the configuration does not load ok
+    if (((option->flags & CLIFLAG_PERMISSIVE_CONFIG) ? cf_reload_permissive() : cf_reload()) != -1)
       result = cli_invoke(option, argc, args, NULL);
+    else {
+      strbuf b = strbuf_alloca(160);
+      strbuf_append_argv(b, argc, args);
+      result = WHYF("configuration unavailable, not running command: %s", strbuf_str(b));
+    }
   } else {
-    cf_reload();
+    // Load configuration so that "unsupported command" log message can get out
+    cf_reload_permissive();
   }
 
   /* clean up after ourselves */

conf.c

@@ -99,7 +99,7 @@ static int load()
       free(buf);
       return WHYF_perror("fclose(%s)", path);
     }
-    INFOF("config file %s successfully read", path);
+    INFOF("config file %s successfully read %ld bytes", path, (long) meta.size);
   }
   struct cf_om_node *new_root = NULL;
   int result = cf_om_parse(path, buf, meta.size, &new_root);
@@ -137,12 +137,17 @@ int cf_om_load()
  */
 int cf_om_reload()
 {
-  if (!has_changed(&conffile_meta))
+  switch (has_changed(&conffile_meta)) {
+  case -1:
+    return CFERROR;
+  case 0:
     return CFOK;
+  default:
     if (conffile_meta.mtime != -1)
       INFOF("config file %s -- detected new version", conffile_path());
     return cf_om_load();
   }
+}
 
 int cf_om_save()
 {
@@ -201,11 +206,12 @@ static int load_and_parse(int permissive)
 	  config = new_config;
 	  config_meta = conffile_meta;
 	  cf_limbo = 0;
+	  logFlush();
 	} else if (result != CFERROR) {
 	  result &= ~CFEMPTY;
 	  config = new_config;
-	  cf_limbo = 0;
 	  WARN("limping along with incomplete configuration");
+	  cf_limbo = 0;
 	}
       }
     }

log.c

@@ -173,6 +173,18 @@ static int _log_prepare(int level, struct __sourceloc whence)
   return 1;
 }
 
+/* Internal logging implementation.
+ *
+ * This function is called after every single log message is appended to logbuf, and is given the
+ * level of the message.  This function must reset the given strbuf after its contents have been
+ * sent to the log, otherwise log messages will be repeated.  If this function never resets the
+ * strbuf, then it may eventually overrun.
+ *
+ * This function is also called to flush the given logbuf, by giving a log level of SILENT.  That
+ * indicates that no new message has been appended since the last time this function was called.
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
 static void _log_internal(int level, struct strbuf *buf)
 {
 #ifdef ANDROID
@@ -183,13 +195,15 @@ static void _log_internal(int level, struct strbuf *buf)
     case LOG_LEVEL_INFO:  alevel = ANDROID_LOG_INFO; break;
     case LOG_LEVEL_WARN:  alevel = ANDROID_LOG_WARN; break;
     case LOG_LEVEL_DEBUG: alevel = ANDROID_LOG_DEBUG; break;
+    case LOG_LEVEL_SILENT: return;
+    default: abort();
   }
   __android_log_print(alevel, "servald", "%s", strbuf_str(buf));
   strbuf_reset(buf);
 #else
   FILE *logf = _open_logging();
   if (logf) {
-    fprintf(logf, "%s\n%s", strbuf_str(buf), strbuf_overrun(buf) ? "LOG OVERRUN\n" : "");
+    fprintf(logf, "%s%s%s", strbuf_str(buf), strbuf_len(buf) ? "\n" : "", strbuf_overrun(buf) ? "LOG OVERRUN\n" : "");
     strbuf_reset(buf);
   }
 #endif
@@ -208,6 +222,12 @@ void set_log_implementation(void (*log_function)(int level, struct strbuf *buf))
   _log_implementation=log_function;
 }
 
+void logFlush()
+{
+  if (_log_implementation)
+    _log_implementation(LOG_LEVEL_SILENT, &logbuf);
+}
+
 void logArgv(int level, struct __sourceloc whence, const char *label, int argc, const char *const *argv)
 {
   if (_log_prepare(level, whence)) {

log.h

@@ -87,6 +87,7 @@ extern const struct __sourceloc __whence; // see above
 void set_logging(FILE *f);
 FILE *open_logging();
 void close_logging();
+void logFlush();
 void logArgv(int level, struct __sourceloc whence, const char *label, int argc, const char *const *argv);
 void logString(int level, struct __sourceloc whence, const char *str);
 void logMessage(int level, struct __sourceloc whence, const char *fmt, ...);