From fd200ff52e139d2180003a8711cbde56dd7e8126 Mon Sep 17 00:00:00 2001 From: Steven Luo Date: Sun, 28 Nov 2010 06:14:17 -0800 Subject: [PATCH 1/1] Use computed offset instead of hand-built array for access to struct swb_config members To access an arbitrary member of struct swb_config, we currently use the entries array in the struct, which contains pointers to the members of the struct in order. This has several disadvantages: * the entries array must be filled by hand for each instance; * structure instances cannot be copied in the normal way; * the swb_config_options array describing the possible config options must be kept in the same order as the members of the struct. A much better solution is to let the compiler compute the offset of structure members using the offsetof() macro and stick the results in the swb_config_options array; we can then access the member by adding the offset to the address of the structure instance. This also allows us to get rid of the entries array in struct swb_config and the swb_config_copy() function. --- config-ui/browser-switchboard-config.c | 25 ++++++++-------- config-ui/browser-switchboard-cp.c | 2 +- config-ui/save-config.c | 9 +++--- config.c | 51 ++++++++++---------------------- config.h | 7 ++--- 5 files changed, 35 insertions(+), 59 deletions(-) diff --git a/config-ui/browser-switchboard-config.c b/config-ui/browser-switchboard-config.c index c28b743..685a0b0 100644 --- a/config-ui/browser-switchboard-config.c +++ b/config-ui/browser-switchboard-config.c @@ -22,7 +22,6 @@ #include -#include #include #include #include @@ -37,7 +36,7 @@ extern struct swb_config_option swb_config_options[]; static int get_config_value(char *name) { struct swb_config cfg; struct swb_config_option *optinfo; - ptrdiff_t i; + void *entry; int retval = 1; swb_config_init(&cfg); @@ -49,14 +48,14 @@ static int get_config_value(char *name) { if (strcmp(name, optinfo->name)) continue; - i = optinfo - swb_config_options; + entry = (char *)&cfg + optinfo->offset; switch (optinfo->type) { case SWB_CONFIG_OPT_STRING: - if (*(char **)cfg.entries[i]) - printf("%s\n", *(char **)cfg.entries[i]); + if (*(char **)entry) + printf("%s\n", *(char **)entry); break; case SWB_CONFIG_OPT_INT: - printf("%d\n", *(int *)cfg.entries[i]); + printf("%d\n", *(int *)entry); break; default: break; @@ -106,7 +105,7 @@ static int get_default_browser(void) { static int set_config_value(char *name, char *value) { struct swb_config orig_cfg, cfg; struct swb_config_option *optinfo; - ptrdiff_t i; + void *entry; int retval = 1; swb_config_init(&orig_cfg); @@ -114,24 +113,24 @@ static int set_config_value(char *name, char *value) { if (!swb_config_load(&orig_cfg)) return 1; - swb_config_copy(&cfg, &orig_cfg); + cfg = orig_cfg; for (optinfo = swb_config_options; optinfo->name; ++optinfo) { if (strcmp(name, optinfo->name)) continue; - i = optinfo - swb_config_options; + entry = (char *)&cfg + optinfo->offset; switch (optinfo->type) { case SWB_CONFIG_OPT_STRING: if (strlen(value) == 0) { /* If the new value is empty, clear the config setting */ - *(char **)cfg.entries[i] = NULL; + *(char **)entry = NULL; cfg.flags &= ~optinfo->set_mask; } else { /* Make a copy of the string -- it's not safe to free value, which comes from argv */ - if (!(*(char **)cfg.entries[i] = + if (!(*(char **)entry = strdup(value))) exit(1); cfg.flags |= optinfo->set_mask; @@ -143,7 +142,7 @@ static int set_config_value(char *name, char *value) { setting */ cfg.flags &= ~optinfo->set_mask; } else { - *(int *)cfg.entries[i] = atoi(value); + *(int *)entry = atoi(value); cfg.flags |= optinfo->set_mask; } break; @@ -164,7 +163,7 @@ static int set_config_value(char *name, char *value) { freed above swb_config_free(&cfg); */ if (optinfo->name && optinfo->type == SWB_CONFIG_OPT_STRING) - free(*(char **)cfg.entries[i]); + free(*(char **)entry); return retval; } diff --git a/config-ui/browser-switchboard-cp.c b/config-ui/browser-switchboard-cp.c index 02a4a07..be9b5b1 100644 --- a/config-ui/browser-switchboard-cp.c +++ b/config-ui/browser-switchboard-cp.c @@ -163,7 +163,7 @@ static void load_config(void) { static void save_config(void) { struct swb_config new_cfg; - swb_config_copy(&new_cfg, &orig_cfg); + new_cfg = orig_cfg; #ifndef FREMANTLE if (get_continuous_mode() != orig_cfg.continuous_mode) { diff --git a/config-ui/save-config.c b/config-ui/save-config.c index 1787f0e..6cb7d5c 100644 --- a/config-ui/save-config.c +++ b/config-ui/save-config.c @@ -21,7 +21,6 @@ */ #include -#include #include #include #include @@ -41,26 +40,26 @@ extern struct swb_config_option swb_config_options[]; static void swb_config_output_option(FILE *fp, unsigned int *oldcfg_seen, struct swb_config *cfg, char *name) { struct swb_config_option *opt; - ptrdiff_t i; + void *entry; for (opt = swb_config_options; opt->name; ++opt) { if (strcmp(opt->name, name)) continue; - i = opt - swb_config_options; + entry = (char *)cfg + opt->offset; 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]); + *(char **)entry); *oldcfg_seen |= opt->set_mask; break; case SWB_CONFIG_OPT_INT: fprintf(fp, "%s = %d\n", opt->name, - *(int *)cfg->entries[i]); + *(int *)entry); *oldcfg_seen |= opt->set_mask; break; } diff --git a/config.c b/config.c index 2dcbfae..fad8271 100644 --- a/config.c +++ b/config.c @@ -29,12 +29,12 @@ /* 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 }, - { "autostart_microb", SWB_CONFIG_OPT_INT, SWB_CONFIG_AUTOSTART_MICROB_SET }, - { NULL, 0, 0 }, + { "continuous_mode", SWB_CONFIG_OPT_INT, SWB_CONFIG_CONTINUOUS_MODE_SET, offsetof(struct swb_config, continuous_mode) }, + { "default_browser", SWB_CONFIG_OPT_STRING, SWB_CONFIG_DEFAULT_BROWSER_SET, offsetof(struct swb_config, default_browser) }, + { "other_browser_cmd", SWB_CONFIG_OPT_STRING, SWB_CONFIG_OTHER_BROWSER_CMD_SET, offsetof(struct swb_config, other_browser_cmd) }, + { "logging", SWB_CONFIG_OPT_STRING, SWB_CONFIG_LOGGING_SET, offsetof(struct swb_config, logging) }, + { "autostart_microb", SWB_CONFIG_OPT_INT, SWB_CONFIG_AUTOSTART_MICROB_SET, offsetof(struct swb_config, autostart_microb) }, + { NULL, 0, 0, 0 }, }; /* Browser Switchboard configuration defaults */ @@ -48,36 +48,16 @@ static struct swb_config swb_config_defaults = { }; -/* 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; - - dst->entries[0] = &(dst->continuous_mode); - dst->entries[1] = &(dst->default_browser); - dst->entries[2] = &(dst->other_browser_cmd); - dst->entries[3] = &(dst->logging); - dst->entries[4] = &(dst->autostart_microb); - - 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; - dst->autostart_microb = src->autostart_microb; -} - /* Initialize a swb_config struct with configuration defaults */ -void swb_config_init(struct swb_config *cfg) { - swb_config_copy(cfg, &swb_config_defaults); +inline void swb_config_init(struct swb_config *cfg) { + *cfg = swb_config_defaults; } /* Free all heap memory used in an swb_config struct This MUST NOT be done if any of the strings are being used elsewhere! */ void swb_config_free(struct swb_config *cfg) { int i; + void *entry; if (!cfg) return; @@ -85,11 +65,12 @@ void swb_config_free(struct swb_config *cfg) { return; for (i = 0; swb_config_options[i].name; ++i) { + entry = (char *)cfg + swb_config_options[i].offset; if (cfg->flags & swb_config_options[i].set_mask) { switch (swb_config_options[i].type) { case SWB_CONFIG_OPT_STRING: - free(*(char **)cfg->entries[i]); - *(char **)cfg->entries[i] = NULL; + free(*(char **)entry); + *(char **)entry = NULL; break; default: break; @@ -104,7 +85,7 @@ void swb_config_free(struct swb_config *cfg) { static int swb_config_load_option(struct swb_config *cfg, char *name, char *value) { struct swb_config_option *opt; - ptrdiff_t i; + void *entry; /* Search through list of recognized config options for a match */ for (opt = swb_config_options; opt->name; ++opt) { @@ -112,13 +93,13 @@ static int swb_config_load_option(struct swb_config *cfg, continue; if (!(cfg->flags & opt->set_mask)) { - i = opt - swb_config_options; + entry = (char *)cfg + opt->offset; switch (opt->type) { case SWB_CONFIG_OPT_STRING: - *(char **)cfg->entries[i] = value; + *(char **)entry = value; break; case SWB_CONFIG_OPT_INT: - *(int *)cfg->entries[i] = atoi(value); + *(int *)entry = atoi(value); free(value); break; } diff --git a/config.h b/config.h index 31149f6..11ef5c5 100644 --- a/config.h +++ b/config.h @@ -32,9 +32,6 @@ 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[5]; int continuous_mode; char *default_browser; @@ -50,10 +47,10 @@ struct swb_config_option { SWB_CONFIG_OPT_INT } type; int set_mask; + size_t offset; }; -void swb_config_copy(struct swb_config *dst, struct swb_config *src); -void swb_config_init(struct swb_config *cfg); +inline void swb_config_init(struct swb_config *cfg); void swb_config_free(struct swb_config *cfg); int swb_config_load(struct swb_config *cfg); -- 1.7.9.5