Re: [ovs-dev] [PATCH] ovsdb: Fix memory leak in execute_update.

[nickcooper-zhangtonghao]

Hi William,
I reviewed your patch codes and found other bugs.

If the ‘ovsdb_condition_from_json’ return the error, cnd->clauses will be
set NULL, so ‘ovsdb_condition_destroy’ should check the 'cnd->clauses’ before
free it.

It is applied to ‘ovsdb_row_from_json’.


Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtong...@opencloud.tech>
---
 ovsdb/column.c      | 5 ++++-
 ovsdb/condition.c   | 6 +++++-
 ovsdb/replication.c | 3 +++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/ovsdb/column.c b/ovsdb/column.c
index 8838df3..f01301f 100644
--- a/ovsdb/column.c
+++ b/ovsdb/column.c
@@ -129,7 +129,10 @@ ovsdb_column_set_init(struct ovsdb_column_set *set)
 void
 ovsdb_column_set_destroy(struct ovsdb_column_set *set)
 {
-    free(set->columns);
+    if (set->columns) {
+        free(set->columns);
+        set->columns = NULL;
+    }
 }

 void
diff --git a/ovsdb/condition.c b/ovsdb/condition.c
index 6da3b08..f337a37 100644
--- a/ovsdb/condition.c
+++ b/ovsdb/condition.c
@@ -485,7 +485,11 @@ ovsdb_condition_destroy(struct ovsdb_condition *cnd)
     for (i = 0; i < cnd->n_clauses; i++) {
         ovsdb_clause_free(&cnd->clauses[i]);
     }
-    free(cnd->clauses);
+
+    if (cnd->clauses) {
+        free(cnd->clauses);
+        cnd->clauses = NULL;
+    }
     cnd->n_clauses = 0;

     ovsdb_condition_optimize_destroy(cnd);
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 52b7085..bfd2ca1 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -568,6 +568,8 @@ execute_delete(struct ovsdb_txn *txn, const char *uuid,
     }

     ovsdb_condition_destroy(&condition);
+    json_destroy(CONST_CAST(struct json *, where));
+
     return error;
 }

@@ -625,6 +627,7 @@ execute_update(struct ovsdb_txn *txn, const char *uuid,
     ovsdb_row_destroy(row);
     ovsdb_column_set_destroy(&columns);
     ovsdb_condition_destroy(&condition);
+    json_destroy(CONST_CAST(struct json *, where));

     return error;
 }
--
1.8.3.1


> From: William Tu <u9012...@gmail.com>
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH] ovsdb: Fix memory leak in execute_update.
> Message-ID: <1469582910-64371-1-git-send-email-u9012...@gmail.com>
> 
> Valgrind testcase 1804 ovsdb-server.at:1023 insert rows, update rows by value
> reports the following leak.
>    json_from_string (json.c:1025)
>    execute_update (replication.c:614), similarily at execute_delete()
>    process_table_update (replication.c:502)
>    process_notification.part.5 (replication.c:445)
>    process_notification (replication.c:402)
>    check_for_notifications (replication.c:418)
>    replication_run (replication.c:110)
> 
> Signed-off-by: William Tu <u9012...@gmail.com>
> ---
> ovsdb/replication.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
> index af7ae5c..fe89d39 100644
> --- a/ovsdb/replication.c
> +++ b/ovsdb/replication.c
> @@ -573,6 +573,8 @@ execute_delete(struct ovsdb_txn *txn, const char *uuid,
>     }
> 
>     ovsdb_condition_destroy(&condition);
> +    json_destroy(CONST_CAST(struct json *, where));
> +
>     return error;
> }
> 
> @@ -630,6 +632,7 @@ execute_update(struct ovsdb_txn *txn, const char *uuid,
>     ovsdb_row_destroy(row);
>     ovsdb_column_set_destroy(&columns);
>     ovsdb_condition_destroy(&condition);
> +    json_destroy(CONST_CAST(struct json *, where));
> 
>     return error;
> }
> -- 
> 2.5.0



_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev