From fe3e7da5c614344c76a38029399b49a234d35e5e Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Fri, 14 Dec 2012 11:39:08 +1030 Subject: [PATCH] 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 | 17 +++++++++++------ conf.c | 18 ++++++++++++------ log.c | 22 +++++++++++++++++++++- log.h | 1 + 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/commandline.c b/commandline.c index eaa738d4..d10fc980 100644 --- a/commandline.c +++ b/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(); - result = cli_invoke(option, argc, args, NULL); + // 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 */ diff --git a/conf.c b/conf.c index b4ea9abc..363df449 100644 --- a/conf.c +++ b/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,11 +137,16 @@ 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; - if (conffile_meta.mtime != -1) - INFOF("config file %s -- detected new version", conffile_path()); - return cf_om_load(); + 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; } } } diff --git a/log.c b/log.c index 82cacb69..301e238e 100644 --- a/log.c +++ b/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 + */ 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)) { diff --git a/log.h b/log.h index 6c3dcf08..be0656e6 100644 --- a/log.h +++ b/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, ...);