From 1737d01e6d25487852d0da2e45088e11115e52a9 Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Mon, 6 Jun 2016 00:03:39 +0200 Subject: [PATCH] daemon: Let all command line options override global options from the config. This replaces 09fb81e130ad70bf0547cec3d3c349ce8cb092cf which provided a specific solution for the `PIDFile' option. The new solution is generic and more nicely abstracted without the need for global variables across multiple modules. Fixes #366 --- src/daemon/collectd.c | 6 ++--- src/daemon/collectd.h | 1 - src/daemon/configfile.c | 50 ++++++++++++++++++++++------------------- src/daemon/configfile.h | 2 +- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/daemon/collectd.c b/src/daemon/collectd.c index a72ff35f..ce4922d6 100644 --- a/src/daemon/collectd.c +++ b/src/daemon/collectd.c @@ -54,7 +54,6 @@ */ char hostname_g[DATA_MAX_NAME_LEN]; cdtime_t interval_g; -int pidfile_from_cli = 0; int timeout_g; #if HAVE_LIBKSTAT kstat_ctl_t *kc; @@ -551,15 +550,14 @@ int main (int argc, char **argv) break; case 'T': test_readall = 1; - global_option_set ("ReadThreads", "-1"); + global_option_set ("ReadThreads", "-1", 1); #if COLLECT_DAEMON daemonize = 0; #endif /* COLLECT_DAEMON */ break; #if COLLECT_DAEMON case 'P': - global_option_set ("PIDFile", optarg); - pidfile_from_cli = 1; + global_option_set ("PIDFile", optarg, 1); break; case 'f': daemonize = 0; diff --git a/src/daemon/collectd.h b/src/daemon/collectd.h index 8fb29455..500bba26 100644 --- a/src/daemon/collectd.h +++ b/src/daemon/collectd.h @@ -316,7 +316,6 @@ typedef uint64_t cdtime_t; extern char hostname_g[]; extern cdtime_t interval_g; -extern int pidfile_from_cli; extern int timeout_g; #endif /* COLLECTD_H */ diff --git a/src/daemon/configfile.c b/src/daemon/configfile.c index 34c23cdf..93e90e3b 100644 --- a/src/daemon/configfile.c +++ b/src/daemon/configfile.c @@ -80,6 +80,7 @@ typedef struct cf_global_option_s { const char *key; char *value; + _Bool from_cli; /* value set from CLI */ const char *def; } cf_global_option_t; @@ -108,21 +109,21 @@ static int cf_value_map_num = STATIC_ARRAY_SIZE (cf_value_map); static cf_global_option_t cf_global_options[] = { - {"BaseDir", NULL, PKGLOCALSTATEDIR}, - {"PIDFile", NULL, PIDFILE}, - {"Hostname", NULL, NULL}, - {"FQDNLookup", NULL, "true"}, - {"Interval", NULL, NULL}, - {"ReadThreads", NULL, "5"}, - {"WriteThreads", NULL, "5"}, - {"WriteQueueLimitHigh", NULL, NULL}, - {"WriteQueueLimitLow", NULL, NULL}, - {"Timeout", NULL, "2"}, - {"AutoLoadPlugin", NULL, "false"}, - {"CollectInternalStats", NULL, "false"}, - {"PreCacheChain", NULL, "PreCache"}, - {"PostCacheChain", NULL, "PostCache"}, - {"MaxReadInterval", NULL, "86400"} + {"BaseDir", NULL, 0, PKGLOCALSTATEDIR}, + {"PIDFile", NULL, 0, PIDFILE}, + {"Hostname", NULL, 0, NULL}, + {"FQDNLookup", NULL, 0, "true"}, + {"Interval", NULL, 0, NULL}, + {"ReadThreads", NULL, 0, "5"}, + {"WriteThreads", NULL, 0, "5"}, + {"WriteQueueLimitHigh", NULL, 0, NULL}, + {"WriteQueueLimitLow", NULL, 0, NULL}, + {"Timeout", NULL, 0, "2"}, + {"AutoLoadPlugin", NULL, 0, "false"}, + {"CollectInternalStats", NULL, 0, "false"}, + {"PreCacheChain", NULL, 0, "PreCache"}, + {"PostCacheChain", NULL, 0, "PostCache"}, + {"MaxReadInterval", NULL, 0, "86400"} }; static int cf_global_options_num = STATIC_ARRAY_SIZE (cf_global_options); @@ -210,19 +211,19 @@ static int dispatch_global_option (const oconfig_item_t *ci) if (ci->values_num != 1) return (-1); if (ci->values[0].type == OCONFIG_TYPE_STRING) - return (global_option_set (ci->key, ci->values[0].value.string)); + return (global_option_set (ci->key, ci->values[0].value.string, 0)); else if (ci->values[0].type == OCONFIG_TYPE_NUMBER) { char tmp[128]; ssnprintf (tmp, sizeof (tmp), "%lf", ci->values[0].value.number); - return (global_option_set (ci->key, tmp)); + return (global_option_set (ci->key, tmp, 0)); } else if (ci->values[0].type == OCONFIG_TYPE_BOOLEAN) { if (ci->values[0].value.boolean) - return (global_option_set (ci->key, "true")); + return (global_option_set (ci->key, "true", 0)); else - return (global_option_set (ci->key, "false")); + return (global_option_set (ci->key, "false", 0)); } return (-1); @@ -927,7 +928,7 @@ static oconfig_item_t *cf_read_generic (const char *path, /* * Public functions */ -int global_option_set (const char *option, const char *value) +int global_option_set (const char *option, const char *value, _Bool from_cli) { int i; @@ -940,10 +941,11 @@ int global_option_set (const char *option, const char *value) if (i >= cf_global_options_num) return (-1); - if (strcasecmp (option, "PIDFile") == 0 && pidfile_from_cli == 1) + if (cf_global_options[i].from_cli && (! from_cli)) { - DEBUG ("Configfile: Ignoring `PIDFILE' option because " - "command-line option `-P' take precedence."); + DEBUG ("Configfile: Ignoring %s `%s' option because " + "it was overriden by a command-line option.", + option, value); return (0); } @@ -954,6 +956,8 @@ int global_option_set (const char *option, const char *value) else cf_global_options[i].value = NULL; + cf_global_options[i].from_cli = from_cli; + return (0); } diff --git a/src/daemon/configfile.h b/src/daemon/configfile.h index 09dbe58f..924d97c5 100644 --- a/src/daemon/configfile.h +++ b/src/daemon/configfile.h @@ -89,7 +89,7 @@ int cf_register_complex (const char *type, int (*callback) (oconfig_item_t *)); */ int cf_read (const char *filename); -int global_option_set (const char *option, const char *value); +int global_option_set (const char *option, const char *value, _Bool from_cli); const char *global_option_get (const char *option); long global_option_get_long (const char *option, long default_value); long global_option_get_long_in_range (const char *option, long default_value, long min, long max); -- 2.30.2