[ovs-dev] [PATCH] valgrind: Fix memory leak at expr_error.

Justin Pettit jpettit at ovn.org
Tue Apr 5 22:08:49 UTC 2016


> On Apr 4, 2016, at 2:51 PM, William Tu <u9012063 at 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 <u9012063 at 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





More information about the dev mailing list