On Sun, Apr 23, 2017 at 9:56 AM, Ben Pfaff <[email protected]> wrote:
> On Fri, Apr 07, 2017 at 02:43:40PM -0700, Yi-Hung Wei wrote:
>> In testcase "ofproto-dpif - fragment handling - actions", valgrind reports
>> memeory leaks with the following call stack.
>> xmalloc (util.c:112)
>> xvasprintf (util.c:176)
>> xasprintf (util.c:272)
>> mf_parse_subfield__ (nx-match.c:1939)
>> mf_parse_subfield (nx-match.c:1991)
>> learn_parse_spec (learn.c:242)
>> learn_parse__ (learn.c:436)
>> learn_parse (learn.c:464)
>> parse_LEARN (ofp-actions.c:4670)
>> ofpact_parse (ofp-actions.c:8231)
>> ofpacts_parse__ (ofp-actions.c:8278)
>> ofpacts_parse (ofp-actions.c:8350)
>> ofpacts_parse_copy (ofp-actions.c:8368)
>> parse_ofp_str__ (ofp-parse.c:543)
>> parse_ofp_str (ofp-parse.c:596)
>> parse_ofp_flow_mod_str (ofp-parse.c:1024)
>> ofctl_flow_mod (ovs-ofctl.c:1496)
>> ovs_cmdl_run_command__ (command-line.c:115)
>> main (ovs-ofctl.c:147)
>>
>> Signed-off-by: Yi-Hung Wei <[email protected]>
>> ---
>> lib/learn.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/learn.c b/lib/learn.c
>> index b7b70ac3d4e2..1c88b8fa402a 100644
>> --- a/lib/learn.c
>> +++ b/lib/learn.c
>> @@ -322,6 +322,7 @@ learn_parse_spec(const char *orig, char *name, char
>> *value,
>> char *tail;
>> char *dst_value = strstr(value, "->");
>>
>> + free(error);
>> if (dst_value == value) {
>> return xasprintf("%s: missing source before `->' in `%s'", name,
>> value);
>> @@ -358,6 +359,8 @@ learn_parse_spec(const char *orig, char *name, char
>> *value,
>> spec->dst = move.dst;
>> }
>> } else if (!strcmp(name, "output")) {
>> + free(error);
>> +
>> char *error = mf_parse_subfield(&spec->src, value);
>> if (error) {
>> return error;
>> @@ -367,6 +370,7 @@ learn_parse_spec(const char *orig, char *name, char
>> *value,
>> spec->src_type = NX_LEARN_SRC_FIELD;
>> spec->dst_type = NX_LEARN_DST_OUTPUT;
>> } else {
>> + free(error);
>> return xasprintf("%s: unknown keyword %s", orig, name);
>> }
>
> Thank you for the fix! That introduces a lot of code duplication. What
> do you think of this version?
>
> --8<--------------------------cut here-------------------------->8--
>
> From: Yi-Hung Wei <[email protected]>
> Date: Fri, 7 Apr 2017 14:43:40 -0700
> Subject: [PATCH] learn: Fix memory leak in learn_parse_sepc()
>
> In testcase "ofproto-dpif - fragment handling - actions", valgrind reports
> memeory leaks with the following call stack.
> xmalloc (util.c:112)
> xvasprintf (util.c:176)
> xasprintf (util.c:272)
> mf_parse_subfield__ (nx-match.c:1939)
> mf_parse_subfield (nx-match.c:1991)
> learn_parse_spec (learn.c:242)
> learn_parse__ (learn.c:436)
> learn_parse (learn.c:464)
> parse_LEARN (ofp-actions.c:4670)
> ofpact_parse (ofp-actions.c:8231)
> ofpacts_parse__ (ofp-actions.c:8278)
> ofpacts_parse (ofp-actions.c:8350)
> ofpacts_parse_copy (ofp-actions.c:8368)
> parse_ofp_str__ (ofp-parse.c:543)
> parse_ofp_str (ofp-parse.c:596)
> parse_ofp_flow_mod_str (ofp-parse.c:1024)
> ofctl_flow_mod (ovs-ofctl.c:1496)
> ovs_cmdl_run_command__ (command-line.c:115)
> main (ovs-ofctl.c:147)
>
> Signed-off-by: Yi-Hung Wei <[email protected]>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> lib/learn.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/lib/learn.c b/lib/learn.c
> index b7b70ac3d4e2..ebbc210c71ce 100644
> --- a/lib/learn.c
> +++ b/lib/learn.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
> + * Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -237,10 +237,12 @@ learn_parse_spec(const char *orig, char *name, char
> *value,
> {
> /* Parse destination and check prerequisites. */
> struct mf_subfield dst;
> - char *error;
>
> - error = mf_parse_subfield(&dst, name);
> - if (!error) {
> + char *error = mf_parse_subfield(&dst, name);
> + bool parse_error = error != NULL;
> + free(error);
> +
> + if (parse_error) {
Thanks, it looks much better. It seems like there is a typo here.
" if (!parse_error) { "
> if (!mf_nxm_header(dst.field->id)) {
> return xasprintf("%s: experimenter OXM field '%s' not supported",
> orig, name);
> --
> 2.10.2
>
-Yi-Hung
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
