[ovs-dev] [PATCH 1/2] ovs-vsctl: Factor out and optimize searching for a command by name.

Ben Pfaff blp at nicira.com
Fri Oct 1 17:23:04 UTC 2010


Thanks, I pushed out both of these.

On Thu, Sep 30, 2010 at 10:49:35AM -0700, Justin Pettit wrote:
> Looks good.
> 
> --Justin
> 
> 
> On Sep 1, 2010, at 1:46 PM, Ben Pfaff wrote:
> 
> > The following commit will introduce a new function that wants to do this
> > a lot, so we might as well do it efficiently.
> > ---
> > utilities/ovs-vsctl.c |  102 ++++++++++++++++++++++++++++--------------------
> > 1 files changed, 59 insertions(+), 43 deletions(-)
> > 
> > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> > index 4d50194..6eea509 100644
> > --- a/utilities/ovs-vsctl.c
> > +++ b/utilities/ovs-vsctl.c
> > @@ -105,6 +105,7 @@ static void parse_options(int argc, char *argv[]);
> > static struct vsctl_command *parse_commands(int argc, char *argv[],
> >                                             size_t *n_commandsp);
> > static void parse_command(int argc, char *argv[], struct vsctl_command *);
> > +static const struct vsctl_command_syntax *find_command(const char *name);
> > static void do_vsctl(const char *args,
> >                      struct vsctl_command *, size_t n_commands,
> >                      struct ovsdb_idl *);
> > @@ -295,6 +296,8 @@ static void
> > parse_command(int argc, char *argv[], struct vsctl_command *command)
> > {
> >     const struct vsctl_command_syntax *p;
> > +    struct shash_node *node;
> > +    int n_arg;
> >     int i;
> > 
> >     shash_init(&command->options);
> > @@ -325,58 +328,71 @@ parse_command(int argc, char *argv[], struct vsctl_command *command)
> >         vsctl_fatal("missing command name");
> >     }
> > 
> > -    for (p = all_commands; p->name; p++) {
> > -        if (!strcmp(p->name, argv[i])) {
> > -            struct shash_node *node;
> > -            int n_arg;
> > +    p = find_command(argv[i]);
> > +    if (!p) {
> > +        vsctl_fatal("unknown command '%s'; use --help for help", argv[i]);
> > +    }
> > 
> > -            SHASH_FOR_EACH (node, &command->options) {
> > -                const char *s = strstr(p->options, node->name);
> > -                int end = s ? s[strlen(node->name)] : EOF;
> > +    SHASH_FOR_EACH (node, &command->options) {
> > +        const char *s = strstr(p->options, node->name);
> > +        int end = s ? s[strlen(node->name)] : EOF;
> > 
> > -                if (end != '=' && end != ',' && end != ' ' && end != '\0') {
> > -                    vsctl_fatal("'%s' command has no '%s' option",
> > -                                argv[i], node->name);
> > -                }
> > -                if ((end == '=') != (node->data != NULL)) {
> > -                    if (end == '=') {
> > -                        vsctl_fatal("missing argument to '%s' option on '%s' "
> > -                                    "command", node->name, argv[i]);
> > -                    } else {
> > -                        vsctl_fatal("'%s' option on '%s' does not accept an "
> > -                                    "argument", node->name, argv[i]);
> > -                    }
> > -                }
> > +        if (end != '=' && end != ',' && end != ' ' && end != '\0') {
> > +            vsctl_fatal("'%s' command has no '%s' option",
> > +                        argv[i], node->name);
> > +        }
> > +        if ((end == '=') != (node->data != NULL)) {
> > +            if (end == '=') {
> > +                vsctl_fatal("missing argument to '%s' option on '%s' "
> > +                            "command", node->name, argv[i]);
> > +            } else {
> > +                vsctl_fatal("'%s' option on '%s' does not accept an "
> > +                            "argument", node->name, argv[i]);
> >             }
> > +        }
> > +    }
> > 
> > -            n_arg = argc - i - 1;
> > -            if (n_arg < p->min_args) {
> > -                vsctl_fatal("'%s' command requires at least %d arguments",
> > -                            p->name, p->min_args);
> > -            } else if (n_arg > p->max_args) {
> > -                int j;
> > -
> > -                for (j = i + 1; j < argc; j++) {
> > -                    if (argv[j][0] == '-') {
> > -                        vsctl_fatal("'%s' command takes at most %d arguments "
> > -                                    "(note that options must precede command "
> > -                                    "names and follow a \"--\" argument)",
> > -                                    p->name, p->max_args);
> > -                    }
> > -                }
> > +    n_arg = argc - i - 1;
> > +    if (n_arg < p->min_args) {
> > +        vsctl_fatal("'%s' command requires at least %d arguments",
> > +                    p->name, p->min_args);
> > +    } else if (n_arg > p->max_args) {
> > +        int j;
> > 
> > -                vsctl_fatal("'%s' command takes at most %d arguments",
> > +        for (j = i + 1; j < argc; j++) {
> > +            if (argv[j][0] == '-') {
> > +                vsctl_fatal("'%s' command takes at most %d arguments "
> > +                            "(note that options must precede command "
> > +                            "names and follow a \"--\" argument)",
> >                             p->name, p->max_args);
> > -            } else {
> > -                command->syntax = p;
> > -                command->argc = n_arg + 1;
> > -                command->argv = &argv[i];
> > -                return;
> >             }
> >         }
> > +
> > +        vsctl_fatal("'%s' command takes at most %d arguments",
> > +                    p->name, p->max_args);
> > +    }
> > +
> > +    command->syntax = p;
> > +    command->argc = n_arg + 1;
> > +    command->argv = &argv[i];
> > +}
> > +
> > +/* Returns the "struct vsctl_command_syntax" for a given command 'name', or a
> > + * null pointer if there is none. */
> > +static const struct vsctl_command_syntax *
> > +find_command(const char *name)
> > +{
> > +    static struct shash commands = SHASH_INITIALIZER(&commands);
> > +
> > +    if (shash_is_empty(&commands)) {
> > +        const struct vsctl_command_syntax *p;
> > +
> > +        for (p = all_commands; p->name; p++) {
> > +            shash_add_assert(&commands, p->name, p);
> > +        }
> >     }
> > 
> > -    vsctl_fatal("unknown command '%s'; use --help for help", argv[i]);
> > +    return shash_find_data(&commands, name);
> > }
> > 
> > static void
> > -- 
> > 1.7.1
> > 
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
> 




More information about the dev mailing list