diff mbox

[ovs-dev,2/3] rstp: fix memory leak reported by valgrind

Message ID 1452029923-8685-2-git-send-email-u9012063@gmail.com
State Accepted
Headers show

Commit Message

William Tu Jan. 5, 2016, 9:38 p.m. UTC 
test case: 1650 RSTP Single bridge, call stacks
    hmap_insert_at (hmap.h:235)
    rstp_port_set_port_number__ (rstp.c:744)
    rstp_add_port (rstp.c:1164)
    new_bridge (test-rstp.c:123)
    test_rstp_main (test-rstp.c:514)
    ovstest_wrapper_test_rstp_main__ (test-rstp.c:714)
    ovs_cmdl_run_command (command-line.c:121)
    main (ovstest.c:132)
fix it by adding hmap_destroy() at rstp_unref()

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/rstp.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniele Venturino Jan. 7, 2016, 12:12 p.m. UTC | #1 
Maybe we could destroy the hmap with the mutex still locked.
Anyway, looks good to me.

Acked-by: Daniele Venturino <daniele.venturino@m3s.it <mailto:daniele.venturino@m3s.it>>

> Il giorno 05 gen 2016, alle ore 22:38, William Tu <u9012063@gmail.com> ha scritto:
> 
> test case: 1650 RSTP Single bridge, call stacks
>    hmap_insert_at (hmap.h:235)
>    rstp_port_set_port_number__ (rstp.c:744)
>    rstp_add_port (rstp.c:1164)
>    new_bridge (test-rstp.c:123)
>    test_rstp_main (test-rstp.c:514)
>    ovstest_wrapper_test_rstp_main__ (test-rstp.c:714)
>    ovs_cmdl_run_command (command-line.c:121)
>    main (ovstest.c:132)
> fix it by adding hmap_destroy() at rstp_unref()
> 
> 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/rstp.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/lib/rstp.c b/lib/rstp.c
> index 9fdf810..f24ca01 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -186,6 +186,7 @@ rstp_unref(struct rstp *rstp)
> 
>         list_remove(&rstp->node);
>         ovs_mutex_unlock(&rstp_mutex);
> +        hmap_destroy(&rstp->ports);
>         free(rstp->name);
>         free(rstp);
>     }
> -- 
> 2.5.0
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Ben Pfaff Jan. 11, 2016, 5:18 p.m. UTC | #2 
On Tue, Jan 05, 2016 at 01:38:42PM -0800, William Tu wrote:
> test case: 1650 RSTP Single bridge, call stacks
>     hmap_insert_at (hmap.h:235)
>     rstp_port_set_port_number__ (rstp.c:744)
>     rstp_add_port (rstp.c:1164)
>     new_bridge (test-rstp.c:123)
>     test_rstp_main (test-rstp.c:514)
>     ovstest_wrapper_test_rstp_main__ (test-rstp.c:714)
>     ovs_cmdl_run_command (command-line.c:121)
>     main (ovstest.c:132)
> fix it by adding hmap_destroy() at rstp_unref()
> 
> 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>

Thanks!  Applied to master, branch-2.5, branch-2.4.
diff mbox

Patch

diff --git a/lib/rstp.c b/lib/rstp.c
index 9fdf810..f24ca01 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -186,6 +186,7 @@  rstp_unref(struct rstp *rstp)
 
         list_remove(&rstp->node);
         ovs_mutex_unlock(&rstp_mutex);
+        hmap_destroy(&rstp->ports);
         free(rstp->name);
         free(rstp);
     }