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.

diff --git a/config-ui/browser-switchboard-config.c b/config-ui/browser-switchboard-config.c
index c28b743..685a0b0 100644 (file)
--- a/config-ui/browser-switchboard-config.c
+++ b/config-ui/browser-switchboard-config.c
@@ -22,7 +22,6 @@
 
 
 #include <stdlib.h>
 
 
 #include <stdlib.h>

-#include <stddef.h>

 #include <string.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <string.h>
 #include <stdio.h>
 #include <unistd.h>

@@ -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;
 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);
        int retval = 1;
 
        swb_config_init(&cfg);

@@ -49,14 +48,14 @@ static int get_config_value(char *name) {
                if (strcmp(name, optinfo->name))
                        continue;
 
                if (strcmp(name, optinfo->name))
                        continue;
 

-               i = optinfo - swb_config_options;
+               entry = (char *)&cfg + optinfo->offset;

                switch (optinfo->type) {
                  case SWB_CONFIG_OPT_STRING:
                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:
                        break;
                  case SWB_CONFIG_OPT_INT:

-                       printf("%d\n", *(int *)cfg.entries[i]);
+                       printf("%d\n", *(int *)entry);

                        break;
                  default:
                        break;
                        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;
 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);
        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;
 
        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;
 
 
        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 */
                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 */
                                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;
                                      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 {
                                   setting */
                                cfg.flags &= ~optinfo->set_mask;
                        } else {

-                               *(int *)cfg.entries[i] = atoi(value);
+                               *(int *)entry = atoi(value);

                                cfg.flags |= optinfo->set_mask;
                        }
                        break;
                                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)
           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;
 }
 
        return retval;
 }

diff --git a/config-ui/browser-switchboard-cp.c b/config-ui/browser-switchboard-cp.c
index 02a4a07..be9b5b1 100644 (file)
--- 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;
 
 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) {
 
 #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 (file)
--- a/config-ui/save-config.c
+++ b/config-ui/save-config.c
@@ -21,7 +21,6 @@
  */
 
 #include <stdlib.h>
  */
 
 #include <stdlib.h>

-#include <stddef.h>

 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>

@@ -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;
 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;
 
 
        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,
                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,
                                *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;
                        }
                                *oldcfg_seen |= opt->set_mask;
                                break;
                        }

diff --git a/config.c b/config.c
index 2dcbfae..fad8271 100644 (file)
--- a/config.c
+++ b/config.c
@@ -29,12 +29,12 @@
 
 /* The Browser Switchboard config file options */
 struct swb_config_option swb_config_options[] = {
 
 /* 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 */
 };
 
 /* 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 */
 /* 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;
 }
 
 /* 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;
 
        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) {
                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:
                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;
                                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;
 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) {
 
        /* 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)) {
                        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:
                        switch (opt->type) {
                          case SWB_CONFIG_OPT_STRING:

-                               *(char **)cfg->entries[i] = value;
+                               *(char **)entry = value;

                                break;
                          case SWB_CONFIG_OPT_INT:
                                break;
                          case SWB_CONFIG_OPT_INT:

-                               *(int *)cfg->entries[i] = atoi(value);
+                               *(int *)entry = atoi(value);

                                free(value);
                                break;
                        }
                                free(value);
                                break;
                        }

diff --git a/config.h b/config.h
index 31149f6..11ef5c5 100644 (file)
--- a/config.h
+++ b/config.h
@@ -32,9 +32,6 @@
 
 struct swb_config {
        unsigned int flags;
 
 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;
 
        int continuous_mode;
        char *default_browser;

@@ -50,10 +47,10 @@ struct swb_config_option {
                SWB_CONFIG_OPT_INT
        } type;
        int set_mask;
                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);
 void swb_config_free(struct swb_config *cfg);
 
 int swb_config_load(struct swb_config *cfg);