The user is required to expose the_idl and the_idl_txn global variables,
so that memory can be cleaned up on fatal errors. This patch changes to
ask user to supply an exit function via ctl_init(). What user needs to
do on exit can now remain private.
Signed-off-by: Andy Zhou <az...@nicira.com>
---
lib/db-ctl-base.c | 24 +++++++++++++-----------
lib/db-ctl-base.h | 11 ++---------
utilities/ovs-vsctl.c | 29 +++++++++++++++++++++++++++--
vtep/vtep-ctl.c | 32 ++++++++++++++++++++++++++++----
4 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index e3ba0c5..a137af6 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -40,12 +40,6 @@
VLOG_DEFINE_THIS_MODULE(db_ctl_base);
-/* The IDL we're using and the current transaction, if any.
- * This is for use by ctl_exit() only, to allow it to clean up.
- * Other code should use its context arguments. */
-struct ovsdb_idl *the_idl;
-struct ovsdb_idl_txn *the_idl_txn;
-
/* This array defines the 'show' command output format. User can check the
* definition in utilities/ovs-vsctl.c as reference.
*
@@ -59,6 +53,14 @@ struct ovsdb_idl_txn *the_idl_txn;
* */
static const struct cmd_show_table *cmd_show_tables;
+/* ctl_exit() is called by ctl_fatal(). User can optionally supply an exit
+ * function ctl_exit_func() via ctl_init. If supplied, this function will
+ * be called by ctl_exit()
+ */
+static void (*ctl_exit_func)(int status) = NULL;
+OVS_NO_RETURN static void ctl_exit(int status);
+
+
/* Represents all tables in the schema. User must define 'tables'
* in implementation and supply via clt_init(). The definition must end
* with an all-NULL entry. */
@@ -1956,11 +1958,9 @@ ctl_fatal(const char *format, ...)
void
ctl_exit(int status)
{
- if (the_idl_txn) {
- ovsdb_idl_txn_abort(the_idl_txn);
- ovsdb_idl_txn_destroy(the_idl_txn);
+ if (ctl_exit_func) {
+ ctl_exit_func(status);
}
- ovsdb_idl_destroy(the_idl);
exit(status);
}
@@ -2007,10 +2007,12 @@ ctl_register_commands(const struct ctl_command_syntax
*commands)
/* Registers the 'db_ctl_commands' to 'all_commands'. */
void
ctl_init(const struct ctl_table_class tables_[],
- const struct cmd_show_table cmd_show_tables_[])
+ const struct cmd_show_table cmd_show_tables_[],
+ void (*ctl_exit_func_)(int status))
{
tables = tables_;
cmd_show_tables = cmd_show_tables_;
+ ctl_exit_func = ctl_exit_func_;
ctl_register_commands(db_ctl_commands);
}
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index e750599..034f29d 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -33,8 +33,6 @@ struct table;
* (structs, commands and functions). To utilize this module, user must
* define the following:
*
- * - the 'the_idl' and 'the_idl_txn'.
- *
* - the command syntaxes for each command. (See 'struct ctl_command_syntax'
* for more info) and regiters them using ctl_register_commands().
*
@@ -46,17 +44,12 @@ struct table;
/* ctl_fatal() also logs the error, so it is preferred in this file. */
#define ovs_fatal please_use_ctl_fatal_instead_of_ovs_fatal
-/* idl and idl transaction to be used for the *ctl command execution.
- * User must provide them. */
-extern struct ovsdb_idl *the_idl;
-extern struct ovsdb_idl_txn *the_idl_txn;
-
struct ctl_table_class;
struct cmd_show_table;
void ctl_init(const struct ctl_table_class *tables,
- const struct cmd_show_table *cmd_show_tables);
+ const struct cmd_show_table *cmd_show_tables,
+ void (*ctl_exit_func)(int status));
char *ctl_default_db(void);
-OVS_NO_RETURN void ctl_exit(int status);
OVS_NO_RETURN void ctl_fatal(const char *, ...) OVS_PRINTF_FORMAT(1, 2);
/* *ctl command syntax structure, to be defined by each command implementation.
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 7794106..ccb9270 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -84,6 +84,14 @@ static bool retry;
static struct table_style table_style = TABLE_STYLE_DEFAULT;
static void vsctl_cmd_init(void);
+
+/* 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;
+OVS_NO_RETURN static void vsctl_exit(int status);
+
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
@@ -1343,7 +1351,7 @@ cmd_br_exists(struct ctl_context *ctx)
vsctl_context_populate_cache(ctx);
if (!find_bridge(vsctl_ctx, ctx->argv[1], false)) {
- ctl_exit(2);
+ vsctl_exit(2);
}
}
@@ -2648,6 +2656,23 @@ try_again:
free(error);
}
+/* 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);
+}
+
/*
* Developers who add new commands to the 'struct ctl_command_syntax' must
* define the 'arguments' member of the struct. The following keywords are
@@ -2749,6 +2774,6 @@ static const struct ctl_command_syntax vsctl_commands[] =
{
static void
vsctl_cmd_init(void)
{
- ctl_init(tables, cmd_show_tables);
+ ctl_init(tables, cmd_show_tables, vsctl_exit);
ctl_register_commands(vsctl_commands);
}
diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index d9c4b26..d3e042e 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -70,6 +70,13 @@ static int timeout;
/* Format for table output. */
static struct table_style table_style = TABLE_STYLE_DEFAULT;
+/* The IDL we're using and the current transaction, if any.
+ * This is for use by vtep_ctl_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;
+
+OVS_NO_RETURN static void vtep_ctl_exit(int status);
static void vtep_ctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
@@ -270,6 +277,23 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
free(options);
}
+/* 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
+vtep_ctl_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
usage(void)
{
@@ -1194,7 +1218,7 @@ cmd_ps_exists(struct ctl_context *ctx)
vtep_ctl_context_populate_cache(ctx);
if (!find_pswitch(vtepctl_ctx, ctx->argv[1], false)) {
- ctl_exit(2);
+ vtep_ctl_exit(2);
}
}
@@ -1368,7 +1392,7 @@ cmd_ls_exists(struct ctl_context *ctx)
vtep_ctl_context_populate_cache(ctx);
if (!find_lswitch(vtepctl_ctx, ctx->argv[1], false)) {
- ctl_exit(2);
+ vtep_ctl_exit(2);
}
}
@@ -2310,6 +2334,6 @@ static const struct ctl_command_syntax vtep_commands[] = {
static void
vtep_ctl_cmd_init(void)
{
- ctl_init(tables, cmd_show_tables);
+ ctl_init(tables, cmd_show_tables, vtep_ctl_exit);
ctl_register_commands(vtep_commands);
}
--
1.9.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
