[ovs-dev] [PATCH 1/5] tests: fix memory leak reported by valgrind
William Tu
u9012063 at gmail.com
Fri Dec 11 01:58:12 UTC 2015
Fix some of the "definitely loss" cases reported by Valgrind.
Signed-off-by: William Tu <u9012063 at gmail.com>
Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
Co-authored-by: Daniele Di Proietto <diproiettod at vmware.com>
---
lib/db-ctl-base.c | 1 +
lib/odp-util.c | 21 +++++++++++++--------
lib/ofp-parse.c | 5 ++++-
lib/ofp-print.c | 2 +-
lib/rstp.c | 1 +
lib/tnl-ports.c | 16 ++++++++++------
lib/tun-metadata.c | 1 +
ovn/lib/expr.c | 7 +++++--
ovsdb/ovsdb-client.c | 19 ++++++++++++++++---
tests/test-aa.c | 10 +++++++---
tests/test-netflow.c | 3 +++
tests/test-odp.c | 1 +
tests/test-sflow.c | 6 +++++-
utilities/ovs-appctl.c | 15 ++++++++++++---
utilities/ovs-ofctl.c | 14 +++++++++++++-
utilities/ovs-vsctl.c | 4 ++++
vtep/vtep-ctl.c | 1 +
17 files changed, 98 insertions(+), 29 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index cf49ac0..c64c96e 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1905,6 +1905,7 @@ ctl_parse_commands(int argc, char *argv[], struct shash *local_options,
}
}
if (!n_commands) {
+ free(commands);
ctl_fatal("missing command name (use --help for help)");
}
*n_commandsp = n_commands;
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 90942c7..335c29c 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -909,11 +909,14 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
void *user_data = NULL;
size_t user_data_size = 0;
bool include_actions = false;
+ int res;
if (!ovs_scan(s, "userspace(pid=%"SCNi32"%n", &pid, &n)) {
return -EINVAL;
}
+ ofpbuf_init(&buf, 16);
+
{
uint32_t output;
uint32_t probability;
@@ -940,8 +943,6 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
user_data_size = sizeof cookie.sflow;
} else if (ovs_scan(&s[n], ",slow_path(%n",
&n1)) {
- int res;
-
n += n1;
cookie.type = USER_ACTION_COOKIE_SLOW_PATH;
cookie.slow_path.unused = 0;
@@ -951,7 +952,7 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
&cookie.slow_path.reason,
SLOW_PATH_REASON_MASK, NULL);
if (res < 0 || s[n + res] != ')') {
- return res;
+ goto out;
}
n += res + 1;
@@ -984,10 +985,10 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
char *end;
n += n1;
- ofpbuf_init(&buf, 16);
end = ofpbuf_put_hex(&buf, &s[n], NULL);
if (end[0] != ')') {
- return -EINVAL;
+ res = -EINVAL;
+ goto out;
}
user_data = buf.data;
user_data_size = buf.size;
@@ -1009,15 +1010,19 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
&tunnel_out_port, &n1)) {
odp_put_userspace_action(pid, user_data, user_data_size,
tunnel_out_port, include_actions, actions);
- return n + n1;
+ res = n + n1;
} else if (s[n] == ')') {
odp_put_userspace_action(pid, user_data, user_data_size,
ODPP_NONE, include_actions, actions);
- return n + 1;
+ res = n + 1;
+ } else {
+ res = -EINVAL;
}
}
- return -EINVAL;
+out:
+ ofpbuf_uninit(&buf);
+ return res;
}
static int
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 10aacbc..0ff1468 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1045,6 +1045,7 @@ parse_ofp_flow_mod_file(const char *file_name, int command,
error = parse_ofp_flow_mod_str(&(*fms)[*n_fms], ds_cstr(&s), command,
&usable);
if (error) {
+ char *return_error;
size_t i;
for (i = 0; i < *n_fms; i++) {
@@ -1059,7 +1060,9 @@ parse_ofp_flow_mod_file(const char *file_name, int command,
fclose(stream);
}
- return xasprintf("%s:%d: %s", file_name, line_number, error);
+ return_error = xasprintf("%s:%d: %s", file_name, line_number, error);
+ free(error);
+ return return_error;
}
*usable_protocols &= usable; /* Each line can narrow the set. */
*n_fms += 1;
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index d4f1972..9a52943 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -3026,7 +3026,7 @@ ofp_print_bundle_add(struct ds *s, const struct ofp_header *oh, int verbosity)
ds_put_char(s, '\n');
msg = ofp_to_string(badd.msg, ntohs(badd.msg->length), verbosity);
if (msg) {
- ds_put_cstr(s, msg);
+ ds_put_and_free_cstr(s, msg);
}
}
diff --git a/lib/rstp.c b/lib/rstp.c
index c1d5e7e..f1487cb 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -185,6 +185,7 @@ rstp_unref(struct rstp *rstp)
list_remove(&rstp->node);
ovs_mutex_unlock(&rstp_mutex);
+ hmap_destroy(&rstp->ports);
free(rstp->name);
free(rstp);
}
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 3006a8b..f28ad05 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -353,8 +353,7 @@ insert_ipdev(const char dev_name[])
error = netdev_get_flags(dev, &flags);
if (error || (flags & NETDEV_LOOPBACK)) {
- netdev_close(dev);
- return;
+ goto out_close;
}
ip_dev = xzalloc(sizeof *ip_dev);
@@ -362,19 +361,24 @@ insert_ipdev(const char dev_name[])
ip_dev->change_seq = netdev_get_change_seq(dev);
error = netdev_get_etheraddr(ip_dev->dev, &ip_dev->mac);
if (error) {
- free(ip_dev);
- return;
+ goto out_free;
}
error4 = netdev_get_in4(ip_dev->dev, (struct in_addr *)&ip_dev->addr4, NULL);
error6 = netdev_get_in6(ip_dev->dev, &ip_dev->addr6);
if (error4 && error6) {
- free(ip_dev);
- return;
+ goto out_free;
}
ovs_strlcpy(ip_dev->dev_name, netdev_get_name(dev), sizeof ip_dev->dev_name);
list_insert(&addr_list, &ip_dev->node);
map_insert_ipdev(ip_dev);
+
+ return;
+
+out_free:
+ free(ip_dev);
+out_close:
+ netdev_close(dev);
}
static void
diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
index 234c72c..aa5b48d 100644
--- a/lib/tun-metadata.c
+++ b/lib/tun-metadata.c
@@ -596,6 +596,7 @@ tun_metadata_add_entry(struct tun_table *map, uint8_t idx, uint16_t opt_class,
err = tun_metadata_alloc_chain(map, len, cur_chain);
if (err) {
+ free(cur_chain);
tun_metadata_del_entry(map, idx);
return OFPERR_NXGTMFC_TABLE_FULL;
}
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index f30500e..1dc4d64 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -1749,6 +1749,9 @@ crush_and_string(struct expr *expr, const struct expr_symbol *symbol)
sub->cmp.string = xstrdup(string);
list_push_back(&expr->andor, &sub->node);
}
+
+ sset_destroy(&result);
+
return expr_fix(expr);
}
@@ -1826,7 +1829,7 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol)
&sub->cmp.value, &sub->cmp.mask)) {
list_push_back(&or->andor, &sub->node);
} else {
- free(sub);
+ expr_destroy(sub);
}
}
free(disjuncts);
@@ -1973,7 +1976,7 @@ crush_or(struct expr *expr, const struct expr_symbol *symbol)
if (compare_cmps_3way(a, b)) {
list_push_back(&expr->andor, &b->node);
} else {
- free(b);
+ expr_destroy(b);
}
}
free(subs);
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 8d20630..07afdbe 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -83,7 +83,7 @@ int
main(int argc, char *argv[])
{
const struct ovsdb_client_command *command;
- const char *database;
+ char *database;
struct jsonrpc *rpc;
ovs_cmdl_proctitle_init(argc, argv);
@@ -127,12 +127,13 @@ main(int argc, char *argv[])
fetch_dbs(rpc, &dbs);
if (argc - optind > command->min_args
&& svec_contains(&dbs, argv[optind])) {
- database = argv[optind++];
+ database = xstrdup(argv[optind++]);
} else if (dbs.n == 1) {
database = xstrdup(dbs.names[0]);
} else if (svec_contains(&dbs, "Open_vSwitch")) {
- database = "Open_vSwitch";
+ database = xstrdup("Open_vSwitch");
} else {
+ jsonrpc_close(rpc);
ovs_fatal(0, "no default database for `%s' command, please "
"specify a database name", command->name);
}
@@ -149,6 +150,8 @@ main(int argc, char *argv[])
command->handler(rpc, database, argc - optind, argv + optind);
+ free(database);
+
jsonrpc_close(rpc);
if (ferror(stdout)) {
@@ -451,6 +454,7 @@ do_list_tables(struct jsonrpc *rpc, const char *database,
}
ovsdb_schema_destroy(schema);
table_print(&t, &table_style);
+ table_destroy(&t);
}
static void
@@ -489,6 +493,7 @@ do_list_columns(struct jsonrpc *rpc, const char *database,
}
ovsdb_schema_destroy(schema);
table_print(&t, &table_style);
+ table_destroy(&t);
}
static void
@@ -822,6 +827,7 @@ do_monitor(struct jsonrpc *rpc, const char *database,
table = shash_find_data(&schema->tables, table_name);
if (!table) {
+ ovsdb_schema_destroy(schema);
ovs_fatal(0, "%s: %s does not have a table named \"%s\"",
server, database, table_name);
}
@@ -859,6 +865,9 @@ do_monitor(struct jsonrpc *rpc, const char *database,
if (error == EAGAIN) {
break;
} else if (error) {
+ json_destroy(request_id);
+ free(mts);
+ ovsdb_schema_destroy(schema);
ovs_fatal(error, "%s: receive failed", server);
}
@@ -895,6 +904,10 @@ do_monitor(struct jsonrpc *rpc, const char *database,
unixctl_server_wait(unixctl);
poll_block();
}
+
+ json_destroy(request_id);
+ free(mts);
+ ovsdb_schema_destroy(schema);
}
struct dump_table_aux {
diff --git a/tests/test-aa.c b/tests/test-aa.c
index 2da572d..c3369b0 100644
--- a/tests/test-aa.c
+++ b/tests/test-aa.c
@@ -189,7 +189,7 @@ test_aa_send(void)
(lldp->lldpd == NULL) ||
list_is_empty(&lldp->lldpd->g_hardware)) {
printf("Error: unable to create dummy lldp instance");
- return 1;
+ goto error;
}
/* Populate instance with local chassis info */
@@ -245,7 +245,7 @@ test_aa_send(void)
if (n == 0) {
printf("Error: unable to build packet\n");
- return 1;
+ goto error;
}
/* Decode the constructed LLDPPDU */
@@ -255,7 +255,7 @@ test_aa_send(void)
/* Expecting returned pointers to allocated structures */
if (!nchassis || !nport) {
printf("Error: unable to decode packet");
- return 1;
+ goto error;
}
/* Verify chassis values */
@@ -268,6 +268,10 @@ test_aa_send(void)
check_received_aa(&hardware.h_lport, nport, map_init);
return 0;
+
+error:
+ lldp_unref(lldp);
+ return 1;
}
diff --git a/tests/test-netflow.c b/tests/test-netflow.c
index 2abc57f..aff16c9 100644
--- a/tests/test-netflow.c
+++ b/tests/test-netflow.c
@@ -199,6 +199,7 @@ test_netflow_main(int argc, char *argv[])
error = unixctl_server_create(NULL, &server);
if (error) {
+ free(server);
ovs_fatal(error, "failed to create unixctl server");
}
unixctl_command_register("exit", "", 0, 0, test_netflow_exit, &exiting);
@@ -233,6 +234,8 @@ test_netflow_main(int argc, char *argv[])
unixctl_server_wait(server);
poll_block();
}
+ ofpbuf_uninit(&buf);
+ unixctl_server_destroy(server);
}
static void
diff --git a/tests/test-odp.c b/tests/test-odp.c
index cdc761f..88abbb6 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -109,6 +109,7 @@ parse_keys(bool wc_keys)
next:
ofpbuf_uninit(&odp_key);
+ ofpbuf_uninit(&odp_mask);
}
ds_destroy(&in);
diff --git a/tests/test-sflow.c b/tests/test-sflow.c
index 08591bf..9022a40 100644
--- a/tests/test-sflow.c
+++ b/tests/test-sflow.c
@@ -658,7 +658,7 @@ print_sflow(struct ofpbuf *buf)
static void
test_sflow_main(int argc, char *argv[])
{
- struct unixctl_server *server;
+ struct unixctl_server *server = NULL;
enum { MAX_RECV = 1500 };
const char *target;
struct ofpbuf buf;
@@ -687,6 +687,8 @@ test_sflow_main(int argc, char *argv[])
error = unixctl_server_create(NULL, &server);
if (error) {
+ if (server)
+ free(server);
ovs_fatal(error, "failed to create unixctl server");
}
unixctl_command_register("exit", "", 0, 0, test_sflow_exit, &exiting);
@@ -717,6 +719,8 @@ test_sflow_main(int argc, char *argv[])
unixctl_server_wait(server);
poll_block();
}
+ ofpbuf_uninit(&buf);
+ unixctl_server_destroy(server);
}
static void
diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
index ff6163c..9a7cf5b 100644
--- a/utilities/ovs-appctl.c
+++ b/utilities/ovs-appctl.c
@@ -128,6 +128,7 @@ parse_command_line(int argc, char *argv[])
char *short_options = xasprintf("+%s", short_options_);
const char *target;
int e_options;
+ int exit_type;
target = NULL;
e_options = 0;
@@ -162,7 +163,8 @@ parse_command_line(int argc, char *argv[])
case 'o':
ovs_cmdl_print_options(long_options);
- exit(EXIT_SUCCESS);
+ exit_type = EXIT_SUCCESS;
+ goto error;
case 'T':
time_alarm(atoi(optarg));
@@ -170,12 +172,14 @@ parse_command_line(int argc, char *argv[])
case 'V':
ovs_print_version(0, 0);
- exit(EXIT_SUCCESS);
+ exit_type = EXIT_SUCCESS;
+ goto error;
VLOG_OPTION_HANDLERS
case '?':
- exit(EXIT_FAILURE);
+ exit_type = EXIT_FAILURE;
+ goto error;
default:
OVS_NOT_REACHED();
@@ -190,6 +194,11 @@ parse_command_line(int argc, char *argv[])
}
return target ? target : "ovs-vswitchd";
+
+error:
+ free(short_options_);
+ free(short_options);
+ exit(exit_type);
}
static struct jsonrpc *
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 685c350..a6612e8 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1296,6 +1296,7 @@ static void
bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
size_t n_fms, enum ofputil_protocol usable_protocols)
{
+ struct ofpbuf *request, *request_next;
enum ofputil_protocol protocol;
struct vconn *vconn;
struct ovs_list requests;
@@ -1309,13 +1310,18 @@ bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
for (i = 0; i < n_fms; i++) {
struct ofputil_flow_mod *fm = &fms[i];
- struct ofpbuf *request = ofputil_encode_flow_mod(fm, protocol);
+ request = ofputil_encode_flow_mod(fm, protocol);
list_push_back(&requests, &request->list_node);
free(CONST_CAST(struct ofpact *, fm->ofpacts));
}
bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
+
+ LIST_FOR_EACH_SAFE (request, request_next, list_node, &requests) {
+ ofpbuf_delete(request);
+ }
+
vconn_close(vconn);
}
@@ -2936,6 +2942,7 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx)
{
enum { FILE_IDX = 0, SWITCH_IDX = 1 };
enum ofputil_protocol usable_protocols, protocol;
+ struct ofpbuf *request, *request_next;
struct flow_tables tables;
struct classifier *cls;
struct ovs_list requests;
@@ -2982,6 +2989,11 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx)
} else {
transact_multiple_noreply(vconn, &requests);
}
+
+ LIST_FOR_EACH_SAFE (request, request_next, list_node, &requests) {
+ ofpbuf_delete(request);
+ }
+
vconn_close(vconn);
fte_free_all(&tables);
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 36290db..d3956d1 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -176,6 +176,8 @@ main(int argc, char *argv[])
ovsdb_idl_run(idl);
if (!ovsdb_idl_is_alive(idl)) {
int retval = ovsdb_idl_get_last_error(idl);
+ free(commands);
+ free(args);
ctl_fatal("%s: database connection failed (%s)",
db, ovs_retval_to_string(retval));
}
@@ -190,6 +192,8 @@ main(int argc, char *argv[])
poll_block();
}
}
+ free(commands);
+ free(args);
}
static void
diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 604d19d..f51d299 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -523,6 +523,7 @@ del_cached_port(struct vtep_ctl_context *vtepctl_ctx,
char *cache_name = xasprintf("%s+%s", port->ps->name, port->port_cfg->name);
list_remove(&port->ports_node);
+ shash_destroy(&port->bindings);
shash_find_and_delete(&vtepctl_ctx->ports, cache_name);
vteprec_physical_port_delete(port->port_cfg);
free(cache_name);
--
2.5.0
More information about the dev
mailing list