diff mbox

[ovs-dev,3/3] ovsdb: Fix replication memory leak.

Message ID 20160909204853.27839-4-joe@ovn.org
State Accepted
Headers show

Commit Message

Joe Stringer Sept. 9, 2016, 8:48 p.m. UTC 
Valgrind reports:

==18725== 32 bytes in 1 blocks are definitely lost in loss record 339 of 497
==18725==    at 0x4C29BBE: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18725==    by 0x450F1F: xmalloc (util.c:112)
==18725==    by 0x41748E: replication_add_local_db (replication.c:137)
==18725==    by 0x40803B: ovsdb_replication_init (ovsdb-server.c:146)
==18725==    by 0x407C9E: ovsdb_server_connect_active_ovsdb_server
(ovsdb-server.c:1165)
==18725==    by 0x450AB3: process_command (unixctl.c:313)
==18725==    by 0x4500DC: run_connection (unixctl.c:347)
==18725==    by 0x44FFB6: unixctl_server_run (unixctl.c:400)
==18725==    by 0x4081AC: main_loop (ovsdb-server.c:182)
==18725==    by 0x406432: main (ovsdb-server.c:429)

Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements")
Signed-off-by: Joe Stringer <joe@ovn.org>
---
This also affects branch-2.6.
---
 ovsdb/replication.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Flaviof Sept. 10, 2016, 3:47 a.m. UTC | #1 
> On Sep 9, 2016, at 4:48 PM, Joe Stringer <joe@ovn.org> wrote:
> 
> Valgrind reports:
> 
> ==18725== 32 bytes in 1 blocks are definitely lost in loss record 339 of 497
> ==18725==    at 0x4C29BBE: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==18725==    by 0x450F1F: xmalloc (util.c:112)
> ==18725==    by 0x41748E: replication_add_local_db (replication.c:137)
> ==18725==    by 0x40803B: ovsdb_replication_init (ovsdb-server.c:146)
> ==18725==    by 0x407C9E: ovsdb_server_connect_active_ovsdb_server
> (ovsdb-server.c:1165)
> ==18725==    by 0x450AB3: process_command (unixctl.c:313)
> ==18725==    by 0x4500DC: run_connection (unixctl.c:347)
> ==18725==    by 0x44FFB6: unixctl_server_run (unixctl.c:400)
> ==18725==    by 0x4081AC: main_loop (ovsdb-server.c:182)
> ==18725==    by 0x406432: main (ovsdb-server.c:429)
> 
> Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements")
> Signed-off-by: Joe Stringer <joe@ovn.org>

Acked-by: Flavio Fernandes <flavio@flaviof.com>

> ---
> This also affects branch-2.6.
> ---
> ovsdb/replication.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
> index c2f9dfb3a2cd..50abf001f48a 100644
> --- a/ovsdb/replication.c
> +++ b/ovsdb/replication.c
> @@ -134,7 +134,6 @@ replication_init(const char *sync_from_, const char *exclude_tables)
> void
> replication_add_local_db(const char *database, struct ovsdb *db)
> {
> -    struct shash_node *node = xmalloc(sizeof *node);
>     shash_add_assert(&local_dbs, database, db);
> }
> 
> --
> 2.9.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Ben Pfaff Sept. 11, 2016, 4:26 a.m. UTC | #2 
On Fri, Sep 09, 2016 at 01:48:53PM -0700, Joe Stringer wrote:
> Valgrind reports:
> 
> ==18725== 32 bytes in 1 blocks are definitely lost in loss record 339 of 497
> ==18725==    at 0x4C29BBE: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==18725==    by 0x450F1F: xmalloc (util.c:112)
> ==18725==    by 0x41748E: replication_add_local_db (replication.c:137)
> ==18725==    by 0x40803B: ovsdb_replication_init (ovsdb-server.c:146)
> ==18725==    by 0x407C9E: ovsdb_server_connect_active_ovsdb_server
> (ovsdb-server.c:1165)
> ==18725==    by 0x450AB3: process_command (unixctl.c:313)
> ==18725==    by 0x4500DC: run_connection (unixctl.c:347)
> ==18725==    by 0x44FFB6: unixctl_server_run (unixctl.c:400)
> ==18725==    by 0x4081AC: main_loop (ovsdb-server.c:182)
> ==18725==    by 0x406432: main (ovsdb-server.c:429)
> 
> Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements")
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> This also affects branch-2.6.

I'm surprised that neither Clang nor GCC gives a "value assigned is
never used" warning for this code.

Acked-by: Ben Pfaff <blp@ovn.org>
Joe Stringer Sept. 12, 2016, 6:06 p.m. UTC | #3 
On 10 September 2016 at 21:26, Ben Pfaff <blp@ovn.org> wrote:
> On Fri, Sep 09, 2016 at 01:48:53PM -0700, Joe Stringer wrote:
>> Valgrind reports:
>>
>> ==18725== 32 bytes in 1 blocks are definitely lost in loss record 339 of 497
>> ==18725==    at 0x4C29BBE: malloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==18725==    by 0x450F1F: xmalloc (util.c:112)
>> ==18725==    by 0x41748E: replication_add_local_db (replication.c:137)
>> ==18725==    by 0x40803B: ovsdb_replication_init (ovsdb-server.c:146)
>> ==18725==    by 0x407C9E: ovsdb_server_connect_active_ovsdb_server
>> (ovsdb-server.c:1165)
>> ==18725==    by 0x450AB3: process_command (unixctl.c:313)
>> ==18725==    by 0x4500DC: run_connection (unixctl.c:347)
>> ==18725==    by 0x44FFB6: unixctl_server_run (unixctl.c:400)
>> ==18725==    by 0x4081AC: main_loop (ovsdb-server.c:182)
>> ==18725==    by 0x406432: main (ovsdb-server.c:429)
>>
>> Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements")
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>> This also affects branch-2.6.
>
> I'm surprised that neither Clang nor GCC gives a "value assigned is
> never used" warning for this code.

Me too.

> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks, applied to master and branch-2.6.
diff mbox

Patch

diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index c2f9dfb3a2cd..50abf001f48a 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -134,7 +134,6 @@  replication_init(const char *sync_from_, const char *exclude_tables)
 void
 replication_add_local_db(const char *database, struct ovsdb *db)
 {
-    struct shash_node *node = xmalloc(sizeof *node);
     shash_add_assert(&local_dbs, database, db);
 }