diff mbox

[ovs-dev,3/4] ovs-ofctl: fix memory leak reported by valgrind, case 376

Message ID 1450981722-101437-3-git-send-email-u9012063@gmail.com
State Changes Requested
Headers show

Commit Message

William Tu Dec. 24, 2015, 6:28 p.m. UTC 
Test case 376 reports two leaks:
    ofpbuf_new(ofpbuf.c:151)
    ofpraw_alloc_xid(ofp-msgs.c:533)
    ofpraw_alloc(ofp-msgs.c:525)
    ofputil_encode_flow_mod(ofp-util.c:2290)
    bundle_flow_mod__(ovs-ofctl.c:1312)
    ofctl_flow_mod__(ovs-ofctl.c:1331)
    ofctl_flow_mod_file.isra.13(ovs-ofctl.c:1365)
    ovs_cmdl_run_command(command-line.c:121)
    main(ovs-ofctl.c:135)
and
    ofpraw_alloc(ofp-msgs.c:525)
    ofputil_encode_flow_mod(ofp-util.c:2290)
    fte_make_flow_mod(ovs-ofctl.c:2936)
    ofctl_replace_flows(ovs-ofctl.c:2981)
    ovs_cmdl_run_command(command-line.c:121)
    main(ovs-ofctl.c:135)

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>
---
 utilities/ovs-ofctl.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Ben Pfaff Jan. 4, 2016, 8:39 p.m. UTC | #1 
On Thu, Dec 24, 2015 at 10:28:41AM -0800, William Tu wrote:
> Test case 376 reports two leaks:
>     ofpbuf_new(ofpbuf.c:151)
>     ofpraw_alloc_xid(ofp-msgs.c:533)
>     ofpraw_alloc(ofp-msgs.c:525)
>     ofputil_encode_flow_mod(ofp-util.c:2290)
>     bundle_flow_mod__(ovs-ofctl.c:1312)
>     ofctl_flow_mod__(ovs-ofctl.c:1331)
>     ofctl_flow_mod_file.isra.13(ovs-ofctl.c:1365)
>     ovs_cmdl_run_command(command-line.c:121)
>     main(ovs-ofctl.c:135)
> and
>     ofpraw_alloc(ofp-msgs.c:525)
>     ofputil_encode_flow_mod(ofp-util.c:2290)
>     fte_make_flow_mod(ovs-ofctl.c:2936)
>     ofctl_replace_flows(ovs-ofctl.c:2981)
>     ovs_cmdl_run_command(command-line.c:121)
>     main(ovs-ofctl.c:135)
> 
> 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>

The change looks correct but please change it to use
ofpbuf_list_delete() instead of explicit loops.
diff mbox

Patch

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 0d57f85..f148203 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1297,6 +1297,7 @@  bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
                   size_t n_fms, enum ofputil_protocol usable_protocols)
 {
     enum ofputil_protocol protocol;
+    struct ofpbuf *request, *request_next;
     struct vconn *vconn;
     struct ovs_list requests;
     size_t i;
@@ -1309,13 +1310,18 @@  bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
 
     for (i = 0; i < n_fms; i++) {
         struct ofputil_flow_mod *fm = &fms[i];
-        struct ofpbuf *request = ofputil_encode_flow_mod(fm, protocol);
+        request = ofputil_encode_flow_mod(fm, protocol);
 
         list_push_back(&requests, &request->list_node);
         free(CONST_CAST(struct ofpact *, fm->ofpacts));
     }
 
     bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
+
+    LIST_FOR_EACH_SAFE (request, request_next, list_node, &requests) {
+        ofpbuf_delete(request);
+    }
+
     vconn_close(vconn);
 }
 
@@ -2936,6 +2942,7 @@  ofctl_replace_flows(struct ovs_cmdl_context *ctx)
 {
     enum { FILE_IDX = 0, SWITCH_IDX = 1 };
     enum ofputil_protocol usable_protocols, protocol;
+    struct ofpbuf *request, *request_next;
     struct flow_tables tables;
     struct classifier *cls;
     struct ovs_list requests;
@@ -2982,6 +2989,11 @@  ofctl_replace_flows(struct ovs_cmdl_context *ctx)
     } else {
         transact_multiple_noreply(vconn, &requests);
     }
+
+    LIST_FOR_EACH_SAFE (request, request_next, list_node, &requests) {
+        ofpbuf_delete(request);
+    }
+
     vconn_close(vconn);
 
     fte_free_all(&tables);