"dev" <dev-boun...@openvswitch.org> wrote on 06/09/2016 12:37:58 AM:
> From: Ben Pfaff <b...@ovn.org>
> To: dev@openvswitch.org
> Cc: Ben Pfaff <b...@ovn.org>
> Subject: [ovs-dev] [PATCH 2/4] expr: Refactor parsing of assignments and
exchanges.
> Sent by: "dev" <dev-boun...@openvswitch.org>
[snip]
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -106,19 +106,35 @@ action_syntax_error(struct action_context *ctx,
const char *message, ...)
> static void
> parse_set_action(struct action_context *ctx)
> {
> - struct expr *prereqs;
> + struct expr *prereqs = NULL;
> + struct expr_field dst;
> char *error;
>
> - error = expr_parse_assignment(ctx->lexer, ctx->ap->symtab,
> - ctx->ap->lookup_port, ctx->ap->aux,
> - ctx->ofpacts, &prereqs);
> + error = expr_parse_field(ctx->lexer, ctx->ap->symtab, &dst);
> if (error) {
> + goto exit;
> + }
> +
> + if (lexer_match(ctx->lexer, LEX_T_EXCHANGE)) {
> + error = expr_parse_exchange(ctx->lexer, &dst, ctx->ap->symtab,
> + ctx->ap->lookup_port, ctx->ap->aux,
> + ctx->ofpacts, &prereqs);
> + } else if (lexer_match(ctx->lexer, LEX_T_EQUALS)) {
> + error = expr_parse_assignment(
> + ctx->lexer, &dst, ctx->ap->symtab, ctx->ap->lookup_port,
> + ctx->ap->aux, ctx->ofpacts, &prereqs);
> + } else {
> + action_syntax_error(ctx, "expecting `=' or `<->'");
> + }
> +
> +exit:
> + if (!error) {
> + ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, prereqs);
> + } else {
> + expr_destroy(prereqs);
> action_error(ctx, "%s", error);
> free(error);
> - return;
> }
> -
> - ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, prereqs);
> }
>
> static void
I looked at this long and hard, and while I don't mind the use of gotos in
C code,
in this case I found it a bit "jarring" and wonder if the following isn't
more readable:
error = expr_parse_field(ctx->lexer, ctx->ap->symtab, &dst);
if (!error) {
if (lexer_match(ctx->lexer, LEX_T_EXCHANGE)) {
error = expr_parse_exchange(ctx->lexer, &dst, ctx->ap->symtab,
ctx->ap->lookup_port, ctx->ap->aux,
ctx->ofpacts, &prereqs);
} else if (lexer_match(ctx->lexer, LEX_T_EQUALS)) {
error = expr_parse_assignment(
ctx->lexer, &dst, ctx->ap->symtab, ctx->ap->lookup_port,
ctx->ap->aux, ctx->ofpacts, &prereqs);
} else {
action_syntax_error(ctx, "expecting `=' or `<->'");
}
if (!error) {
ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, prereqs);
}
}
if (error) {
expr_destroy(prereqs);
action_error(ctx, "%s", error);
free(error);
}
Ryan
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
