From 1bc6ff291822a37ac95659f347e8edba125cd09c Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 27 Jan 2010 11:25:20 -0800 Subject: [PATCH] ovs-vsctl: Make parsing functions return error instead of aborting. The upcoming "remove" command for ovs-vsctl wants to try parsing an argument two different ways. This doesn't work if a parse error always aborts immediately. This commit fixes the problem, by making a parsing failure pass up an error for higher layers to deal with instead of aborting immediately. This commit should have no user-visible effect. --- lib/ovsdb-data.c | 155 +++++++++++++++++++++++++++--------------- lib/ovsdb-data.h | 12 ++-- utilities/ovs-vsctl.c | 126 +++++++++++++++++++++++----------- 3 files changed, 195 insertions(+), 98 deletions(-) diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c index cb68d09d..b89aa57a 100644 --- a/lib/ovsdb-data.c +++ b/lib/ovsdb-data.c @@ -371,8 +371,11 @@ ovsdb_atom_to_json(const union ovsdb_atom *atom, enum ovsdb_atomic_type type) * an arbitrary string. * * - OVSDB_TYPE_UUID: A UUID in RFC 4122 format. + * + * Returns a null pointer if successful, otherwise an error message describing + * the problem. The caller is responsible for freeing the error. */ -void +char * ovsdb_atom_from_string(union ovsdb_atom *atom, enum ovsdb_atomic_type type, const char *s) { @@ -383,7 +386,7 @@ ovsdb_atom_from_string(union ovsdb_atom *atom, enum ovsdb_atomic_type type, case OVSDB_TYPE_INTEGER: { long long int integer; if (!str_to_llong(s, 10, &integer)) { - ovs_fatal(0, "%s is not a valid integer", s); + return xasprintf("\"%s\" is not a valid integer", s); } atom->integer = integer; } @@ -391,7 +394,7 @@ ovsdb_atom_from_string(union ovsdb_atom *atom, enum ovsdb_atomic_type type, case OVSDB_TYPE_REAL: if (!str_to_double(s, &atom->real)) { - ovs_fatal(0, "%s is not a valid real number", s); + return xasprintf("\"%s\" is not a valid real number", s); } break; @@ -403,22 +406,26 @@ ovsdb_atom_from_string(union ovsdb_atom *atom, enum ovsdb_atomic_type type, || !strcmp(s, "0")) { atom->boolean = false; } else { - ovs_fatal(0, "%s is not a valid boolean " - "(use \"true\" or \"false\")", s); + return xasprintf("\"%s\" is not a valid boolean " + "(use \"true\" or \"false\")", s); } break; case OVSDB_TYPE_STRING: if (*s == '\0') { - ovs_fatal(0, "use \"\" to represent the empty string"); + return xstrdup("An empty string is not valid as input; " + "use \"\" to represent the empty string"); } else if (*s == '"') { size_t s_len = strlen(s); if (s_len < 2 || s[s_len - 1] != '"') { - ovs_fatal(0, "%s: missing quote at end of quoted string", s); + return xasprintf("%s: missing quote at end of " + "quoted string", s); } else if (!json_string_unescape(s + 1, s_len - 2, &atom->string)) { - ovs_fatal(0, "%s: %s", s, atom->string); + char *error = xasprintf("%s: %s", s, atom->string); + free(atom->string); + return error; } } else { atom->string = xstrdup(s); @@ -427,7 +434,7 @@ ovsdb_atom_from_string(union ovsdb_atom *atom, enum ovsdb_atomic_type type, case OVSDB_TYPE_UUID: if (!uuid_from_string(&atom->uuid, s)) { - ovs_fatal(0, "%s is not a valid UUID", s); + return xasprintf("\"%s\" is not a valid UUID", s); } break; @@ -435,6 +442,8 @@ ovsdb_atom_from_string(union ovsdb_atom *atom, enum ovsdb_atomic_type type, default: NOT_REACHED(); } + + return NULL; } static bool @@ -802,31 +811,42 @@ skip_spaces(const char *p) return p + strspn(p, " "); } -static const char * -parse_key_value(const char *s, const struct ovsdb_type *type, - union ovsdb_atom *key, union ovsdb_atom *value) +static char * +parse_atom_token(const char **s, enum ovsdb_atomic_type type, + union ovsdb_atom *atom) { - char *key_string; - const char *p; + char *token, *error; - /* Parse key. */ - p = ovsdb_token_parse(s, &key_string); - ovsdb_atom_from_string(key, type->key_type, key_string); - free(key_string); + error = ovsdb_token_parse(s, &token); + if (!error) { + error = ovsdb_atom_from_string(atom, type, token); + free(token); + } + return error; +} - /* Parse value. */ - if (type->value_type != OVSDB_TYPE_VOID) { - char *value_string; - if (*p != '=') { - ovs_fatal(0, "%s: syntax error at \"%c\" expecting \"=\"", - s, *p); +static char * +parse_key_value(const char **s, const struct ovsdb_type *type, + union ovsdb_atom *key, union ovsdb_atom *value) +{ + const char *start = *s; + char *error; + + error = parse_atom_token(s, type->key_type, key); + if (!error && type->value_type != OVSDB_TYPE_VOID) { + if (**s == '=') { + (*s)++; + error = parse_atom_token(s, type->value_type, value); + } else { + error = xasprintf("%s: syntax error at \"%c\" expecting \"=\"", + start, **s); + } + if (error) { + ovsdb_atom_destroy(key, type->key_type); } - p = ovsdb_token_parse(p + 1, &value_string); - ovsdb_atom_from_string(value, type->value_type, value_string); - free(value_string); } - return p; + return error; } static void @@ -845,13 +865,15 @@ free_key_value(const struct ovsdb_type *type, * acceptable to ovsdb_atom_from_string(). Optionally, a set may be enclosed * in "[]" or a map in "{}"; for an empty set or map these punctuators are * required. */ -void +char * ovsdb_datum_from_string(struct ovsdb_datum *datum, const struct ovsdb_type *type, const char *s) { bool is_map = ovsdb_type_is_map(type); + struct ovsdb_error *dberror; const char *p; int end_delim; + char *error; ovsdb_datum_init_empty(datum); @@ -862,9 +884,9 @@ ovsdb_datum_from_string(struct ovsdb_datum *datum, p = skip_spaces(p + 1); } else if (!*p) { if (is_map) { - ovs_fatal(0, "use \"{}\" to specify the empty map"); + return xstrdup("use \"{}\" to specify the empty map"); } else { - ovs_fatal(0, "use \"[]\" to specify the empty set"); + return xstrdup("use \"[]\" to specify the empty set"); } } else { end_delim = 0; @@ -874,12 +896,16 @@ ovsdb_datum_from_string(struct ovsdb_datum *datum, union ovsdb_atom key, value; if (ovsdb_token_is_delim(*p)) { - ovs_fatal(0, "%s: unexpected \"%c\" parsing %s", - s, *p, ovsdb_type_to_english(type)); + error = xasprintf("%s: unexpected \"%c\" parsing %s", + s, *p, ovsdb_type_to_english(type)); + goto error; } /* Add to datum. */ - p = parse_key_value(p, type, &key, &value); + error = parse_key_value(&p, type, &key, &value); + if (error) { + goto error; + } ovsdb_datum_add_unsafe(datum, &key, &value, type); free_key_value(type, &key, &value); @@ -891,34 +917,47 @@ ovsdb_datum_from_string(struct ovsdb_datum *datum, } if (*p != end_delim) { - ovs_fatal(0, "%s: missing \"%c\" at end of data", s, end_delim); + error = xasprintf("%s: missing \"%c\" at end of data", s, end_delim); + goto error; } if (end_delim) { p = skip_spaces(p + 1); if (*p) { - ovs_fatal(0, "%s: trailing garbage after \"%c\"", s, end_delim); + error = xasprintf("%s: trailing garbage after \"%c\"", + s, end_delim); + goto error; } } if (datum->n < type->n_min) { - ovs_fatal(0, "%s: %u %s were specified but at least %u are required", - s, datum->n, - type->value_type == OVSDB_TYPE_VOID ? "values" : "pairs", - type->n_min); + error = xasprintf("%s: %u %s specified but the minimum number is %u", + s, datum->n, is_map ? "pair(s)" : "value(s)", + type->n_min); + goto error; } else if (datum->n > type->n_max) { - ovs_fatal(0, "%s: %u %s were specified but at most %u are allowed", - s, datum->n, - type->value_type == OVSDB_TYPE_VOID ? "values" : "pairs", - type->n_max); + error = xasprintf("%s: %u %s specified but the maximum number is %u", + s, datum->n, is_map ? "pair(s)" : "value(s)", + type->n_max); + goto error; } - if (ovsdb_datum_sort(datum, type)) { + dberror = ovsdb_datum_sort(datum, type); + if (dberror) { + ovsdb_error_destroy(dberror); if (ovsdb_type_is_map(type)) { - ovs_fatal(0, "%s: map contains duplicate key", s); + error = xasprintf("%s: map contains duplicate key", s); } else { - ovs_fatal(0, "%s: set contains duplicate value", s); + error = xasprintf("%s: set contains duplicate value", s); } + goto error; } + + return NULL; + +error: + ovsdb_datum_destroy(datum, type); + ovsdb_datum_init_empty(datum); + return error; } /* Appends to 'out' the 'datum' (with the given 'type') in a format acceptable @@ -1292,23 +1331,25 @@ ovsdb_symbol_table_put(struct ovsdb_symbol_table *symtab, const char *name, * quotes are retained in the output. (Backslashes inside double quotes are * not removed, either.) */ -const char * -ovsdb_token_parse(const char *s, char **outp) +char * +ovsdb_token_parse(const char **s, char **outp) { const char *p; struct ds out; bool in_quotes; + char *error; ds_init(&out); in_quotes = false; - for (p = s; *p != '\0'; ) { + for (p = *s; *p != '\0'; ) { int c = *p++; if (c == '\\') { if (in_quotes) { ds_put_char(&out, '\\'); } if (!*p) { - ovs_fatal(0, "%s: backslash at end of argument", s); + error = xasprintf("%s: backslash at end of argument", *s); + goto error; } ds_put_char(&out, *p++); } else if (!in_quotes && ovsdb_token_is_delim(c)) { @@ -1322,10 +1363,18 @@ ovsdb_token_parse(const char *s, char **outp) } } if (in_quotes) { - ovs_fatal(0, "%s: quoted string extends past end of argument", s); + error = xasprintf("%s: quoted string extends past end of argument", + *s); + goto error; } *outp = ds_cstr(&out); - return p; + *s = p; + return NULL; + +error: + ds_destroy(&out); + *outp = NULL; + return error; } /* Returns true if 'c' delimits tokens, or if 'c' is 0, and false otherwise. */ diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h index 70dfd0f6..0638fa1b 100644 --- a/lib/ovsdb-data.h +++ b/lib/ovsdb-data.h @@ -74,8 +74,9 @@ struct ovsdb_error *ovsdb_atom_from_json(union ovsdb_atom *, struct json *ovsdb_atom_to_json(const union ovsdb_atom *, enum ovsdb_atomic_type); -void ovsdb_atom_from_string(union ovsdb_atom *, enum ovsdb_atomic_type, - const char *); +char *ovsdb_atom_from_string(union ovsdb_atom *, enum ovsdb_atomic_type, + const char *) + WARN_UNUSED_RESULT; void ovsdb_atom_to_string(const union ovsdb_atom *, enum ovsdb_atomic_type, struct ds *); @@ -128,8 +129,9 @@ struct ovsdb_error *ovsdb_datum_from_json(struct ovsdb_datum *, struct json *ovsdb_datum_to_json(const struct ovsdb_datum *, const struct ovsdb_type *); -void ovsdb_datum_from_string(struct ovsdb_datum *, - const struct ovsdb_type *, const char *); +char *ovsdb_datum_from_string(struct ovsdb_datum *, + const struct ovsdb_type *, const char *) + WARN_UNUSED_RESULT; void ovsdb_datum_to_string(const struct ovsdb_datum *, const struct ovsdb_type *, struct ds *); @@ -204,7 +206,7 @@ void ovsdb_symbol_table_put(struct ovsdb_symbol_table *, const char *name, * * Used by ovsdb_atom_from_string() and ovsdb_datum_from_string(). */ -const char *ovsdb_token_parse(const char *, char **outp); +char *ovsdb_token_parse(const char **, char **outp) WARN_UNUSED_RESULT; bool ovsdb_token_is_delim(unsigned char); #endif /* ovsdb-data.h */ diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index e33d2d1e..c5864b7e 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -1648,6 +1648,14 @@ static const struct vsctl_table_class tables[] = { {NULL, NULL, {{NULL, NULL, NULL}, {NULL, NULL, NULL}}} }; +static void +die_if_error(char *error) +{ + if (error) { + ovs_fatal(0, "%s", error); + } +} + static int to_lower_and_underscores(unsigned c) { @@ -1794,8 +1802,9 @@ get_row(struct vsctl_context *ctx, return row; } -static const struct vsctl_column * -get_column(const struct vsctl_table_class *table, const char *column_name) +static char * +get_column(const struct vsctl_table_class *table, const char *column_name, + const struct vsctl_column **columnp) { const struct vsctl_column *column; const struct vsctl_column *best_match = NULL; @@ -1813,44 +1822,52 @@ get_column(const struct vsctl_table_class *table, const char *column_name) } } } + + *columnp = best_match; if (best_match) { - return best_match; + return NULL; } else if (best_score) { - ovs_fatal(0, "%s has more than one column whose name matches \"%s\"", - table->class->name, column_name); + return xasprintf("%s contains more than one column whose name " + "matches \"%s\"", table->class->name, column_name); } else { - ovs_fatal(0, "%s does not have a column \"%s\"", - table->class->name, column_name); + return xasprintf("%s does not contain a column whose name matches " + "\"%s\"", table->class->name, column_name); } } -static void -check_trailer(const char *s, const char *p) -{ - if (*p != '\0') { - ovs_fatal(0, "%s: trailing garbage in argument at offset %td", - s, p - s); - } -} - -static void +static char * WARN_UNUSED_RESULT parse_column_key_value(const char *arg, const struct vsctl_table_class *table, const struct vsctl_column **columnp, char **keyp, char **valuep) { const char *p = arg; + char *error; assert(columnp || keyp); + if (keyp) { + *keyp = NULL; + } + if (valuep) { + *valuep = NULL; + } /* Parse column name. */ if (columnp) { char *column_name; - p = ovsdb_token_parse(arg, &column_name); + error = ovsdb_token_parse(&arg, &column_name); + if (error) { + goto error; + } if (column_name[0] == '\0') { - ovs_fatal(0, "%s: missing column name", arg); + free(column_name); + error = xasprintf("%s: missing column name", arg); + goto error; + } + error = get_column(table, column_name, columnp); + if (error) { + goto error; } - *columnp = get_column(table, column_name); free(column_name); } @@ -1859,9 +1876,13 @@ parse_column_key_value(const char *arg, const struct vsctl_table_class *table, if (columnp) { p++; } else if (!keyp) { - ovs_fatal(0, "%s: key not accepted here", arg); + error = xasprintf("%s: key not accepted here", arg); + goto error; + } + error = ovsdb_token_parse(&p, keyp); + if (error) { + goto error; } - p = ovsdb_token_parse(p, keyp); } else if (keyp) { *keyp = NULL; } @@ -1869,16 +1890,35 @@ parse_column_key_value(const char *arg, const struct vsctl_table_class *table, /* Parse value string. */ if (*p == '=') { if (!valuep) { - ovs_fatal(0, "%s: value not accepted here", arg); + error = xasprintf("%s: value not accepted here", arg); + goto error; } *valuep = xstrdup(p + 1); - return; } else { if (valuep) { *valuep = NULL; } - check_trailer(arg, p); + if (*p != '\0') { + error = xasprintf("%s: trailing garbage in argument at offset %td", + arg, p - arg); + goto error; + } + } + return NULL; + +error: + if (columnp) { + *columnp = NULL; + } + if (keyp) { + free(*keyp); + *keyp = NULL; + } + if (valuep) { + free(*valuep); + *valuep = NULL; } + return error; } static void @@ -1898,8 +1938,8 @@ cmd_get(struct vsctl_context *ctx) struct ovsdb_datum datum; char *key_string; - parse_column_key_value(ctx->argv[i], table, - &column, &key_string, NULL); + die_if_error(parse_column_key_value(ctx->argv[i], table, + &column, &key_string, NULL)); ovsdb_idl_txn_read(row, column->idl, &datum); if (key_string) { @@ -1911,8 +1951,9 @@ cmd_get(struct vsctl_context *ctx) column->idl->name); } - ovsdb_atom_from_string(&key, column->idl->type.key_type, - key_string); + die_if_error(ovsdb_atom_from_string(&key, + column->idl->type.key_type, + key_string)); idx = ovsdb_datum_find_key(&datum, &key, column->idl->type.key_type); @@ -1937,8 +1978,8 @@ cmd_get(struct vsctl_context *ctx) } static void -list_record(const struct vsctl_table_class *table, const struct ovsdb_idl_row *row, - struct ds *out) +list_record(const struct vsctl_table_class *table, + const struct ovsdb_idl_row *row, struct ds *out) { const struct vsctl_column *column; @@ -2092,9 +2133,11 @@ cmd_set(struct vsctl_context *ctx) for (i = 3; i < ctx->argc; i++) { const struct vsctl_column *column; char *key_string, *value_string; + char *error; - parse_column_key_value(ctx->argv[i], table, - &column, &key_string, &value_string); + error = parse_column_key_value(ctx->argv[i], table, + &column, &key_string, &value_string); + die_if_error(error); if (column->flags & VSCF_READONLY) { ovs_fatal(0, "%s: cannot modify read-only column %s in table %s", ctx->argv[i], column->idl->name, table_name); @@ -2112,10 +2155,12 @@ cmd_set(struct vsctl_context *ctx) column->idl->name); } - ovsdb_atom_from_string(&key, column->idl->type.key_type, - key_string); - ovsdb_atom_from_string(&value, column->idl->type.value_type, - value_string); + die_if_error(ovsdb_atom_from_string(&key, + column->idl->type.key_type, + key_string)); + die_if_error(ovsdb_atom_from_string(&value, + column->idl->type.value_type, + value_string)); ovsdb_datum_init_empty(&new); ovsdb_datum_add_unsafe(&new, &key, &value, &column->idl->type); @@ -2128,7 +2173,8 @@ cmd_set(struct vsctl_context *ctx) } else { struct ovsdb_datum datum; - ovsdb_datum_from_string(&datum, &column->idl->type, value_string); + die_if_error(ovsdb_datum_from_string(&datum, &column->idl->type, + value_string)); check_constraint(&datum, &column->idl->type, column->constraint); ovsdb_idl_txn_write(row, column->idl, &datum); } @@ -2153,7 +2199,7 @@ cmd_add(struct vsctl_context *ctx) table = get_table(table_name); row = get_row(ctx, table, record_id); - column = get_column(table, column_name); + die_if_error(get_column(table, column_name, &column)); type = &column->idl->type; ovsdb_idl_txn_read(row, column->idl, &old); for (i = 4; i < ctx->argc; i++) { @@ -2168,7 +2214,7 @@ cmd_add(struct vsctl_context *ctx) add_type = *type; add_type.n_min = 1; add_type.n_max = UINT_MAX; - ovsdb_datum_from_string(&add, &add_type, ctx->argv[i]); + die_if_error(ovsdb_datum_from_string(&add, &add_type, ctx->argv[i])); ovsdb_datum_union(&old, &add, type, false); ovsdb_datum_destroy(&add, type); } -- 2.30.2