diff mbox

[ovs-dev,v2,3/3] odp-util.c: fix memory leak reported by valgrind

Message ID 1451953121-23921-3-git-send-email-u9012063@gmail.com
State Accepted
Headers show

Commit Message

William Tu Jan. 5, 2016, 12:18 a.m. UTC 
Test case: OVS datapath key parsing and formatting (377)
Return without freeing buf:
    xmalloc(util.c:112)
    ofpbuf_init(ofpbuf.c:124)
    parse_odp_userspace_action(odp-util.c:987)
    parse_odp_action(odp-util.c:1552)
    odp_actions_from_string(odp-util.c:1721)
    parse_actions(test-odp.c:132)

Test case: OVS datapath actions parsing and formatting (380)
Exit withtou uninit in test-odp.c
    xrealloc(util.c:123)
    ofpbuf_resize__(ofpbuf.c:243)
    ofpbuf_put_uninit(ofpbuf.c:364)
    nl_msg_put_uninit(netlink.c:178)
    nl_msg_put_unspec_uninit(netlink.c:216)
    nl_msg_put_unspec(netlink.c:243)
    parse_odp_key_mask_attr(odp-util.c:3974)
    odp_flow_from_string(odp-util.c:4151)
    parse_keys(test-odp.c:49)
    test_odp_main(test-odp.c:237)
    ovstest_wrapper_test_odp_main__(test-odp.c:251)
    ovs_cmdl_run_command(command-line.c:121)
    main(ovstest.c:132)

Signed-off-by: William Tu <u9012063@gmail.com>
Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Co-authored-by: Daniele Di Proietto <diproiettod@vmware.com>
---
 lib/odp-util.c   | 22 +++++++++++++---------
 tests/test-odp.c |  1 +
 2 files changed, 14 insertions(+), 9 deletions(-)

Comments

Ben Pfaff Jan. 5, 2016, 1:01 a.m. UTC | #1 
On Mon, Jan 04, 2016 at 04:18:41PM -0800, William Tu wrote:
> Test case: OVS datapath key parsing and formatting (377)
> Return without freeing buf:
>     xmalloc(util.c:112)
>     ofpbuf_init(ofpbuf.c:124)
>     parse_odp_userspace_action(odp-util.c:987)
>     parse_odp_action(odp-util.c:1552)
>     odp_actions_from_string(odp-util.c:1721)
>     parse_actions(test-odp.c:132)
> 
> Test case: OVS datapath actions parsing and formatting (380)
> Exit withtou uninit in test-odp.c
>     xrealloc(util.c:123)
>     ofpbuf_resize__(ofpbuf.c:243)
>     ofpbuf_put_uninit(ofpbuf.c:364)
>     nl_msg_put_uninit(netlink.c:178)
>     nl_msg_put_unspec_uninit(netlink.c:216)
>     nl_msg_put_unspec(netlink.c:243)
>     parse_odp_key_mask_attr(odp-util.c:3974)
>     odp_flow_from_string(odp-util.c:4151)
>     parse_keys(test-odp.c:49)
>     test_odp_main(test-odp.c:237)
>     ovstest_wrapper_test_odp_main__(test-odp.c:251)
>     ovs_cmdl_run_command(command-line.c:121)
>     main(ovstest.c:132)
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
> Co-authored-by: Daniele Di Proietto <diproiettod@vmware.com>

Applied, thanks!
diff mbox

Patch

diff --git a/lib/odp-util.c b/lib/odp-util.c
index aa5bd6b..f16e113 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -909,11 +909,14 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
     void *user_data = NULL;
     size_t user_data_size = 0;
     bool include_actions = false;
+    int res;
 
     if (!ovs_scan(s, "userspace(pid=%"SCNi32"%n", &pid, &n)) {
         return -EINVAL;
     }
 
+    ofpbuf_init(&buf, 16);
+
     {
         uint32_t output;
         uint32_t probability;
@@ -940,8 +943,6 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             user_data_size = sizeof cookie.sflow;
         } else if (ovs_scan(&s[n], ",slow_path(%n",
                             &n1)) {
-            int res;
-
             n += n1;
             cookie.type = USER_ACTION_COOKIE_SLOW_PATH;
             cookie.slow_path.unused = 0;
@@ -951,7 +952,7 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
                                   &cookie.slow_path.reason,
                                   SLOW_PATH_REASON_MASK, NULL);
             if (res < 0 || s[n + res] != ')') {
-                return res;
+                goto out;
             }
             n += res + 1;
 
@@ -984,10 +985,10 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             char *end;
 
             n += n1;
-            ofpbuf_init(&buf, 16);
             end = ofpbuf_put_hex(&buf, &s[n], NULL);
             if (end[0] != ')') {
-                return -EINVAL;
+                res = -EINVAL;
+                goto out;
             }
             user_data = buf.data;
             user_data_size = buf.size;
@@ -1009,15 +1010,18 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
                      &tunnel_out_port, &n1)) {
             odp_put_userspace_action(pid, user_data, user_data_size,
                                      tunnel_out_port, include_actions, actions);
-            return n + n1;
+            res = n + n1;
         } else if (s[n] == ')') {
             odp_put_userspace_action(pid, user_data, user_data_size,
                                      ODPP_NONE, include_actions, actions);
-            return n + 1;
+            res = n + 1;
+        } else {
+            res = -EINVAL;
         }
     }
-
-    return -EINVAL;
+out:
+    ofpbuf_uninit(&buf);
+    return res;
 }
 
 static int
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 55ee97e..8565ab6 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -109,6 +109,7 @@  parse_keys(bool wc_keys)
 
     next:
         ofpbuf_uninit(&odp_key);
+        ofpbuf_uninit(&odp_mask);
     }
     ds_destroy(&in);