[ovs-dev] [PATCH 07/23] db-ctl-base: Don't die in get_row_by_id() on multiple matches.

[Mark Michelson]

I'm not sure why you went with a different approach on this compared to 
the other functions. I would have expected you would change 
get_row_by_id() to return a string (NULL if no error, otherwise an error 
string). Then return the row as an output parameter.

On 07/02/2018 06:50 AM, Jakub Sitnicki wrote:
> Signal that multiple rows match the record identifier via a new output
> parameter instead of reporting the problem and dying, so that the caller
> can handle the error without terminating the process if needed.
> 
> Signed-off-by: Jakub Sitnicki <jkbs at redhat.com>
> ---
>   lib/db-ctl-base.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index e8f66811a..34daaef68 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -277,8 +277,11 @@ record_id_equals(const union ovsdb_atom *name, enum ovsdb_atomic_type type,
>   static const struct ovsdb_idl_row *
>   get_row_by_id(struct ctl_context *ctx,
>                 const struct ovsdb_idl_table_class *table,
> -              const struct ctl_row_id *id, const char *record_id)
> +              const struct ctl_row_id *id, const char *record_id,
> +              bool *multiple_matches)
>   {
> +    ovs_assert(multiple_matches);
> +    *multiple_matches = false;
>   
>       if (!id->name_column) {
>           return NULL;
> @@ -336,8 +339,8 @@ get_row_by_id(struct ctl_context *ctx,
>           /* If the name equals 'record_id', take it. */
>           if (record_id_equals(name, name_type, record_id)) {
>               if (referrer) {
> -                ctl_fatal("multiple rows in %s match \"%s\"",
> -                          id_table->name, record_id);
> +                *multiple_matches = true;
> +                return NULL;
>               }
>               referrer = row;
>           }
> @@ -386,8 +389,18 @@ ctl_get_row(struct ctl_context *ctx,
>           const struct ctl_table_class *ctl_class
>               = &ctl_classes[table - idl_classes];
>           for (int i = 0; i < ARRAY_SIZE(ctl_class->row_ids); i++) {
> -            row = get_row_by_id(ctx, table, &ctl_class->row_ids[i],
> -                                record_id);
> +            const struct ctl_row_id *id = &ctl_class->row_ids[i];
> +            bool multiple_matches;
> +
> +            row = get_row_by_id(ctx, table, id, record_id, &multiple_matches);
> +            if (multiple_matches) {
> +                const struct ovsdb_idl_class *class =
> +                    ovsdb_idl_get_class(ctx->idl);
> +                const struct ovsdb_idl_table_class *table_class =
> +                    ovsdb_idl_table_class_from_column(class, id->name_column);
> +                ctl_fatal("multiple rows in %s match \"%s\"",
> +                          table_class->name, record_id);
> +            }
>               if (row) {
>                   break;
>               }
>