[ovs-dev,1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
diff mbox series

Message ID 1570435834-17079-1-git-send-email-damjan.skvarc@gmail.com
State New
Headers show
Series
  • [ovs-dev,1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Related show

Commit Message

Damijan Skvarc Oct. 7, 2019, 8:10 a.m. UTC
memory leak is reported by valgrind while executing functional test
"ovsdb-tool convert-to-standalone"

==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are definitely lost in loss record 20 of 20
==13842==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13842==    by 0x45EE2E: xmalloc (util.c:138)
==13842==    by 0x43E386: json_create (json.c:1451)
==13842==    by 0x43BDD2: json_object_create (json.c:254)
==13842==    by 0x43DEE3: json_parser_push_object (json.c:1273)
==13842==    by 0x43E167: json_parser_input (json.c:1371)
==13842==    by 0x43D6EA: json_lex_input (json.c:991)
==13842==    by 0x43DAC1: json_parser_feed (json.c:1149)
==13842==    by 0x40D108: parse_body (log.c:411)
==13842==    by 0x40D386: ovsdb_log_read (log.c:476)
==13842==    by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571)
==13842==    by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606)
==13842==    by 0x438670: ovs_cmdl_run_command__ (command-line.c:223)
==13842==    by 0x438720: ovs_cmdl_run_command (command-line.c:254)
==13842==    by 0x405A4C: main (ovsdb-tool.c:79)

The problem was in do_convert_to_standalone() function which while reading log file
allocate json object which was not deallocated at the end.

Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
---
 ovsdb/ovsdb-tool.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

Comments

Ben Pfaff Oct. 7, 2019, 6:21 p.m. UTC | #1
On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote:
> memory leak is reported by valgrind while executing functional test
> "ovsdb-tool convert-to-standalone"
> 
> ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are definitely lost in loss record 20 of 20
> ==13842==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==13842==    by 0x45EE2E: xmalloc (util.c:138)
> ==13842==    by 0x43E386: json_create (json.c:1451)
> ==13842==    by 0x43BDD2: json_object_create (json.c:254)
> ==13842==    by 0x43DEE3: json_parser_push_object (json.c:1273)
> ==13842==    by 0x43E167: json_parser_input (json.c:1371)
> ==13842==    by 0x43D6EA: json_lex_input (json.c:991)
> ==13842==    by 0x43DAC1: json_parser_feed (json.c:1149)
> ==13842==    by 0x40D108: parse_body (log.c:411)
> ==13842==    by 0x40D386: ovsdb_log_read (log.c:476)
> ==13842==    by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571)
> ==13842==    by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606)
> ==13842==    by 0x438670: ovs_cmdl_run_command__ (command-line.c:223)
> ==13842==    by 0x438720: ovs_cmdl_run_command (command-line.c:254)
> ==13842==    by 0x405A4C: main (ovsdb-tool.c:79)
> 
> The problem was in do_convert_to_standalone() function which while reading log file
> allocate json object which was not deallocated at the end.
> 
> Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>

Applied to master, thanks.

Patch
diff mbox series

diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 3bbf4c8..a8f3135 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -951,26 +951,30 @@  raft_header_to_standalone_log(const struct raft_header *h,
 {
     if (h->snap_index) {
         if (!h->snap.data || json_array(h->snap.data)->n != 2) {
-        ovs_fatal(0, "Incorrect raft header data array length");
+            ovs_fatal(0, "Incorrect raft header data array length");
         }
 
-        struct json *schema_json = json_array(h->snap.data)->elems[0];
+        struct json_array *pa = json_array(h->snap.data);
+        struct json *schema_json = pa->elems[0];
+        struct ovsdb_error *error = NULL;
+
         if (schema_json->type != JSON_NULL) {
             struct ovsdb_schema *schema;
             check_ovsdb_error(ovsdb_schema_from_json(schema_json, &schema));
             ovsdb_schema_destroy(schema);
-            check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
-                                                       schema_json));
+            error = ovsdb_log_write(db_log_data, schema_json);
         }
 
-        struct json *data_json = json_array(h->snap.data)->elems[1];
-        if (!data_json || data_json->type != JSON_OBJECT) {
-            ovs_fatal(0, "Invalid raft header data");
-        }
-        if (data_json->type != JSON_NULL) {
-            check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
-                                                       data_json));
+        if (!error) {
+            struct json *data_json = pa->elems[1];
+            if (!data_json || data_json->type != JSON_OBJECT) {
+                ovs_fatal(0, "Invalid raft header data");
+            }
+            if (data_json->type != JSON_NULL) {
+                error = ovsdb_log_write(db_log_data, data_json);
+            }
         }
+        check_ovsdb_error(error);
     }
 }
 
@@ -982,14 +986,14 @@  raft_record_to_standalone_log(const struct raft_record *r,
         if (!r->entry.data) {
             return;
         }
-        if (json_array(r->entry.data)->n != 2) {
+        struct json_array *pa = json_array(r->entry.data);
+
+        if (pa->n != 2) {
             ovs_fatal(0, "Incorrect raft record array length");
         }
-
-        struct json *data_json = json_array(r->entry.data)->elems[1];
+        struct json *data_json = pa->elems[1];
         if (data_json->type != JSON_NULL) {
-            check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
-                                                       data_json));
+            check_ovsdb_error(ovsdb_log_write(db_log_data, data_json));
         }
     }
 }
@@ -1584,6 +1588,7 @@  do_convert_to_standalone(struct ovsdb_log *log, struct ovsdb_log *db_log_data)
             raft_record_to_standalone_log(&r, db_log_data);
             raft_record_uninit(&r);
         }
+        json_destroy(json);
     }
 }