[ovs-dev] [PATCH ovn] ovn-nbctl: Fix double-free of parsed commands on error path.

[Dumitru Ceara]

Reported-by: Daniel Alvarez <dalvarez at redhat.com>
Fixes: 8fb54e16378c ("ovn-nbctl: Cleanup allocated memory to keep valgrind happy.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
 tests/ovn-nbctl.at    | 12 ++++++++++++
 utilities/ovn-nbctl.c | 17 ++++++-----------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 11091d8..01edfcb 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1976,6 +1976,18 @@ AT_CHECK([ovn-nbctl list forwarding_group], [0], [])
 
 ])
 
+dnl ---------------------------------------------------------------------
+
+OVN_NBCTL_TEST([ovn_nbctl_negative], [basic negative tests], [
+AT_CHECK([ovn-nbctl --id=@ls create logical_switch name=foo -- \
+          set logical_switch foo1 name=bar],
+         [1], [], [dnl
+ovn-nbctl: no row "foo1" in table Logical_Switch
+])
+])
+
+dnl ---------------------------------------------------------------------
+
 AT_SETUP([ovn-nbctl - daemon retry connection])
 OVN_NBCTL_TEST_START daemon
 AT_CHECK([kill `cat ovsdb-server.pid`])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index a050f8d..d19e1b6 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -6430,12 +6430,6 @@ out_error:
     the_idl_txn = NULL;
 
     ovsdb_symbol_table_destroy(symtab);
-    for (c = commands; c < &commands[n_commands]; c++) {
-        ds_destroy(&c->output);
-        table_destroy(c->table);
-        free(c->table);
-    }
-
     return error;
 }
 
@@ -6851,17 +6845,18 @@ server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_,
         } else {
             ds_put_cstr(&output, ds_cstr_ro(&c->output));
         }
-
-        ds_destroy(&c->output);
-        table_destroy(c->table);
-        free(c->table);
     }
     unixctl_command_reply(conn, ds_cstr_ro(&output));
     ds_destroy(&output);
 
 out:
     free(error);
-    for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) {
+
+    struct ctl_command *c;
+    for (c = commands; c < &commands[n_commands]; c++) {
+        ds_destroy(&c->output);
+        table_destroy(c->table);
+        free(c->table);
         shash_destroy_free_data(&c->options);
     }
     free(commands);
-- 
1.8.3.1