[ovs-dev] [PATCH v2 2/3] ovn-nbctl: Separate command-line options parsing and interpretation.
Mark Michelson
mmichels at redhat.com
Mon Aug 6 20:39:13 UTC 2018
On 08/03/2018 01:54 PM, Ben Pfaff wrote:
> This will allow selected options to be interpreted locally and others to
> be passed to the daemon, when the daemon is in use.
>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> lib/command-line.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++
> lib/command-line.h | 10 +++++
> ovn/utilities/ovn-nbctl.c | 39 +++++++++--------
> 3 files changed, 138 insertions(+), 19 deletions(-)
>
> diff --git a/lib/command-line.c b/lib/command-line.c
> index 81283314d1f3..235e59b25716 100644
> --- a/lib/command-line.c
> +++ b/lib/command-line.c
> @@ -52,6 +52,114 @@ ovs_cmdl_long_options_to_short_options(const struct option options[])
> return xstrdup(short_options);
> }
>
> +static char * OVS_WARN_UNUSED_RESULT
> +build_short_options(const struct option *long_options)
> +{
> + char *tmp, *short_options;
> +
> + tmp = ovs_cmdl_long_options_to_short_options(long_options);
> + short_options = xasprintf("+:%s", tmp);
> + free(tmp);
> +
> + return short_options;
> +}
> +
> +static const struct option *
> +find_option_by_value(const struct option *options, int value)
> +{
> + const struct option *o;
> +
> + for (o = options; o->name; o++) {
> + if (o->val == value) {
> + return o;
> + }
> + }
> + return NULL;
> +}
> +
> +/* Parses the command-line options in 'argc' and 'argv'. The caller specifies
> + * the supported options in 'options'. On success, stores the parsed options
> + * in '*pop', the number of options in '*n_pop', and returns NULL. On failure,
> + * returns an error message and zeros the output arguments. */
> +char * OVS_WARN_UNUSED_RESULT
> +ovs_cmdl_parse_all(int argc, char *argv[],
> + const struct option *options,
> + struct ovs_cmdl_parsed_option **pop, size_t *n_pop)
> +{
> + /* Count number of options so we can have better assertions later. */
> + size_t n_options OVS_UNUSED = 0;
> + while (options[n_options].name) {
> + n_options++;
> + }
> +
> + char *short_options = build_short_options(options);
> +
> + struct ovs_cmdl_parsed_option *po = NULL;
> + size_t allocated_po = 0;
> + size_t n_po = 0;
> +
> + char *error;
> +
> + optind = 0;
> + opterr = 0;
> + for (;;) {
> + int idx = -1;
> + int c = getopt_long(argc, argv, short_options, options, &idx);
> + switch (c) {
> + case -1:
> + *pop = po;
> + *n_pop = n_po;
> + free(short_options);
> + return NULL;
> +
> + case 0:
> + /* getopt_long() processed the option directly by setting a flag
> + * variable. This is probably undesirable for use with this
> + * function. */
> + OVS_NOT_REACHED();
> +
> + case '?':
> + if (optopt && find_option_by_value(options, optopt)) {
> + error = xasprintf("option '%s' doesn't allow an argument",
> + argv[optind - 1]);
> + } else if (optopt) {
> + error = xasprintf("unrecognized option '%c'", optopt);
> + } else {
> + error = xasprintf("unrecognized option '%s'",
> + argv[optind - 1]);
> + }
> + goto error;
> +
> + case ':':
> + error = xasprintf("option '%s' requires an argument",
> + argv[optind - 1]);
> + goto error;
> +
> + default:
> + if (allocated_po >= n_po) {
This if is backwards. It should be:
if (n_po >= allocated_po) {
> + po = x2nrealloc(po, &allocated_po, sizeof *po);
> + }
> + if (idx == -1) {
> + po[n_po].o = find_option_by_value(options, c);
> + } else {
> + ovs_assert(idx >= 0 && idx < n_options);
> + po[n_po].o = &options[idx];
> + }
> + po[n_po].arg = optarg;
> + n_po++;
> + break;
> + }
> + }
> + OVS_NOT_REACHED();
> +
> +error:
> + free(po);
> + *pop = NULL;
> + *n_pop = 0;
> + free(short_options);
> + return error;
> +}
> +
> /* Given the 'struct ovs_cmdl_command' array, prints the usage of all commands. */
> void
> ovs_cmdl_print_commands(const struct ovs_cmdl_command commands[])
> diff --git a/lib/command-line.h b/lib/command-line.h
> index 00ace949bad6..9d62dc2501c5 100644
> --- a/lib/command-line.h
> +++ b/lib/command-line.h
> @@ -45,8 +45,18 @@ struct ovs_cmdl_command {
> };
>
> char *ovs_cmdl_long_options_to_short_options(const struct option *options);
> +
> +struct ovs_cmdl_parsed_option {
> + const struct option *o;
> + char *arg;
> +};
> +char *ovs_cmdl_parse_all(int argc, char *argv[], const struct option *,
> + struct ovs_cmdl_parsed_option **, size_t *)
> + OVS_WARN_UNUSED_RESULT;
> +
> void ovs_cmdl_print_options(const struct option *options);
> void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);
> +
> void ovs_cmdl_run_command(struct ovs_cmdl_context *,
> const struct ovs_cmdl_command[]);
> void ovs_cmdl_run_command_read_only(struct ovs_cmdl_context *,
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index c625546bb8c0..0659ced378ef 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -368,24 +368,23 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> {NULL, 0, NULL, 0},
> };
> const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
> - char *short_options;
> struct option *options;
> size_t i;
>
> - short_options = build_short_options(global_long_options, true);
> options = append_command_options(global_long_options, OPT_LOCAL);
>
> - for (;;) {
> - int idx;
> - int c;
> -
> - c = getopt_long(argc, argv, short_options, options, &idx);
> - if (c == -1) {
> - break;
> - }
> + struct ovs_cmdl_parsed_option *parsed_options;
> + size_t n_po;
> + char *error = ovs_cmdl_parse_all(argc, argv, options,
> + &parsed_options, &n_po);
> + if (error) {
> + ctl_fatal("%s", error);
> + }
>
> + for (const struct ovs_cmdl_parsed_option *po = parsed_options;
> + po < &parsed_options[n_po]; po++) {
> bool handled;
> - char *error = handle_main_loop_option(c, optarg, &handled);
> + error = handle_main_loop_option(po->o->val, po->arg, &handled);
> if (error) {
> ctl_fatal("%s", error);
> }
> @@ -393,9 +392,10 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> continue;
> }
>
> - switch (c) {
> + optarg = po->arg;
> + switch (po->o->val) {
> case OPT_DB:
> - db = optarg;
> + db = po->arg;
> break;
>
> case OPT_NO_SYSLOG:
> @@ -403,13 +403,13 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> break;
>
> case OPT_LOCAL:
> - if (shash_find(local_options, options[idx].name)) {
> + if (shash_find(local_options, po->o->name)) {
> ctl_fatal("'%s' option specified multiple times",
> - options[idx].name);
> + po->o->name);
> }
> shash_add_nocopy(local_options,
> - xasprintf("--%s", options[idx].name),
> - nullable_xstrdup(optarg));
> + xasprintf("--%s", po->o->name),
> + nullable_xstrdup(po->arg));
> break;
>
> case 'h':
> @@ -435,7 +435,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> STREAM_SSL_OPTION_HANDLERS
>
> case OPT_BOOTSTRAP_CA_CERT:
> - stream_ssl_set_ca_cert_file(optarg, true);
> + stream_ssl_set_ca_cert_file(po->arg, true);
> break;
>
> case '?':
> @@ -448,7 +448,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> break;
> }
> }
> - free(short_options);
> + free(parsed_options);
>
> if (!db) {
> db = default_nb_db();
> @@ -4984,6 +4984,7 @@ server_parse_options(int argc, char *argv[], struct shash *local_options,
> int *n_options_p)
> {
> static const struct option global_long_options[] = {
> + VLOG_LONG_OPTIONS,
> MAIN_LOOP_LONG_OPTIONS,
> TABLE_LONG_OPTIONS,
> {NULL, 0, NULL, 0},
>
More information about the dev
mailing list