On Fri, Jul 28, 2017 at 03:48:38PM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit <[email protected]> > Acked-by: Han Zhou <[email protected]> > --- > v1->v2: Incorporate Ben and Han's feedback. > Improve output of "ovn-nbctl acl-list".
Thank you! I'm appending a few final suggestions. The only important one is the one that moves the free() call, which I believe fixes a double-free on the error path: the old name is freed but not set to NULL, which means that ovnact_log_free() eventually frees it again. Acked-by: Ben Pfaff <[email protected]> --8<--------------------------cut here-------------------------->8-- diff --git a/include/ovn/actions.h b/include/ovn/actions.h index b88effee7437..0a04af7aaf79 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -415,7 +415,6 @@ enum action_opcode { * Arguments are as follows: * - An 8-bit verdict. * - An 8-bit severity. - * - An 16-bit string length for the name. * - A variable length string containing the name. */ ACTION_OPCODE_LOG, diff --git a/ovn/lib/acl-log.c b/ovn/lib/acl-log.c index 0f5e43aff720..f47b0af437da 100644 --- a/ovn/lib/acl-log.c +++ b/ovn/lib/acl-log.c @@ -78,7 +78,7 @@ log_severity_from_string(const char *name) void handle_acl_log(const struct flow *headers, struct ofpbuf *userdata) { - if(!VLOG_IS_INFO_ENABLED()) { + if (!VLOG_IS_INFO_ENABLED()) { return; } diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index f010b05a1f48..d0d73b69c32d 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -1781,9 +1781,6 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) return; } /* If multiple names are given, use the most recent. */ - if (log->name) { - free(log->name); - } if (ctx->lexer->token.type == LEX_T_STRING) { /* Arbitrarily limit the name length to 64 bytes, since * these will be encoded in datapath actions. */ @@ -1792,6 +1789,7 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) "than 64 characters"); return; } + free(log->name); log->name = xstrdup(ctx->lexer->token.s); } else { lexer_syntax_error(ctx->lexer, "expecting string"); _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
