diff mbox

[ovs-dev] ovsdb-server: Fix memory leak reported by Valgind.

Message ID 1463160787-82530-1-git-send-email-u9012063@gmail.com
State Superseded
Headers show

Commit Message

William Tu May 13, 2016, 5:33 p.m. UTC
Reported by test 1657: ovsdb-server/add-db and remove-db.
  ds_put_format (dynamic-string.c:142)
  query_db_remotes (ovsdb-server.c:798)
  reconfigure_remotes (ovsdb-server.c:988)
  main_loop (ovsdb-server.c:156)

Signed-off-by: William Tu <u9012063@gmail.com>
---
 ovsdb/ovsdb-server.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Ryan Moats May 14, 2016, 2:13 a.m. UTC | #1
"dev" <dev-bounces@openvswitch.org> wrote on 05/13/2016 12:33:07 PM:

> From: William Tu <u9012063@gmail.com>
> To: dev@openvswitch.org
> Date: 05/13/2016 12:33 PM
> Subject: [ovs-dev] [PATCH] ovsdb-server: Fix memory leak reported by
Valgind.
> Sent by: "dev" <dev-bounces@openvswitch.org>
>
> Reported by test 1657: ovsdb-server/add-db and remove-db.
>   ds_put_format (dynamic-string.c:142)
>   query_db_remotes (ovsdb-server.c:798)
>   reconfigure_remotes (ovsdb-server.c:988)
>   main_loop (ovsdb-server.c:156)
>
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  ovsdb/ovsdb-server.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 4e2c8d0..0ec3053 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -196,6 +196,7 @@ main_loop(struct ovsdb_jsonrpc_server *jsonrpc,
> struct shash *all_dbs,
>          }
>      }
>
> +    free(remotes_error);

Shouldn't this be

    if (remotes_error) {
        free(remotes_error);
    }

Ryan Moats
William Tu May 14, 2016, 5:19 a.m. UTC | #2
Hi Ryan,

Thanks for the comments.
I think free() in libc internally will check if remotes_error is NULL or
not.

Regards,
William

> >      }
> >
> > +    free(remotes_error);
>
> Shouldn't this be
>
>     if (remotes_error) {
>         free(remotes_error);
>     }
>
> Ryan Moats
>
Ben Pfaff May 15, 2016, 4:38 a.m. UTC | #3
On Fri, May 13, 2016 at 09:13:13PM -0500, Ryan Moats wrote:
> > +    free(remotes_error);
> 
> Shouldn't this be
> 
>     if (remotes_error) {
>         free(remotes_error);
>     }

No.  free() is a no-op when passed a NULL pointer.
Ben Pfaff May 15, 2016, 4:40 a.m. UTC | #4
On Fri, May 13, 2016 at 10:33:07AM -0700, William Tu wrote:
> Reported by test 1657: ovsdb-server/add-db and remove-db.
>   ds_put_format (dynamic-string.c:142)
>   query_db_remotes (ovsdb-server.c:798)
>   reconfigure_remotes (ovsdb-server.c:988)
>   main_loop (ovsdb-server.c:156)
> 
> Signed-off-by: William Tu <u9012063@gmail.com>

Applied, thanks!
diff mbox

Patch

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 4e2c8d0..0ec3053 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -196,6 +196,7 @@  main_loop(struct ovsdb_jsonrpc_server *jsonrpc, struct shash *all_dbs,
         }
     }
 
+    free(remotes_error);
 }
 
 int