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

Yifeng Sun pkusunyifeng at gmail.com
Wed Sep 18 17:36:53 UTC 2019


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