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) {
         if (!mf_nxm_header(dst.field->id)) {
             return xasprintf("%s: experimenter OXM field '%s' not supported",
                              orig, name);
-- 
2.10.2

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to