From f95d7c189bb89474c57dc9b60832df70fe57760f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 9 Sep 2008 20:43:31 +0000 Subject: [PATCH] Refactor unit test macros and tor_free_all() logic a bit so as to make it easier to free memory on failing tests, in order to suppress scanner warnings and to make dmalloc() usable with tests. svn:r16816 --- ChangeLog | 3 +- src/common/test.h | 18 +++---- src/or/buffers.c | 2 +- src/or/main.c | 11 ++-- src/or/rendcommon.c | 6 ++- src/or/test.c | 126 +++++++++++++++++++++++++++++++++++++------- 6 files changed, 130 insertions(+), 36 deletions(-) diff --git a/ChangeLog b/ChangeLog index f7c943f72b..e8096ab9c3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,7 +33,8 @@ Changes in version 0.2.1.6-alpha - 2008-09-xx o Code simplifications and refactoring: - Revise the connection_new functions so that a more typesafe variant exists. This will lower false positives from some scanning tools. - + - Refactor unit testing logic so that dmalloc can be used sensibly with + unit tests to check for memory leaks. Changes in version 0.2.0.31 - 2008-09-03 Tor 0.2.0.31 addresses two potential anonymity issues, starts to fix diff --git a/src/common/test.h b/src/common/test.h index 4709595cf9..bb6a8ae009 100644 --- a/src/common/test.h +++ b/src/common/test.h @@ -31,7 +31,7 @@ extern int have_failed; __LINE__, \ PRETTY_FUNCTION, \ msg); \ - return; \ + goto done; \ STMT_END #define test_fail() test_fail_msg("Assertion failed.") @@ -45,7 +45,7 @@ extern int have_failed; __LINE__, \ PRETTY_FUNCTION, \ #expr); \ - return; \ + goto done; \ } STMT_END #define test_eq_type(tp, fmt, expr1, expr2) \ @@ -61,7 +61,7 @@ extern int have_failed; PRETTY_FUNCTION, \ #expr1, #expr2, \ _test_v1, _test_v2); \ - return; \ + goto done; \ } STMT_END #define test_eq(expr1, expr2) \ @@ -83,7 +83,7 @@ extern int have_failed; PRETTY_FUNCTION, \ #expr1, #expr2, \ _test_v1, _test_v2); \ - return; \ + goto done; \ } STMT_END #define test_neq(expr1, expr2) \ @@ -104,7 +104,7 @@ extern int have_failed; PRETTY_FUNCTION, \ #expr1, #expr2, \ _test_v1, _test_v2); \ - return; \ + goto done; \ } STMT_END #define test_strneq(expr1, expr2) \ @@ -119,7 +119,7 @@ extern int have_failed; PRETTY_FUNCTION, \ #expr1, #expr2, \ _test_v1, _test_v2); \ - return; \ + goto done; \ } STMT_END #define test_memeq(expr1, expr2, len) \ @@ -139,7 +139,7 @@ extern int have_failed; __LINE__, \ PRETTY_FUNCTION, \ #expr1, #expr2, mem1, mem2); \ - return; \ + goto done; \ } STMT_END #define test_memeq_hex(expr1, hex) \ @@ -160,7 +160,7 @@ extern int have_failed; __LINE__, \ PRETTY_FUNCTION, \ #expr1, _test_v2, _mem1, _test_v2); \ - return; \ + goto done; \ } \ tor_free(_mem2); \ STMT_END @@ -177,7 +177,7 @@ extern int have_failed; __LINE__, \ PRETTY_FUNCTION, \ #expr1, #expr2); \ - return; \ + goto done; \ } STMT_END #endif diff --git a/src/or/buffers.c b/src/or/buffers.c index 3893ab08ae..61d4b9d7f7 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -9,7 +9,7 @@ const char buffers_c_id[] = /** * \file buffers.c - * \brief Implements a generic buffer interface. Buffers are + * \brief Implements a generic interface buffer. Buffers are * fairly opaque string holders that can read to or flush from: * memory, file descriptors, or TLS connections. **/ diff --git a/src/or/main.c b/src/or/main.c index a4bc71ef65..a44b427740 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -244,6 +244,8 @@ connection_in_array(connection_t *conn) smartlist_t * get_connection_array(void) { + if (!connection_array) + connection_array = smartlist_create(); return connection_array; } @@ -1951,9 +1953,12 @@ tor_free_all(int postfork) tor_tls_free_all(); } /* stuff in main.c */ - smartlist_free(connection_array); - smartlist_free(closeable_connection_lst); - smartlist_free(active_linked_connection_lst); + if (connection_array) + smartlist_free(connection_array); + if (closeable_connection_lst) + smartlist_free(closeable_connection_lst); + if (active_linked_connection_lst) + smartlist_free(active_linked_connection_lst); tor_free(timeout_event); /* Stuff in util.c and address.c*/ if (!postfork) { diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 44cbc1b534..49a21f63ae 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -837,8 +837,10 @@ _rend_cache_entry_free(void *p) void rend_cache_free_all(void) { - strmap_free(rend_cache, _rend_cache_entry_free); - digestmap_free(rend_cache_v2_dir, _rend_cache_entry_free); + if (rend_cache) + strmap_free(rend_cache, _rend_cache_entry_free); + if (rend_cache_v2_dir) + digestmap_free(rend_cache_v2_dir, _rend_cache_entry_free); rend_cache = NULL; rend_cache_v2_dir = NULL; } diff --git a/src/or/test.c b/src/or/test.c index 311d23979e..0e8b488649 100644 --- a/src/or/test.c +++ b/src/or/test.c @@ -14,6 +14,7 @@ const char tor_svn_revision[] = ""; **/ #include "orconfig.h" + #include #ifdef HAVE_FCNTL_H #include @@ -44,6 +45,10 @@ const char tor_svn_revision[] = ""; #include "mempool.h" #include "memarea.h" +#ifdef USE_DMALLOC +#include +#endif + int have_failed = 0; static char temp_dir[256]; @@ -341,20 +346,22 @@ test_buffers(void) test_eq(eof, 1); } #endif + + done: + ; } static void test_crypto_dh(void) { - crypto_dh_env_t *dh1, *dh2; + crypto_dh_env_t *dh1 = crypto_dh_new(); + crypto_dh_env_t *dh2 = crypto_dh_new(); char p1[DH_BYTES]; char p2[DH_BYTES]; char s1[DH_BYTES]; char s2[DH_BYTES]; int s1len, s2len; - dh1 = crypto_dh_new(); - dh2 = crypto_dh_new(); test_eq(crypto_dh_get_bytes(dh1), DH_BYTES); test_eq(crypto_dh_get_bytes(dh2), DH_BYTES); @@ -380,6 +387,7 @@ test_crypto_dh(void) */ } + done: crypto_dh_free(dh1); crypto_dh_free(dh2); } @@ -771,6 +779,9 @@ test_crypto(void) crypto_digest(d_out2, "abcdef", 6); test_memeq(d_out1, d_out2, DIGEST_LEN); } + + done: + ; } static void @@ -778,7 +789,7 @@ test_crypto_s2k(void) { char buf[29]; char buf2[29]; - char *buf3; + char *buf3 = NULL; int i; memset(buf, 0, sizeof(buf)); @@ -800,6 +811,8 @@ test_crypto_s2k(void) } crypto_digest(buf2+9, buf3, 65536); test_memeq(buf, buf2, 29); + + done: tor_free(buf3); } @@ -1184,6 +1197,9 @@ test_util(void) test_eq(round_to_power_of_2(U64_LITERAL(40000000000000000)), U64_LITERAL(1)<<55); test_eq(round_to_power_of_2(0), 2); + + done: + ; } /** Helper: assert that IPv6 addresses a and b are the same. On @@ -1562,6 +1578,9 @@ test_util_ip6_helpers(void) tor_inet_ntop(AF_INET6, &t2.sa6.sin6_addr, buf, sizeof(buf)); printf("\nv6 address: %s (family=%i)", buf, IN_FAMILY(&t2)); #endif + + done: + ; } static void @@ -1941,12 +1960,15 @@ test_util_smartlist(void) } smartlist_free(sl); + + done: + ; } static void test_util_bitarray(void) { - bitarray_t *ba; + bitarray_t *ba = NULL; int i, j, ok=1; ba = bitarray_init_zero(1); @@ -1977,7 +1999,10 @@ test_util_bitarray(void) else i += 7; } - bitarray_free(ba); + + done: + if (ba) + bitarray_free(ba); } static void @@ -1988,7 +2013,7 @@ test_util_digestset(void) int i; int ok = 1; int false_positives = 0; - digestset_t *set; + digestset_t *set = NULL; for (i = 0; i < 1000; ++i) { crypto_rand(d, DIGEST_LEN); @@ -2011,7 +2036,12 @@ test_util_digestset(void) ++false_positives; } test_assert(false_positives < 50); /* Should be far lower. */ - digestset_free(set); + + done: + if (set) + digestset_free(set); + SMARTLIST_FOREACH(included, char *, cp, tor_free(cp)); + smartlist_free(included); } /* stop threads running at once. */ @@ -2118,6 +2148,8 @@ test_util_threads(void) tor_free(s1); tor_free(s2); + done: + ; } static int @@ -2129,7 +2161,7 @@ _compare_strings_for_pqueue(const void *s1, const void *s2) static void test_util_pqueue(void) { - smartlist_t *sl; + smartlist_t *sl = NULL; int (*cmp)(const void *, const void*); #define OK() smartlist_pqueue_assert_ok(sl, cmp) @@ -2177,7 +2209,10 @@ test_util_pqueue(void) test_eq(smartlist_len(sl), 0); OK(); #undef OK - smartlist_free(sl); + + done: + if (sl) + smartlist_free(sl); } static void @@ -2284,6 +2319,8 @@ test_util_gzip(void) tor_free(buf2); tor_free(buf3); tor_free(buf1); + done: + ; } static void @@ -2367,6 +2404,9 @@ test_util_strmap(void) strmap_assert_ok(map); test_eq_ptr(strmap_get_lc(map,"AB.C"), NULL); strmap_free(map,NULL); + + done: + ; } static void @@ -2435,12 +2475,14 @@ test_util_mmap(void) tor_free(fname2); tor_free(fname3); tor_free(buf); + done: + ; } static void test_util_control_formats(void) { - char *out; + char *out = NULL; const char *inp = "..This is a test\r\nof the emergency \nbroadcast\r\n..system.\r\nZ.\r\n"; size_t sz; @@ -2450,6 +2492,7 @@ test_util_control_formats(void) ".This is a test\nof the emergency \nbroadcast\n.system.\nZ.\n"); test_eq(sz, strlen(out)); + done: tor_free(out); } @@ -2484,8 +2527,6 @@ test_onion_handshake(void) memset(c_keys, 0, 40); test_assert(! onion_skin_client_handshake(c_dh, s_buf, c_keys, 40)); - crypto_dh_free(c_dh); - if (memcmp(c_keys, s_keys, 40)) { puts("Aiiiie"); exit(1); @@ -2493,7 +2534,12 @@ test_onion_handshake(void) test_memeq(c_keys, s_keys, 40); memset(s_buf, 0, 40); test_memneq(c_keys, s_buf, 40); - crypto_free_pk_env(pk); + + done: + if (c_dh) + crypto_dh_free(c_dh); + if (pk) + crypto_free_pk_env(pk); } extern smartlist_t *fingerprint_list; @@ -2813,6 +2859,8 @@ test_dir_format(void) "Tor 0.2.1.0-dev (r99)")); test_eq(1, tor_version_as_new_as("Tor 0.2.1.1", "Tor 0.2.1.0-dev (r99)")); + done: + ; } extern const char AUTHORITY_CERT_1[]; @@ -2834,6 +2882,8 @@ test_same_voter(networkstatus_voter_info_t *v1, test_eq(v1->or_port, v2->or_port); test_streq(v1->contact, v2->contact); test_memeq(v1->vote_digest, v2->vote_digest, DIGEST_LEN); + done: + ; } static void @@ -2860,6 +2910,9 @@ test_util_order_functions(void) //smartlist_shuffle(sl); test_eq(25, median()); /* 12,12,24,25,60,77,77 */ #undef median + + done: + ; } static routerinfo_t * @@ -2882,7 +2935,7 @@ generate_ri_from_rs(const vote_routerstatus_t *vrs) r->exit_policy = smartlist_create(); r->cache_info.published_on = ++published + time(NULL); return r; -}; +} static void test_v3_networkstatus(void) @@ -3355,6 +3408,8 @@ test_v3_networkstatus(void) authority_cert_free(cert1); authority_cert_free(cert2); authority_cert_free(cert3); + done: + ; } static void @@ -3362,8 +3417,8 @@ test_policy_summary_helper(const char *policy_str, const char *expected_summary) { config_line_t line; - smartlist_t *policy; - char *summary; + smartlist_t *policy = NULL; + char *summary = NULL; policy = NULL; line.key = (char*)"foo"; @@ -3375,8 +3430,11 @@ test_policy_summary_helper(const char *policy_str, test_assert(summary != NULL); test_streq(summary, expected_summary); + + done: tor_free(summary); - addr_policy_list_free(policy); + if (policy) + addr_policy_list_free(policy); } static void @@ -3528,6 +3586,8 @@ test_policies(void) tor_free(policy_str); SMARTLIST_FOREACH(sm, char *, s, tor_free(s)); smartlist_clear(sm); + done: + ; } static void @@ -3589,6 +3649,8 @@ test_rend_fns(void) crypto_free_pk_env(pk1); crypto_free_pk_env(pk2); + done: + ; } static void @@ -3731,6 +3793,8 @@ test_util_mempool(void) mp_pool_assert_ok(pool); mp_pool_destroy(pool); smartlist_free(allocated); + done: + ; } static void @@ -3821,14 +3885,17 @@ test_util_memarea(void) test_assert(memarea_owns_ptr(area, p1)); test_assert(memarea_owns_ptr(area, p2)); + + done: memarea_drop_all(area); + ; } static void test_util_datadir(void) { char buf[1024]; - char *f; + char *f = NULL; f = get_datadir_fname(NULL); test_streq(f, temp_dir); @@ -3851,6 +3918,8 @@ test_util_datadir(void) tor_snprintf(buf, sizeof(buf), "%s"PATH_SEPARATOR"cache.foo", temp_dir); test_streq(f, buf); + + done: tor_free(f); } @@ -3968,6 +4037,8 @@ test_crypto_aes_iv(void) tor_free(encrypted2); tor_free(decrypted1); tor_free(decrypted2); + done: + ; } /* Test base32 decoding. */ @@ -4000,6 +4071,9 @@ test_crypto_base32_decode(void) encoded[0] = '!'; res = base32_decode(decoded, 60, encoded, 96); test_assert(res < 0); + + done: + ; } /* Test encoding and parsing of v2 rendezvous service descriptors. */ @@ -4090,6 +4164,8 @@ test_rend_fns_v2(void) smartlist_free(descs); rend_service_descriptor_free(parsed); rend_service_descriptor_free(generated); + done: + ; } static void @@ -4145,6 +4221,8 @@ test_geoip(void) test_assert(s); test_streq("ab=16,xy=8", s); tor_free(s); + done: + ; } #define ENT(x) { #x, test_ ## x, 0, 0 } @@ -4301,7 +4379,15 @@ main(int c, char**v) } puts(""); - crypto_global_cleanup(); + { + void *x = tor_malloc(1024); + (void)x; + } + +#ifdef USE_DMALLOC + tor_free_all(0); + dmalloc_log_unfreed(); +#endif if (have_failed) return 1;