[ovs-dev] [PATCH 01/23] db-ctl-base: Don't die in parse_column_names() on error.

Mark Michelson mmichels at redhat.com
Mon Jul 2 14:55:01 UTC 2018


Hi Jakub,

Your cover letter still hasn't come through, so feel free to tell me if 
my comments here are misguided.

I'm guessing that you did not remove ctl_fatal() calls from 
parse_commands() and ctl_parse_commands() because there is no 
ctl_context present. However, it's not clear why the ctl_fatal() call in 
cmd_add() was not converted to use ctl_error() + return.

I think patches 15 and 19 could be combined into one.

You *could* combine patches 1-13 into one patch, 16-18 into one patch, 
and 20-23 in one patch, but I don't feel strongly enough about this to 
make you have to re-do the whole thing.

On 07/02/2018 06:49 AM, Jakub Sitnicki wrote:
> Return the error message to the caller instead of reporting it 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 | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index 0e26f09cc..d589df487 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -918,7 +918,8 @@ cmd_get(struct ctl_context *ctx)
>       }
>   }
>   
> -static void
> +/* Returns NULL on success or malloc()'ed error message on failure. */
> +static char * OVS_WARN_UNUSED_RESULT
>   parse_column_names(const char *column_names,
>                      const struct ovsdb_idl_table_class *table,
>                      const struct ovsdb_idl_column ***columnsp,
> @@ -951,7 +952,12 @@ parse_column_names(const char *column_names,
>               if (!strcasecmp(column_name, "_uuid")) {
>                   column = NULL;
>               } else {
> -                die_if_error(get_column(table, column_name, &column));
> +                char *error = get_column(table, column_name, &column);
> +                if (error) {
> +                    free(columns);
> +                    free(s);
> +                    return error;
> +                }
>               }
>               if (n_columns >= allocated_columns) {
>                   columns = x2nrealloc(columns, &allocated_columns,
> @@ -962,11 +968,12 @@ parse_column_names(const char *column_names,
>           free(s);
>   
>           if (!n_columns) {
> -            ctl_fatal("must specify at least one column name");
> +            return xstrdup("must specify at least one column name");
>           }
>       }
>       *columnsp = columns;
>       *n_columnsp = n_columns;
> +    return NULL;
>   }
>   
>   static void
> @@ -978,7 +985,7 @@ pre_list_columns(struct ctl_context *ctx,
>       size_t n_columns;
>       size_t i;
>   
> -    parse_column_names(column_names, table, &columns, &n_columns);
> +    die_if_error(parse_column_names(column_names, table, &columns, &n_columns));
>       for (i = 0; i < n_columns; i++) {
>           if (columns[i]) {
>               ovsdb_idl_add_column(ctx->idl, columns[i]);
> @@ -1067,7 +1074,7 @@ cmd_list(struct ctl_context *ctx)
>       int i;
>   
>       table = get_table(table_name);
> -    parse_column_names(column_names, table, &columns, &n_columns);
> +    die_if_error(parse_column_names(column_names, table, &columns, &n_columns));
>       out = ctx->table = list_make_table(columns, n_columns);
>       if (ctx->argc > 2) {
>           for (i = 2; i < ctx->argc; i++) {
> @@ -1140,7 +1147,7 @@ cmd_find(struct ctl_context *ctx)
>       size_t n_columns;
>   
>       table = get_table(table_name);
> -    parse_column_names(column_names, table, &columns, &n_columns);
> +    die_if_error(parse_column_names(column_names, table, &columns, &n_columns));
>       out = ctx->table = list_make_table(columns, n_columns);
>       for (row = ovsdb_idl_first_row(ctx->idl, table); row;
>            row = ovsdb_idl_next_row(row)) {
> 



More information about the dev mailing list