[ovs-dev] [PATCH 2/2] db-ctl-base: Give better error messages for ambiguous abbreviations.

Ben Pfaff blp at ovn.org
Wed Sep 18 15:45:15 UTC 2019


THank you for the reviews.  I applied these to master.

On Wed, Sep 18, 2019 at 10:36:53AM -0700, Yifeng Sun wrote:
> LGTM, thanks.
> 
> Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
> 
> On Wed, Sep 18, 2019 at 9:32 AM Ben Pfaff <blp at ovn.org> wrote:
> >
> > Tables and columns may be abbreviated to unique prefixes, but until now
> > the error messages have just said there's more than one match.  This commit
> > makes the error messages list the possibilities.
> >
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> >  lib/db-ctl-base.c  | 58 +++++++++++++++++++++++++++++-----------------
> >  tests/ovs-vsctl.at |  5 +++-
> >  2 files changed, 41 insertions(+), 22 deletions(-)
> >
> > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> > index 3bd9f006acb1..6ae638be5a2b 100644
> > --- a/lib/db-ctl-base.c
> > +++ b/lib/db-ctl-base.c
> > @@ -430,31 +430,39 @@ static char *
> >  get_column(const struct ovsdb_idl_table_class *table, const char *column_name,
> >             const struct ovsdb_idl_column **columnp)
> >  {
> > +    struct sset best_matches = SSET_INITIALIZER(&best_matches);
> >      const struct ovsdb_idl_column *best_match = NULL;
> >      unsigned int best_score = 0;
> > -    size_t i;
> >
> > -    for (i = 0; i < table->n_columns; i++) {
> > +    for (size_t i = 0; i < table->n_columns; i++) {
> >          const struct ovsdb_idl_column *column = &table->columns[i];
> >          unsigned int score = score_partial_match(column->name, column_name);
> > -        if (score > best_score) {
> > +        if (score && score >= best_score) {
> > +            if (score > best_score) {
> > +                sset_clear(&best_matches);
> > +            }
> > +            sset_add(&best_matches, column->name);
> >              best_match = column;
> >              best_score = score;
> > -        } else if (score == best_score) {
> > -            best_match = NULL;
> >          }
> >      }
> >
> > -    *columnp = best_match;
> > -    if (best_match) {
> > -        return NULL;
> > -    } else if (best_score) {
> > -        return xasprintf("%s contains more than one column whose name "
> > -                         "matches \"%s\"", table->name, column_name);
> > +    char *error = NULL;
> > +    *columnp = NULL;
> > +    if (!best_match) {
> > +        error = xasprintf("%s does not contain a column whose name matches "
> > +                          "\"%s\"", table->name, column_name);
> > +    } else if (sset_count(&best_matches) == 1) {
> > +        *columnp = best_match;
> >      } else {
> > -        return xasprintf("%s does not contain a column whose name matches "
> > -                         "\"%s\"", table->name, column_name);
> > +        char *matches = sset_join(&best_matches, ", ", "");
> > +        error = xasprintf("%s contains more than one column "
> > +                          "whose name matches \"%s\": %s",
> > +                          table->name, column_name, matches);
> > +        free(matches);
> >      }
> > +    sset_destroy(&best_matches);
> > +    return error;
> >  }
> >
> >  static char * OVS_WARN_UNUSED_RESULT
> > @@ -1207,27 +1215,35 @@ cmd_list(struct ctl_context *ctx)
> >  static char * OVS_WARN_UNUSED_RESULT
> >  get_table(const char *table_name, const struct ovsdb_idl_table_class **tablep)
> >  {
> > +    struct sset best_matches = SSET_INITIALIZER(&best_matches);
> >      const struct ovsdb_idl_table_class *best_match = NULL;
> >      unsigned int best_score = 0;
> > -    char *error = NULL;
> >
> >      for (const struct ovsdb_idl_table_class *table = idl_classes;
> >           table < &idl_classes[n_classes]; table++) {
> >          unsigned int score = score_partial_match(table->name, table_name);
> > -        if (score > best_score) {
> > +        if (score && score >= best_score) {
> > +            if (score > best_score) {
> > +                sset_clear(&best_matches);
> > +            }
> > +            sset_add(&best_matches, table->name);
> >              best_match = table;
> >              best_score = score;
> > -        } else if (score == best_score) {
> > -            best_match = NULL;
> >          }
> >      }
> > -    if (best_match) {
> > +
> > +    char *error = NULL;
> > +    if (!best_match) {
> > +        error = xasprintf("unknown table \"%s\"", table_name);
> > +    } else if (sset_count(&best_matches) == 1) {
> >          *tablep = best_match;
> > -    } else if (best_score) {
> > -        error = xasprintf("multiple table names match \"%s\"", table_name);
> >      } else {
> > -        error = xasprintf("unknown table \"%s\"", table_name);
> > +        char *matches = sset_join(&best_matches, ", ", "");
> > +        error = xasprintf("\"%s\" matches multiple table names: %s",
> > +                          table_name, matches);
> > +        free(matches);
> >      }
> > +    sset_destroy(&best_matches);
> >      return error;
> >  }
> >
> > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > index 4907be35d342..1b9c6d90219d 100644
> > --- a/tests/ovs-vsctl.at
> > +++ b/tests/ovs-vsctl.at
> > @@ -848,6 +848,9 @@ targets             : ["1.2.3.4:567"]
> >  AT_CHECK([RUN_OVS_VSCTL([list interx x])],
> >    [1], [], [ovs-vsctl: unknown table "interx"
> >  ])
> > +AT_CHECK([RUN_OVS_VSCTL([list c x])],
> > +  [1], [], [ovs-vsctl: "c" matches multiple table names: CT_Timeout_Policy, CT_Zone, Controller
> > +])
> >  AT_CHECK([RUN_OVS_VSCTL([list bridge x])],
> >    [1], [], [ovs-vsctl: no row "x" in table Bridge
> >  ])
> > @@ -855,7 +858,7 @@ AT_CHECK([RUN_OVS_VSCTL([get bridge x datapath_id])],
> >    [1], [], [ovs-vsctl: no row "x" in table Bridge
> >  ])
> >  AT_CHECK([RUN_OVS_VSCTL([get bridge br0 d])],
> > -  [1], [], [ovs-vsctl: Bridge contains more than one column whose name matches "d"
> > +  [1], [], [ovs-vsctl: Bridge contains more than one column whose name matches "d": datapath_id, datapath_type, datapath_version
> >  ])
> >  AT_CHECK([RUN_OVS_VSCTL([get bridge br0 x])],
> >    [1], [], [ovs-vsctl: Bridge does not contain a column whose name matches "x"
> > --
> > 2.21.0
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list