ovs-vsctl: Fix memory leaks.

[openvswitch] / utilities / ovs-vsctl.c

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index cf550c5211358e0e8a02dd505ec1e459b7779a6a..f8863bfc1238990838b39cc58a5622e6829e79a6 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -90,6 +90,13 @@ static int timeout = 5;
 /* All supported commands. */
 static const struct vsctl_command_syntax all_commands[];
 
+/* The IDL we're using and the current transaction, if any.
+ * This is for use by vsctl_exit() only, to allow it to clean up.
+ * Other code should use its context arguments. */
+static struct ovsdb_idl *the_idl;
+static struct ovsdb_idl_txn *the_idl_txn;
+
+static void vsctl_exit(int status) NO_RETURN;
 static void vsctl_fatal(const char *, ...) PRINTF_FORMAT(1, 2) NO_RETURN;
 static char *default_db(void);
 static void usage(void) NO_RETURN;
@@ -134,7 +141,7 @@ main(int argc, char *argv[])
     /* Do basic command syntax checking. */
 
     /* Now execute the commands. */
-    idl = ovsdb_idl_create(db, &ovsrec_idl_class);
+    idl = the_idl = ovsdb_idl_create(db, &ovsrec_idl_class);
     seqno = ovsdb_idl_get_seqno(idl);
     trials = 0;
     for (;;) {
@@ -339,7 +346,24 @@ vsctl_fatal(const char *format, ...)
     vlog_set_levels(VLM_vsctl, VLF_CONSOLE, VLL_EMER);
     VLOG_ERR("%s", message);
     ovs_error(0, "%s", message);
-    exit(EXIT_FAILURE);
+    vsctl_exit(EXIT_FAILURE);
+}
+
+/* Frees the current transaction and the underlying IDL and then calls
+ * exit(status).
+ *
+ * Freeing the transaction and the IDL is not strictly necessary, but it makes
+ * for a clean memory leak report from valgrind in the normal case.  That makes
+ * it easier to notice real memory leaks. */
+static void
+vsctl_exit(int status)
+{
+    if (the_idl_txn) {
+        ovsdb_idl_txn_abort(the_idl_txn);
+        ovsdb_idl_txn_destroy(the_idl_txn);
+    }
+    ovsdb_idl_destroy(the_idl);
+    exit(status);
 }
 
 static void
@@ -925,7 +949,7 @@ cmd_br_exists(struct vsctl_context *ctx)
 
     get_info(ctx->ovs, &info);
     if (!find_bridge(&info, ctx->argv[1], false)) {
-        exit(2);
+        vsctl_exit(2);
     }
     free_info(&info);
 }
@@ -1875,10 +1899,10 @@ parse_column_key_value(const char *arg, const struct vsctl_table_class *table,
             goto error;
         }
         error = get_column(table, column_name, columnp);
+        free(column_name);
         if (error) {
             goto error;
         }
-        free(column_name);
     }
 
     /* Parse key string. */
@@ -2169,6 +2193,9 @@ set_column(const struct vsctl_table_class *table,
         ovsdb_datum_init_empty(&new);
         ovsdb_datum_add_unsafe(&new, &key, &value, &column->idl->type);
 
+        ovsdb_atom_destroy(&key, column->idl->type.key_type);
+        ovsdb_atom_destroy(&value, column->idl->type.value_type);
+
         ovsdb_idl_txn_read(row, column->idl, &old);
         ovsdb_datum_union(&old, &new, &column->idl->type, true);
         ovsdb_idl_txn_write(row, column->idl, &old);
@@ -2187,6 +2214,7 @@ set_column(const struct vsctl_table_class *table,
     }
 
     free(key_string);
+    free(value_string);
 }
 
 static void
@@ -2452,7 +2480,7 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands,
     int64_t next_cfg = 0;
     char *comment;
 
-    txn = ovsdb_idl_txn_create(idl);
+    txn = the_idl_txn = ovsdb_idl_txn_create(idl);
     if (dry_run) {
         ovsdb_idl_txn_set_dry_run(txn);
     }
@@ -2501,6 +2529,7 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands,
         }
     }
     ovsdb_idl_txn_destroy(txn);
+    the_idl_txn = NULL;
 
     switch (status) {
     case TXN_INCOMPLETE: