| Message ID | 20190918143252.3317-2-blp@ovn.org |
|---|---|
| State | Accepted |
| Commit | cf0e4239460cdf702be05d9abe781d089dbf7dd7 |
| Headers | show |
| Series | [ovs-dev,1/2] sset: New function sset_join(). | expand |
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@gmail.com> > > On Wed, Sep 18, 2019 at 9:32 AM Ben Pfaff <blp@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@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@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
LGTM, thanks. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Wed, Sep 18, 2019 at 9:32 AM Ben Pfaff <blp@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@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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"
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@ovn.org> --- lib/db-ctl-base.c | 58 +++++++++++++++++++++++++++++----------------- tests/ovs-vsctl.at | 5 +++- 2 files changed, 41 insertions(+), 22 deletions(-)