> On Aug 14, 2016, at 3:24 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index b56fcd5..b6350ae 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -1,4 +1,4 @@
> -/*
> + /*
> * Copyright (c) 2015, 2016 Nicira, Inc.
My guess is that this change to the beginning of the comment was in error.
> @@ -1835,7 +1725,7 @@ parse_actions(struct action_context *ctx)
> * '*prereqsp' is set to NULL, but some tokens may have been consumed from
> * 'lexer'.
> */
> -char * OVS_WARN_UNUSED_RESULT
> +bool
> ovnacts_parse(struct lexer *lexer, const struct ovnact_parse_params *pp,
> struct ofpbuf *ovnacts, struct expr **prereqsp)
> {
The comment describing ovnacts_parse() needs to be updated about the return
value.
> diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
> index 25281e0..c3a26f5 100644
> --- a/ovn/lib/expr.c
> +++ b/ovn/lib/expr.c
...
> @@ -842,10 +777,14 @@ parse_constant_set(struct expr_context *ctx, struct
> expr_constant_set *cs)
> * type of 'f' into 'c'. On success, returns NULL. On failure, returns an
> * xmalloc()'ed error message that the caller must free and 'c' is
> * indeterminate. */
> -char * OVS_WARN_UNUSED_RESULT
> +bool
> expr_constant_parse(struct lexer *lexer, const struct expr_field *f,
> union expr_constant *c)
The return value and error handling changed in this function, but the comment
wasn't updated to match.
> @@ -900,12 +839,14 @@ expr_constant_destroy(const union expr_constant *c,
> * Returns NULL on success, in which case the caller owns 'cs', otherwise a
> * xmalloc()'ed error message owned by the caller, in which case 'cs' is
> * indeterminate. */
> -char * OVS_WARN_UNUSED_RESULT
> +bool
> expr_constant_set_parse(struct lexer *lexer, struct expr_constant_set *cs)
Same here.
>
> @@ -1183,15 +1127,12 @@ expr_parse__(struct expr_context *ctx)
> * expr_destroy()) or error (with free()). */
> struct expr *
> expr_parse(struct lexer *lexer, const struct shash *symtab,
> - const struct shash *macros, char **errorp)
> + const struct shash *macros)
The 'errorp' argument in expr_parse() is no longer present, but the comment
style documents it.
> {
> struct expr_context ctx = { .lexer = lexer,
> .symtab = symtab,
> .macros = macros };
> - struct expr *e = expr_parse__(&ctx);
> - *errorp = ctx.error;
> - ovs_assert((ctx.error != NULL) != (e != NULL));
> - return e;
> + return lexer->error ? NULL : expr_parse__(&ctx);
> }
>
> /* Like expr_parse(), but the expression is taken from 's'. */
> @@ -1200,13 +1141,13 @@ expr_parse_string(const char *s, const struct shash
> *symtab,
> const struct shash *macros, char **errorp)
> {
expr_parse_string() still uses 'errorp', but it's no longer documented, since
before the only reference was from expr_parse().
> @@ -1218,23 +1159,25 @@ expr_parse_string(const char *s, const struct shash
> *symtab,
> /* Parses a field or subfield from 'lexer' into 'field', obtaining field names
> * from 'symtab'. Returns NULL if successful, otherwise an error message
> owned
> * by the caller. */
> -char * OVS_WARN_UNUSED_RESULT
> +bool
> expr_field_parse(struct lexer *lexer, const struct shash *symtab,
> struct expr_field *field, struct expr **prereqsp)
The comment for expr_field_parse() needs to be updated, too.
> {
> struct expr_context ctx = { .lexer = lexer, .symtab = symtab };
> if (parse_field(&ctx, field) && field->symbol->predicate) {
> - ctx.error = xasprintf("Predicate symbol %s used where lvalue "
> - "required.", field->symbol->name);
> + lexer_error(lexer, "Predicate symbol %s used where lvalue required.",
> + field->symbol->name);
> }
> - if (!ctx.error) {
> + if (!lexer->error) {
> const struct expr_symbol *symbol = field->symbol;
> while (symbol) {
> if (symbol->prereqs) {
> + char *error;
> struct ovs_list nesting = OVS_LIST_INITIALIZER(&nesting);
> struct expr *e = parse_and_annotate(symbol->prereqs, symtab,
> - &nesting, &ctx.error);
> - if (ctx.error) {
> + &nesting, &error);
> + if (error) {
> + lexer_error(lexer, "%s", error);
> break;
Don't we need to free 'error'?
> diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c
> index 95edeaf..a05edfa 100644
> --- a/ovn/lib/lex.c
> +++ b/ovn/lib/lex.c
> ...
> +bool
> +lexer_force_match(struct lexer *lexer, enum lex_type t)
> +{
> + if (lexer_match(lexer, t)) {
> + return true;
> + } else {
> + struct lex_token token = { .type = t };
> + struct ds s = DS_EMPTY_INITIALIZER;
> + lex_token_format(&token, &s);
> +
> + lexer_syntax_error(lexer, "expecting `%s'", ds_cstr(&s));
Not a big deal at all, but using different left and right quotes looks a little
odd to me in an error message.
Acked-by: Justin Pettit <jpet...@ovn.org>
--Justin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
