On Thu, Sep 11, 2014 at 05:38:21PM -0700, Alex Wang wrote:
> This commit adds function that extracts (both long and short)
> options and returns them as string. Right now, only ovs-appctl
> command provides an option to print this function output. And
> the output is to be used by bash command-line completion script
> for completion other options of the same command.
>
> Signed-off-by: Alex Wang <[email protected]>
The output format doesn't distinguish between optional and required
options. Should it?
> + ret = xstrdup(ds_cstr(&ds));
> + ds_destroy(&ds);
> +
> + return ret;
I would replace the above by just "return ds.string;"
> static void
> +print_appctl_options(const struct option options[])
> +{
> + char *opts;
> +
> + opts = print_options(options);
> + printf("%s", opts);
> + free(opts);
> +
> + exit(EXIT_SUCCESS);
> +}
print_appctl_options() plus print_options() makes for a flexible
interface, but do you expect that there will be other users that don't
want to what print_appctl_options() does? I'd be inclined toward just
making print_options() do what print_appctl_options() does and skip the
extra wrapper.
None of this matters that much so:
Acked-by: Ben Pfaff <[email protected]>
but I don't want it to be applied until the command-line completion code
is also reviewed by someone.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
