[Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a command line argument.
This patch provides a way to use the entire table of configuration options as "long" command line switches. Signed-off-by: Richard Cochran --- config.c | 64 ++-- config.h | 11 +++ ptp4l.c | 11 +-- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/config.c b/config.c index 5da5ecc..384b437 100644 --- a/config.c +++ b/config.c @@ -329,6 +329,7 @@ static enum parser_result parse_section_line(char *s, enum config_section *secti } static enum parser_result parse_item(struct config *cfg, +int commandline, const char *section, const char *option, const char *value) @@ -387,7 +388,7 @@ static enum parser_result parse_item(struct config *cfg, return NOT_PARSED; } } - } else if (cgi->flags & CFG_ITEM_LOCKED) { + } else if (!commandline && cgi->flags & CFG_ITEM_LOCKED) { /* This global option was set on the command line. */ return PARSED_OK; } else { @@ -415,6 +416,10 @@ static enum parser_result parse_item(struct config *cfg, dst->flags |= CFG_ITEM_DYNSTR; break; } + + if (commandline) { + dst->flags &= CFG_ITEM_LOCKED; + } return PARSED_OK; } @@ -490,6 +495,25 @@ static void check_deprecated_options(const char **option) } } +static struct option *config_alloc_longopts(struct config *cfg) +{ + struct config_item *ci; + struct option *opts; + int i; + + opts = calloc(1, (1 + N_CONFIG_ITEMS) * sizeof(*opts)); + if (!opts) { + return NULL; + } + for (i = 0; i < N_CONFIG_ITEMS; i++) { + ci = &config_tab[i]; + opts[i].name = ci->label; + opts[i].has_arg = required_argument; + } + + return opts; +} + int config_read(char *name, struct config *cfg) { enum config_section current_section = UNKNOWN_SECTION; @@ -554,7 +578,7 @@ int config_read(char *name, struct config *cfg) check_deprecated_options(&option); - parser_res = parse_item(cfg, current_section == GLOBAL_SECTION ? + parser_res = parse_item(cfg, 0, current_section == GLOBAL_SECTION ? NULL : current_port->name, option, value); switch (parser_res) { @@ -627,8 +651,15 @@ struct config *config_create(void) } STAILQ_INIT(&cfg->interfaces); + cfg->opts = config_alloc_longopts(cfg); + if (!cfg->opts) { + free(cfg); + return NULL; + } + cfg->htab = hash_create(); if (!cfg->htab) { + free(cfg->opts); free(cfg); return NULL; } @@ -657,6 +688,7 @@ struct config *config_create(void) return cfg; fail: hash_destroy(cfg->htab, NULL); + free(cfg->opts); free(cfg); return NULL; } @@ -670,6 +702,7 @@ void config_destroy(struct config *cfg) free(iface); } hash_destroy(cfg->htab, config_item_free); + free(cfg->opts); free(cfg); } @@ -720,6 +753,33 @@ char *config_get_string(struct config *cfg, const char *section, return ci->val.s; } +int config_parse_option(struct config *cfg, const char *opt, const char *val) +{ + enum parser_result result; + + result = parse_item(cfg, 1, NULL, opt, val); + + switch (result) { + case PARSED_OK: + return 0; + case NOT_PARSED: + fprintf(stderr, "unknown option %s\n", opt); + break; + case BAD_VALUE: + fprintf(stderr, "%s is a bad value for option %s\n", val, opt); + break; + case MALFORMED: + fprintf(stderr, "%s is a malformed value for option %s\n", + val, opt); + break; + case OUT_OF_RANGE: + fprintf(stderr, "%s is an out of range value for option %s\n", + val, opt); + break; + } + return -1; +} + int config_set_double(struct config *cfg, const char *option, double val) { struct config_item *ci = config_find_item(cfg, NULL, option); diff --git a/config.h b/config.h index b02bde6..1cc7051 100644 --- a/config.h +++ b/config.h @@ -20,6 +20,7 @@ #ifndef HAVE_CONFIG_H #define HAVE_CONFIG_H +#include #include #include "ds.h" @@ -43,6 +44,9 @@ struct config { STAILQ_HEAD(interfaces_head, interface) interfaces; int n_interfaces; + /* for parsing command line options */ + struct option *opts; + /* hash of all non-legacy items */ struct hash *htab; }; @@ -64,6 +68
Re: [Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a command line argument.
Hi Richard, This patch looks great. My comments below are just my process for understanding what the code does. I really like this approach, it's a pretty small amount of code and lets us easily override any option from the command line, without having to continuously add more short options for esoteric configurations. Regards, Jake > -Original Message- > From: Richard Cochran [mailto:richardcoch...@gmail.com] > Sent: Tuesday, December 06, 2016 10:50 AM > To: linuxptp-devel@lists.sourceforge.net > Subject: [Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a > command line argument. > > This patch provides a way to use the entire table of configuration options > as "long" command line switches. > Neat, I didn't think it would be so little code! Thanks! > Signed-off-by: Richard Cochran > --- > config.c | 64 > ++ > -- > config.h | 11 +++ > ptp4l.c | 11 +-- > 3 files changed, 82 insertions(+), 4 deletions(-) > > diff --git a/config.c b/config.c > index 5da5ecc..384b437 100644 > --- a/config.c > +++ b/config.c > @@ -329,6 +329,7 @@ static enum parser_result parse_section_line(char *s, > enum config_section *secti > } > > static enum parser_result parse_item(struct config *cfg, > + int commandline, Ok so we add a boolean to set whether we're calling parse_item from command line code. >const char *section, >const char *option, >const char *value) > @@ -387,7 +388,7 @@ static enum parser_result parse_item(struct config *cfg, > return NOT_PARSED; > } > } > - } else if (cgi->flags & CFG_ITEM_LOCKED) { > + } else if (!commandline && cgi->flags & CFG_ITEM_LOCKED) { And here, we do allow over-writing a previous assignment with a later assignment from the command line. Good. > /* This global option was set on the command line. */ > return PARSED_OK; > } else { > @@ -415,6 +416,10 @@ static enum parser_result parse_item(struct config *cfg, > dst->flags |= CFG_ITEM_DYNSTR; > break; > } > + > + if (commandline) { > + dst->flags &= CFG_ITEM_LOCKED; > + } Finally, if we run from the command line, we make sure we lock the item now so that it doesn't change again. This makes sense. > return PARSED_OK; > } > > @@ -490,6 +495,25 @@ static void check_deprecated_options(const char > **option) > } > } > > +static struct option *config_alloc_longopts(struct config *cfg) > +{ > + struct config_item *ci; > + struct option *opts; > + int i; > + > + opts = calloc(1, (1 + N_CONFIG_ITEMS) * sizeof(*opts)); > + if (!opts) { > + return NULL; > + } > + for (i = 0; i < N_CONFIG_ITEMS; i++) { > + ci = &config_tab[i]; > + opts[i].name = ci->label; > + opts[i].has_arg = required_argument; > + } > + > + return opts; > +} > + A new function here to generate long option structures from the configuration data. Ok. > int config_read(char *name, struct config *cfg) > { > enum config_section current_section = UNKNOWN_SECTION; > @@ -554,7 +578,7 @@ int config_read(char *name, struct config *cfg) > > check_deprecated_options(&option); > > - parser_res = parse_item(cfg, current_section == > GLOBAL_SECTION ? > + parser_res = parse_item(cfg, 0, current_section == > GLOBAL_SECTION ? > NULL : current_port->name, option, > value); > > switch (parser_res) { > @@ -627,8 +651,15 @@ struct config *config_create(void) > } > STAILQ_INIT(&cfg->interfaces); > > + cfg->opts = config_alloc_longopts(cfg); > + if (!cfg->opts) { > + free(cfg); > + return NULL; > + } > + > cfg->htab = hash_create(); > if (!cfg->htab) { > + free(cfg->opts); > free(cfg); > return NULL; > } > @@ -657,6 +688,7 @@ struct config *config_create(void) > return cfg; > fail: > hash_destroy(cfg->htab, NULL); > + free(cfg->opts); > free(cfg); > return NULL; > } > @@ -670,6 +702,7 @@ void config_destroy(struct config *cfg) > free(iface); > } > hash_destroy(cfg-&g
Re: [Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a command line argument.
On Tue, Dec 06, 2016 at 11:59:40PM +, Keller, Jacob E wrote: > Neat, I didn't think it would be so little code! Thanks! Yeah, sometimes you get lucky. Thanks, Richard -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today.http://sdm.link/xeonphi ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a command line argument.
On Tue, Dec 06, 2016 at 07:49:56PM +0100, Richard Cochran wrote: > This patch provides a way to use the entire table of configuration options > as "long" command line switches. I'm testing the latest code from git which includes this feature. It doesn't seem to work correctly for me. The program accepts long options, but the behaviour doesn't change as if they were not specified. Also, valgrind reports an error about invalid free on exit. ==4584== Invalid free() / delete / delete[] / realloc() ==4584==at 0x4C2ED4A: free (vg_replace_malloc.c:530) ==4584==by 0x4077DD: config_item_free (config.c:310) ==4584==by 0x4091F3: hash_destroy (hash.c:62) ==4584==by 0x4085F9: config_destroy (config.c:704) ==4584==by 0x40228A: main (ptp4l.c:221) ==4584== Address 0x622d20 is 2240 bytes inside data symbol "config_tab" > + > + if (commandline) { > + dst->flags &= CFG_ITEM_LOCKED; Should this be dst->flags |= CFG_ITEM_LOCKED ? -- Miroslav Lichvar -- Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a command line argument.
On Tue, Dec 13, 2016 at 04:47:23PM +0100, Miroslav Lichvar wrote: > I'm testing the latest code from git which includes this feature. It > doesn't seem to work correctly for me. The program accepts long > options, but the behaviour doesn't change as if they were not > specified. Also, valgrind reports an error about invalid free on exit. I'll check it. > > + if (commandline) { > > + dst->flags &= CFG_ITEM_LOCKED; > > Should this be dst->flags |= CFG_ITEM_LOCKED ? Ouch. Yes of course. Thanks, Richard -- Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH] ptp4l: Accept any configuration option as a command line argument.
On Tue, Dec 13, 2016 at 05:34:03PM +0100, Richard Cochran wrote: > On Tue, Dec 13, 2016 at 04:47:23PM +0100, Miroslav Lichvar wrote: > > > + if (commandline) { > > > + dst->flags &= CFG_ITEM_LOCKED; > > > > Should this be dst->flags |= CFG_ITEM_LOCKED ? > > Ouch. Yes of course. Ok, with this change, valgrind reports no errors and the options seem to be working as expected. This is a nice feature! Could you please mention it in the man page? Thanks, -- Miroslav Lichvar -- Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel