From 477b01a24f842c98e8abac56e3fe2ffb3d3a56db Mon Sep 17 00:00:00 2001 From: Steven Luo Date: Wed, 26 May 2010 04:06:30 -0700 Subject: [PATCH] Clean up configuration again Commit ec8b58af... ("Refactor configuration") still has too much boilerplate code and leaves too many places that have to be modified to add a new config option. To fix this, add a new array of config options with name and type of information stored, and then add an array of pointers to the elements of struct swb_config with indices matching the list of config options to struct swb_config. This consolidates all the work needed to add new config options to config.h and config.c, and allows us to replace the boilerplate code with loops over the array of config options. --- config-ui/browser-switchboard-cp.c | 4 +- config-ui/save-config.c | 96 +++++++++++++----------------- config.c | 115 +++++++++++++++++++++++------------- config.h | 14 +++++ 4 files changed, 131 insertions(+), 98 deletions(-) diff --git a/config-ui/browser-switchboard-cp.c b/config-ui/browser-switchboard-cp.c index 6866c84..f0f4e40 100644 --- a/config-ui/browser-switchboard-cp.c +++ b/config-ui/browser-switchboard-cp.c @@ -163,7 +163,9 @@ static void load_config(void) { } static void save_config(void) { - struct swb_config new_cfg = orig_cfg; + struct swb_config new_cfg; + + swb_config_copy(&new_cfg, &orig_cfg); if (get_continuous_mode() != orig_cfg.continuous_mode) { new_cfg.continuous_mode = get_continuous_mode(); diff --git a/config-ui/save-config.c b/config-ui/save-config.c index 6b010e5..4a066f5 100644 --- a/config-ui/save-config.c +++ b/config-ui/save-config.c @@ -30,6 +30,40 @@ #include "configfile.h" #include "config.h" +extern struct swb_config_option swb_config_options[]; + +/* Outputs a config file line for the named option to a file descriptor */ +static void swb_config_output_option(FILE *fp, unsigned int *oldcfg_seen, + struct swb_config *cfg, char *name) { + int i; + struct swb_config_option opt; + + for (i = 0; swb_config_options[i].name; ++i) { + opt = swb_config_options[i]; + if (strcmp(opt.name, name)) + continue; + + if (!(*oldcfg_seen & opt.set_mask) && + (cfg->flags & opt.set_mask)) { + switch (opt.type) { + case SWB_CONFIG_OPT_STRING: + fprintf(fp, "%s = \"%s\"\n", + opt.name, + *(char **)cfg->entries[i]); + *oldcfg_seen |= opt.set_mask; + break; + case SWB_CONFIG_OPT_INT: + fprintf(fp, "%s = %d\n", + opt.name, + *(int *)cfg->entries[i]); + *oldcfg_seen |= opt.set_mask; + break; + } + } + break; + } +} + /* Save the settings in the provided swb_config struct to the config file Returns true on success, false otherwise */ int swb_config_save(struct swb_config *cfg) { @@ -39,6 +73,7 @@ int swb_config_save(struct swb_config *cfg) { int retval = 1; struct swb_config_line line; unsigned int oldcfg_seen = 0; + int i; /* If CONFIGFILE_DIR doesn't exist already, try to create it */ if (!(homedir = getenv("HOME"))) @@ -78,46 +113,8 @@ int swb_config_save(struct swb_config *cfg) { while (!parse_config_file_line(fp, &line)) { if (line.parsed) { /* Is a config line, print the new value here */ - if (!strcmp(line.key, "continuous_mode")) { - if (!(oldcfg_seen & - SWB_CONFIG_CONTINUOUS_MODE_SET)) { - fprintf(tmpfp, "%s = %d\n", - line.key, - cfg->continuous_mode); - oldcfg_seen |= - SWB_CONFIG_CONTINUOUS_MODE_SET; - } - } else if (!strcmp(line.key, - "default_browser")) { - if (!(oldcfg_seen & - SWB_CONFIG_DEFAULT_BROWSER_SET)) { - fprintf(tmpfp, "%s = \"%s\"\n", - line.key, - cfg->default_browser); - oldcfg_seen |= - SWB_CONFIG_DEFAULT_BROWSER_SET; - } - } else if (!strcmp(line.key, - "other_browser_cmd")) { - if (!(oldcfg_seen & - SWB_CONFIG_OTHER_BROWSER_CMD_SET)) { - fprintf(tmpfp, "%s = \"%s\"\n", - line.key, - cfg->other_browser_cmd); - oldcfg_seen |= - SWB_CONFIG_OTHER_BROWSER_CMD_SET; - } - } else if (!strcmp(line.key, - "logging")) { - if (!(oldcfg_seen & - SWB_CONFIG_LOGGING_SET)) { - fprintf(tmpfp, "%s = \"%s\"\n", - line.key, - cfg->logging); - oldcfg_seen |= - SWB_CONFIG_LOGGING_SET; - } - } + swb_config_output_option(tmpfp, &oldcfg_seen, + cfg, line.key); } else { /* Just copy the old line over */ fprintf(tmpfp, "%s\n", line.key); @@ -129,22 +126,9 @@ int swb_config_save(struct swb_config *cfg) { } /* If we haven't written them yet, write out any new config values */ - if (!(oldcfg_seen & SWB_CONFIG_CONTINUOUS_MODE_SET) && - (cfg->flags & SWB_CONFIG_CONTINUOUS_MODE_SET)) - fprintf(tmpfp, "%s = %d\n", - "continuous_mode", cfg->continuous_mode); - if (!(oldcfg_seen & SWB_CONFIG_DEFAULT_BROWSER_SET) && - (cfg->flags & SWB_CONFIG_DEFAULT_BROWSER_SET)) - fprintf(tmpfp, "%s = \"%s\"\n", - "default_browser", cfg->default_browser); - if (!(oldcfg_seen & SWB_CONFIG_OTHER_BROWSER_CMD_SET) && - (cfg->flags & SWB_CONFIG_OTHER_BROWSER_CMD_SET)) - fprintf(tmpfp, "%s = \"%s\"\n", - "other_browser_cmd", cfg->other_browser_cmd); - if (!(oldcfg_seen & SWB_CONFIG_LOGGING_SET) && - (cfg->flags & SWB_CONFIG_LOGGING_SET)) - fprintf(tmpfp, "%s = \"%s\"\n", - "logging", cfg->logging); + for (i = 0; swb_config_options[i].name; ++i) + swb_config_output_option(tmpfp, &oldcfg_seen, cfg, + swb_config_options[i].name); /* Replace the old config file with the new one */ fclose(tmpfp); diff --git a/config.c b/config.c index 663858a..727287f 100644 --- a/config.c +++ b/config.c @@ -26,17 +26,47 @@ #include "configfile.h" #include "config.h" -/* Initialize a swb_config struct with configuration defaults */ -void swb_config_init(struct swb_config *cfg) { - if (!cfg) +/* The Browser Switchboard config file options */ +struct swb_config_option swb_config_options[] = { + { "continuous_mode", SWB_CONFIG_OPT_INT, SWB_CONFIG_CONTINUOUS_MODE_SET }, + { "default_browser", SWB_CONFIG_OPT_STRING, SWB_CONFIG_DEFAULT_BROWSER_SET }, + { "other_browser_cmd", SWB_CONFIG_OPT_STRING, SWB_CONFIG_OTHER_BROWSER_CMD_SET }, + { "logging", SWB_CONFIG_OPT_STRING, SWB_CONFIG_LOGGING_SET }, + { NULL, 0, 0 }, +}; + +/* Browser Switchboard configuration defaults */ +static struct swb_config swb_config_defaults = { + .flags = SWB_CONFIG_INITIALIZED, + .continuous_mode = 0, + .default_browser = "microb", + .other_browser_cmd = NULL, + .logging = "stdout", +}; + + +/* Copy the contents of an swb_config struct + The entries[] array means that the standard copy will not work */ +void swb_config_copy(struct swb_config *dst, struct swb_config *src) { + if (!dst || !src) return; - cfg->continuous_mode = 0; - cfg->default_browser = "microb"; - cfg->other_browser_cmd = NULL; - cfg->logging = "stdout"; + dst->entries[0] = &(dst->continuous_mode); + dst->entries[1] = &(dst->default_browser); + dst->entries[2] = &(dst->other_browser_cmd); + dst->entries[3] = &(dst->logging); - cfg->flags = SWB_CONFIG_INITIALIZED; + dst->flags = src->flags; + + dst->continuous_mode = src->continuous_mode; + dst->default_browser = src->default_browser; + dst->other_browser_cmd = src->other_browser_cmd; + dst->logging = src->logging; +} + +/* Initialize a swb_config struct with configuration defaults */ +void swb_config_init(struct swb_config *cfg) { + swb_config_copy(cfg, &swb_config_defaults); } /* Free all heap memory used in an swb_config struct @@ -63,6 +93,40 @@ void swb_config_free(struct swb_config *cfg) { cfg->flags = 0; } +/* Load a value into the part of a struct swb_config indicated by name */ +static int swb_config_load_option(struct swb_config *cfg, + char *name, char *value) { + int i; + struct swb_config_option opt; + int retval = 0; + + for (i = 0; swb_config_options[i].name; ++i) { + opt = swb_config_options[i]; + if (strcmp(name, opt.name)) + continue; + + if (!(cfg->flags & opt.set_mask)) { + switch (opt.type) { + case SWB_CONFIG_OPT_STRING: + *(char **)cfg->entries[i] = value; + break; + case SWB_CONFIG_OPT_INT: + *(int *)cfg->entries[i] = atoi(value); + free(value); + break; + } + cfg->flags |= opt.set_mask; + } + retval = 1; + break; + } + + if (!retval) + free(value); + + return retval; +} + /* Read the config file and load settings into the provided swb_config struct Caller is responsible for freeing allocated strings with free() Returns true on success, false otherwise */ @@ -81,39 +145,8 @@ int swb_config_load(struct swb_config *cfg) { if (!parse_config_file_begin()) goto out; while (!parse_config_file_line(fp, &line)) { - if (line.parsed) { - if (!strcmp(line.key, "continuous_mode")) { - if (!(cfg->flags & - SWB_CONFIG_CONTINUOUS_MODE_SET)) { - cfg->continuous_mode = atoi(line.value); - cfg->flags |= - SWB_CONFIG_CONTINUOUS_MODE_SET; - } - free(line.value); - } else if (!strcmp(line.key, "default_browser")) { - if (!(cfg->flags & - SWB_CONFIG_DEFAULT_BROWSER_SET)) { - cfg->default_browser = line.value; - cfg->flags |= - SWB_CONFIG_DEFAULT_BROWSER_SET; - } - } else if (!strcmp(line.key, "other_browser_cmd")) { - if (!(cfg->flags & - SWB_CONFIG_OTHER_BROWSER_CMD_SET)) { - cfg->other_browser_cmd = line.value; - cfg->flags |= - SWB_CONFIG_OTHER_BROWSER_CMD_SET; - } - } else if (!strcmp(line.key, "logging")) { - if (!(cfg->flags & SWB_CONFIG_LOGGING_SET)) { - cfg->logging = line.value; - cfg->flags |= SWB_CONFIG_LOGGING_SET; - } - } else { - /* Don't need this line's contents */ - free(line.value); - } - } + if (line.parsed) + swb_config_load_option(cfg, line.key, line.value); free(line.key); } parse_config_file_end(); diff --git a/config.h b/config.h index 39df717..858aec5 100644 --- a/config.h +++ b/config.h @@ -31,12 +31,26 @@ struct swb_config { unsigned int flags; + /* Array of pointers to the elements of the struct, in the order given + in swb_config_options[] */ + void *entries[4]; + int continuous_mode; char *default_browser; char *other_browser_cmd; char *logging; }; +struct swb_config_option { + char *name; + enum { + SWB_CONFIG_OPT_STRING, + SWB_CONFIG_OPT_INT + } type; + int set_mask; +}; + +void swb_config_copy(struct swb_config *dst, struct swb_config *src); void swb_config_init(struct swb_config *cfg); void swb_config_free(struct swb_config *cfg); -- 1.7.9.5