You're right; I was thinking that "ctx" was being handed to "expr_parse_field".
I pushed the change to master.
Thanks!
--Justin
> On Apr 5, 2016, at 3:51 PM, William Tu <u9012...@gmail.com> wrote:
>
> Hi Justin,
> Thanks for the feedback. I think the original code is OK, although a little
> misleading.
>
> At action_parse_field(), the address of *ctx is not passed into
> expr_parse_field. The expr_parse_field has a ctx as local variable on its
> stack. So when it returns, the 'error' contains a newly malloc char* and
> error != ctx->error.
>
> It is when calling action_error(), then the 'ctx->error' is NULL and gets a
> copy of 'error', so we need to free(error).
>
> Regards,
> William
>
> On Tue, Apr 5, 2016 at 3:08 PM, Justin Pettit <jpet...@ovn.org> wrote:
>
> > On Apr 4, 2016, at 2:51 PM, William Tu <u9012...@gmail.com> wrote:
> >
> > Reported by test case 2015: ovn -- action parsing.
> > xvasprintf (util.c:164)
> > expr_error (expr.c:489)
> > expr_parse_field (expr.c:2910)
> > action_parse_field (actions.c:287)
> >
> > Signed-off-by: William Tu <u9012...@gmail.com>
> > ---
> > ovn/lib/actions.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> > index 44957c7..a17b5a7 100644
> > --- a/ovn/lib/actions.c
> > +++ b/ovn/lib/actions.c
> > @@ -288,6 +288,7 @@ action_parse_field(struct action_context *ctx,
> > &prereqs);
> > if (error) {
> > action_error(ctx, "%s", error);
> > + free(error);
> > return false;
> > }
>
> Unless I'm reading it wrong, the original code seems a little funky. "error"
> is set by expr_parse_field() as the value of "ctx->error". "ctx" is then
> handed to action_error() which won't do anything since "ctx->error" is not
> NULL. It seems like this call to action_error() is not necessary at all.
>
> Back to your fix, if there is a problem, it seems like action_force_match()
> would have similar problems to action_parse_field(); they're only called by
> parse_action(). You're essentially freeing "ctx->error", but isn't it needed
> by parse_action()?
>
> --Justin
>
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
