From 93ff0290fda0f02904686989243089faaa9229e6 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 1 Feb 2010 14:57:48 -0800 Subject: [PATCH] tests: Fix memory leaks in test programs. This makes it easier to see memory leaks in the code under test. Found with valgrind. --- tests/test-json.c | 4 +++- tests/test-jsonrpc.c | 3 ++- tests/test-lockfile.c | 11 +++++++++-- tests/test-ovsdb.c | 18 ++++++++++++++++-- tests/test-stp.c | 17 ++++++++++++++++- tests/test-timeval.c | 7 +++++-- 6 files changed, 51 insertions(+), 9 deletions(-) diff --git a/tests/test-json.c b/tests/test-json.c index 28b0edf1..f297dc20 100644 --- a/tests/test-json.c +++ b/tests/test-json.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009 Nicira Networks. + * Copyright (c) 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -152,5 +152,7 @@ main(int argc, char *argv[]) ok = print_and_free_json(json_from_stream(stream)); } + fclose(stream); + return !ok; } diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c index 570ae11a..03d30001 100644 --- a/tests/test-jsonrpc.c +++ b/tests/test-jsonrpc.c @@ -251,9 +251,10 @@ do_listen(int argc UNUSED, char *argv[]) } poll_block(); } + free(rpcs); + pstream_close(pstream); } - static void do_request(int argc UNUSED, char *argv[]) { diff --git a/tests/test-lockfile.c b/tests/test-lockfile.c index 31e13a72..f0f4b010 100644 --- a/tests/test-lockfile.c +++ b/tests/test-lockfile.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009 Nicira Networks. + * Copyright (c) 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -100,10 +100,15 @@ do_fork(void) static void run_lock_blocks_other_process(void) { - struct lockfile *lockfile; + /* Making this static prevents a memory leak warning from valgrind for the + * parent process, which cannot easily unlock (and free) 'lockfile' because + * it can only do so after the child has exited, and it's the caller of + * this function that does the wait() call. */ + static struct lockfile *lockfile; assert(lockfile_lock("file", 0, &lockfile) == 0); if (do_fork() == CHILD) { + lockfile_unlock(lockfile); assert(lockfile_lock("file", 0, &lockfile) == EAGAIN); exit(11); } @@ -144,6 +149,7 @@ run_lock_timeout_gets_the_lock(void) assert(lockfile_lock("file", 0, &lockfile) == 0); if (do_fork() == CHILD) { + lockfile_unlock(lockfile); assert(lockfile_lock("file", TIME_UPDATE_INTERVAL * 3, &lockfile) == 0); exit(11); @@ -164,6 +170,7 @@ run_lock_timeout_runs_out(void) assert(lockfile_lock("file", 0, &lockfile) == 0); if (do_fork() == CHILD) { + lockfile_unlock(lockfile); assert(lockfile_lock("file", TIME_UPDATE_INTERVAL, &lockfile) == ETIMEDOUT); exit(11); diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 18f8c46e..a40b780f 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009 Nicira Networks. + * Copyright (c) 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -233,7 +233,9 @@ static void check_ovsdb_error(struct ovsdb_error *error) { if (error) { - ovs_fatal(0, "%s", ovsdb_error_to_string(error)); + char *s = ovsdb_error_to_string(error); + ovsdb_error_destroy(error); + ovs_fatal(0, "%s", s); } } @@ -309,6 +311,7 @@ do_log_io(int argc, char *argv[]) char *s = ovsdb_error_to_string(error); printf("%s: %s failed: %s\n", name, command, s); free(s); + ovsdb_error_destroy(error); } else { printf("%s: %s successful\n", name, command); } @@ -490,6 +493,7 @@ do_sort_atoms(int argc UNUSED, char *argv[]) ovsdb_atom_destroy(&atoms[i], type); } print_and_free_json(json_array_create(json_atoms, n_atoms)); + free(atoms); } static void @@ -626,6 +630,10 @@ do_compare_rows(int argc, char *argv[]) } } } + for (i = 0; i < n_rows; i++) { + ovsdb_row_destroy(rows[i]); + free(names[i]); + } free(rows); free(names); @@ -730,9 +738,11 @@ do_evaluate_conditions(int argc UNUSED, char *argv[]) for (i = 0; i < n_conditions; i++) { ovsdb_condition_destroy(&conditions[i]); } + free(conditions); for (i = 0; i < n_rows; i++) { ovsdb_row_destroy(rows[i]); } + free(rows); ovsdb_table_destroy(table); /* Also destroys 'ts'. */ } @@ -859,9 +869,11 @@ do_execute_mutations(int argc UNUSED, char *argv[]) for (i = 0; i < n_sets; i++) { ovsdb_mutation_set_destroy(&sets[i]); } + free(sets); for (i = 0; i < n_rows; i++) { ovsdb_row_destroy(rows[i]); } + free(rows); ovsdb_table_destroy(table); /* Also destroys 'ts'. */ } @@ -1123,6 +1135,7 @@ do_execute(int argc UNUSED, char *argv[]) result = ovsdb_execute(db, params, 0, NULL); s = json_to_string(result, JSSF_SORT); printf("%s\n", s); + free(s); json_destroy(params); json_destroy(result); } @@ -1144,6 +1157,7 @@ do_trigger_dump(struct test_trigger *t, long long int now, const char *title) result = ovsdb_trigger_steal_result(&t->trigger); s = json_to_string(result, JSSF_SORT); printf("t=%lld: trigger %d (%s): %s\n", now, t->number, title, s); + free(s); json_destroy(result); ovsdb_trigger_destroy(&t->trigger); free(t); diff --git a/tests/test-stp.c b/tests/test-stp.c index ce9decc8..eab13d63 100644 --- a/tests/test-stp.c +++ b/tests/test-stp.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009 Nicira Networks. + * Copyright (c) 2008, 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -301,6 +301,7 @@ simulate(struct test_case *tc, int granularity) struct bpdu *bpdu = &b->rxq[b->rxq_tail % RXQ_SIZE]; stp_received_bpdu(stp_get_port(b->stp, bpdu->port_no), bpdu->data, bpdu->size); + free(bpdu->data); any = true; } } @@ -357,6 +358,7 @@ get_token(void) pos++; } if (*pos == '\0') { + free(token); token = NULL; return false; } @@ -644,6 +646,19 @@ main(int argc, char *argv[]) err("trailing garbage on line"); } } + free(token); + + for (i = 0; i < tc->n_lans; i++) { + struct lan *lan = tc->lans[i]; + free((char *) lan->name); + free(lan); + } + for (i = 0; i < tc->n_bridges; i++) { + struct bridge *bridge = tc->bridges[i]; + stp_destroy(bridge->stp); + free(bridge); + } + free(tc); return 0; } diff --git a/tests/test-timeval.c b/tests/test-timeval.c index 3b39b411..533f81ae 100644 --- a/tests/test-timeval.c +++ b/tests/test-timeval.c @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -95,7 +96,7 @@ main(int argc, char *argv[]) } else if (!strcmp(argv[1], "daemon")) { /* Test that time still advances even in a daemon. This is an * interesting test because fork() cancels the interval timer. */ - char cwd[1024]; + char cwd[1024], *pidfile; FILE *success; assert(getcwd(cwd, sizeof cwd) == cwd); @@ -104,7 +105,9 @@ main(int argc, char *argv[]) /* Daemonize, with a pidfile in the current directory. */ set_detach(); - set_pidfile(xasprintf("%s/test-timeval.pid", cwd)); + pidfile = xasprintf("%s/test-timeval.pid", cwd); + set_pidfile(pidfile); + free(pidfile); set_no_chdir(); daemonize(); -- 2.30.2