Switch to a safer FREE_AND_NULL implementation

This one only evaluates the input once, so it cannot mess up even if
there are side effects.

diff --git a/src/common/address.h b/src/common/address.h
index 84d136d671eaba698279d47dbcd979f1eb7d4edc..6f59e1c962e4eae566e3c1db75ae28e206d965d3 100644 (file)
--- a/src/common/address.h
+++ b/src/common/address.h
@@ -208,7 +208,7 @@ MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family,
 tor_addr_t *addr));
 void interface_address6_list_free_(smartlist_t * addrs);// XXXX
 #define interface_address6_list_free(addrs) \
-  FREE_AND_NULL(interface_address6_list, (addrs))
+  FREE_AND_NULL_UNMATCHED(smartlist_t, interface_address6_list_free_, (addrs))
 MOCK_DECL(smartlist_t *,get_interface_address6_list,(int severity,
                                                      sa_family_t family,
                                                      int include_internal));

diff --git a/src/common/aes.h b/src/common/aes.h
index 4874f728d457e08e55d67231e365bc0fcd08050b..c2720d29b8fd637136642bbd2f0ca167fe294335 100644 (file)
--- a/src/common/aes.h
+++ b/src/common/aes.h
@@ -18,7 +18,8 @@ typedef struct aes_cnt_cipher aes_cnt_cipher_t;
 aes_cnt_cipher_t* aes_new_cipher(const uint8_t *key, const uint8_t *iv,
                                  int key_bits);
 void aes_cipher_free_(aes_cnt_cipher_t *cipher);
-#define aes_cipher_free(cipher) FREE_AND_NULL(aes_cipher, (cipher))
+#define aes_cipher_free(cipher) \
+  FREE_AND_NULL_UNMATCHED(aes_cnt_cipher_t, aes_cipher_free_, (cipher))
 void aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len);
 
 int evaluate_evp_for_aes(int force_value);

diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h
index 65807315c99cb1e063ede962c068773bcb7d8143..2c86194d040b2e6517151e3a33174712b20536e3 100644 (file)
--- a/src/common/compat_libevent.h
+++ b/src/common/compat_libevent.h
@@ -20,7 +20,8 @@ void suppress_libevent_log_msg(const char *msg);
   (sock),(tcp),(cb),(data));
 
 void tor_event_free_(struct event *ev);
-#define tor_event_free(ev) FREE_AND_NULL(tor_event, (ev))
+#define tor_event_free(ev) \
+  FREE_AND_NULL_UNMATCHED(struct event, tor_event_free_, (ev))
 
 typedef struct periodic_timer_t periodic_timer_t;
 

diff --git a/src/common/compress.h b/src/common/compress.h
index eee4a315db70aec662499406836779822061e810..7420f169fde91de4a9fbe2ab6716e21536c2511f 100644 (file)
--- a/src/common/compress.h
+++ b/src/common/compress.h
@@ -81,7 +81,8 @@ tor_compress_output_t tor_compress_process(tor_compress_state_t *state,
                                            const char **in, size_t *in_len,
                                            int finish);
 void tor_compress_free_(tor_compress_state_t *state);
-#define tor_compress_free(st) FREE_AND_NULL(tor_compress, (st))
+#define tor_compress_free(st) \
+  FREE_AND_NULL_UNMATCHED(tor_compress_state_t, tor_compress_free_, (st))
 
 size_t tor_compress_state_size(const tor_compress_state_t *state);
 

diff --git a/src/common/compress_lzma.h b/src/common/compress_lzma.h
index 1e92fd660c14bc62a2751cd2b0d61be89783b05d..986bfe9a67343f20a5864b86d75d52e252d4f1b3 100644 (file)
--- a/src/common/compress_lzma.h
+++ b/src/common/compress_lzma.h
@@ -32,7 +32,9 @@ tor_lzma_compress_process(tor_lzma_compress_state_t *state,
                           int finish);
 
 void tor_lzma_compress_free_(tor_lzma_compress_state_t *state);
-#define tor_lzma_compress_free(st) FREE_AND_NULL(tor_lzma_compress, (st))
+#define tor_lzma_compress_free(st)                      \
+  FREE_AND_NULL_UNMATCHED(tor_lzma_compress_state_t,   \
+                           tor_lzma_compress_free_, (st))
 
 size_t tor_lzma_compress_state_size(const tor_lzma_compress_state_t *state);
 

diff --git a/src/common/compress_zlib.h b/src/common/compress_zlib.h
index 3377e0e8de1cf103edf8ad232322afd00ed0a63e..701e2f9e89295b65630e44bca0957539cee04503 100644 (file)
--- a/src/common/compress_zlib.h
+++ b/src/common/compress_zlib.h
@@ -32,7 +32,9 @@ tor_zlib_compress_process(tor_zlib_compress_state_t *state,
                           int finish);
 
 void tor_zlib_compress_free_(tor_zlib_compress_state_t *state);
-#define tor_zlib_compress_free(st) FREE_AND_NULL(tor_zlib_compress, (st))
+#define tor_zlib_compress_free(st)                      \
+  FREE_AND_NULL_UNMATCHED(tor_zlib_compress_state_t,   \
+                           tor_zlib_compress_free_, (st))
 
 size_t tor_zlib_compress_state_size(const tor_zlib_compress_state_t *state);
 

diff --git a/src/common/compress_zstd.h b/src/common/compress_zstd.h
index 9f67a9c5884ea73cfa1f305edda8dc8104a73a5b..3e18febb12ba5a3f427db59dea5905cc3f1eb884 100644 (file)
--- a/src/common/compress_zstd.h
+++ b/src/common/compress_zstd.h
@@ -32,7 +32,9 @@ tor_zstd_compress_process(tor_zstd_compress_state_t *state,
                           int finish);
 
 void tor_zstd_compress_free_(tor_zstd_compress_state_t *state);
-#define tor_zstd_compress_free(st) FREE_AND_NULL(tor_zstd_compress, (st))
+#define tor_zstd_compress_free(st)                      \
+  FREE_AND_NULL_UNMATCHED(tor_zstd_compress_state_t,   \
+                           tor_zstd_compress_free_, (st))
 
 size_t tor_zstd_compress_state_size(const tor_zstd_compress_state_t *state);
 

diff --git a/src/common/log.c b/src/common/log.c
index 83098b10117adcaf7ad286b73e1f788044b9cd98..d305c9ae5e45998cbf6910441f228aedc6d16eb3 100644 (file)
--- a/src/common/log.c
+++ b/src/common/log.c
@@ -65,7 +65,7 @@ typedef struct logfile_t {
 
 static void log_free_(logfile_t *victim);
 #define log_free(lg)    \
-  FREE_AND_NULL(log, (lg))
+  FREE_AND_NULL_UNMATCHED(logfile_t, log_free_, (lg))
 
 /** Helper: map a log severity to descriptive string. */
 static inline const char *

diff --git a/src/common/timers.h b/src/common/timers.h
index 6c9594d31b0bc7b9861e5ba7934f3338ec59102c..86e892016994a89245b4dfc73e8778f53531a497 100644 (file)
--- a/src/common/timers.h
+++ b/src/common/timers.h
@@ -18,7 +18,7 @@ void timer_get_cb(const tor_timer_t *t,
 void timer_schedule(tor_timer_t *t, const struct timeval *delay);
 void timer_disable(tor_timer_t *t);
 void timer_free_(tor_timer_t *t);
-#define timer_free(t) FREE_AND_NULL(timer, (t))
+#define timer_free(t) FREE_AND_NULL_UNMATCHED(tor_timer_t, timer_free_, (t))
 
 void timers_initialize(void);
 void timers_shutdown(void);

diff --git a/src/common/util.h b/src/common/util.h
index c5bd3f0bda8aa718ed91b9a2a5e9f7e41a920c49..9ed11260dc0ae33063b5f4eedf1a72a0f4322743 100644 (file)
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -109,13 +109,24 @@ extern int dmalloc_free(const char *file, const int line, void *pnt,
 
 void tor_log_mallinfo(int severity);
 
+/* Helper macro: free a variable of type 'typename' using freefn, and
+ * set the variable to NULL.
+ *
+ * We use this for legacy cases when freefn and typename don't line up
+ * perfectly.
+ */
+#define FREE_AND_NULL_UNMATCHED(typename, freefn, var)                  \
+  do {                                                                  \
+    /* only evaluate (var) once. */                                     \
+    typename **tmp__free__ptr ## freefn = &(var);                       \
+    freefn(*tmp__free__ptr ## freefn);                                  \
+    (*tmp__free__ptr ## freefn) = NULL;                                 \
+  } while (0)
+
 /* Helper macro: free a variable of type 'type' using type_free_, and
  * set the variable to NULL. */
-#define FREE_AND_NULL(type, var)                                \
-  do {                                                          \
-      type ## _free_(var);                                      \
-      (var) = NULL;                                             \
-  } while (0)
+#define FREE_AND_NULL(type, var)                                        \
+  FREE_AND_NULL_UNMATCHED(type ## _t, type ## _free_, (var))
 
 /** Macro: yield a pointer to the field at position <b>off</b> within the
  * structure <b>st</b>.  Example:

diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index e88fb23892156a10d3f4329ec6b602c197475cef..1f93c2d5208ce80578f540e0d5e5a898f1f307cf 100644 (file)
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -370,7 +370,7 @@ service_intro_point_free_(hs_service_intro_point_t *ip)
 static void
 service_intro_point_free_void(void *obj)
 {
-  service_intro_point_free(obj);
+  service_intro_point_free_(obj);
 }
 
 /* Return a newly allocated service intro point and fully initialized from the

diff --git a/src/or/hs_service.h b/src/or/hs_service.h
index b759030166d3028bda30dc770d3dbc80e2fb8fbb..f933ba6ab165ca5019654beff6bb11eea225eb2a 100644 (file)
--- a/src/or/hs_service.h
+++ b/src/or/hs_service.h
@@ -295,8 +295,9 @@ STATIC hs_service_intro_point_t *service_intro_point_new(
                                          const extend_info_t *ei,
                                          unsigned int is_legacy);
 STATIC void service_intro_point_free_(hs_service_intro_point_t *ip);
-#define service_intro_point_free(ip)            \
-  FREE_AND_NULL(service_intro_point, (ip))
+#define service_intro_point_free(ip)                            \
+  FREE_AND_NULL_UNMATCHED(hs_service_intro_point_t,             \
+                          service_intro_point_free_, (ip))
 STATIC void service_intro_point_add(digest256map_t *map,
                                     hs_service_intro_point_t *ip);
 STATIC void service_intro_point_remove(const hs_service_t *service,
@@ -330,7 +331,9 @@ STATIC char *
 encode_desc_rev_counter_for_state(const hs_service_descriptor_t *desc);
 
 STATIC void service_descriptor_free_(hs_service_descriptor_t *desc);
-#define service_descriptor_free(d) FREE_AND_NULL(service_descriptor, (d))
+#define service_descriptor_free(d) \
+  FREE_AND_NULL_UNMATCHED(hs_service_descriptor_t, \
+                           service_descriptor_free_, (d))
 
 STATIC uint64_t
 check_state_line_for_service_rev_counter(const char *state_line,

diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
index f62b5d240918b14438be7ceb053cdb4391511e83..37350bb1b3a208ee21e69bc1db2fb5332456e221 100644 (file)
--- a/src/or/networkstatus.h
+++ b/src/or/networkstatus.h
@@ -21,7 +21,8 @@ int router_reload_consensus_networkstatus(void);
 void routerstatus_free_(routerstatus_t *rs);
 #define routerstatus_free(rs) FREE_AND_NULL(routerstatus, (rs))
 void networkstatus_vote_free_(networkstatus_t *ns);
-#define networkstatus_vote_free(ns) FREE_AND_NULL(networkstatus_vote, (ns))
+#define networkstatus_vote_free(ns) \
+  FREE_AND_NULL_UNMATCHED(networkstatus_t, networkstatus_vote_free_, (ns))
 networkstatus_voter_info_t *networkstatus_get_voter_by_id(
                                        networkstatus_t *vote,
                                        const char *identity);

diff --git a/src/or/policies.h b/src/or/policies.h
index 062f3301834fff59c4fc36da66024c730a7ea449..451a7a7f365bf3f344f4f34d4ccdaec51b2eb0dd 100644 (file)
--- a/src/or/policies.h
+++ b/src/or/policies.h
@@ -116,7 +116,8 @@ int policy_write_item(char *buf, size_t buflen, const addr_policy_t *item,
                       int format_for_desc);
 
 void addr_policy_list_free_(smartlist_t *p);
-#define addr_policy_list_free(lst) FREE_AND_NULL(addr_policy_list, (lst))
+#define addr_policy_list_free(lst) \
+  FREE_AND_NULL_UNMATCHED(smartlist_t, addr_policy_list_free_, (lst))
 void addr_policy_free_(addr_policy_t *p);
 #define addr_policy_free(p) FREE_AND_NULL(addr_policy, (p))
 void policies_free_all(void);

diff --git a/src/or/rendcache.c b/src/or/rendcache.c
index fa454321d0bbe4d0af9e0edd578cd0f73bfd37d0..6f56df671875565990b1a554137c76842b3a339c 100644 (file)
--- a/src/or/rendcache.c
+++ b/src/or/rendcache.c
@@ -131,7 +131,7 @@ rend_cache_failure_intro_entry_free_(rend_cache_failure_intro_t *entry)
 static void
 rend_cache_failure_intro_entry_free_void(void *entry)
 {
-  rend_cache_failure_intro_entry_free(entry);
+  rend_cache_failure_intro_entry_free_(entry);
 }
 
 /** Allocate a rend cache failure intro object and return it. <b>failure</b>
@@ -165,7 +165,7 @@ rend_cache_failure_entry_free_(rend_cache_failure_t *entry)
 STATIC void
 rend_cache_failure_entry_free_void(void *entry)
 {
-  rend_cache_failure_entry_free(entry);
+  rend_cache_failure_entry_free_(entry);
 }
 
 /** Allocate a rend cache failure object and return it. This function can
@@ -219,7 +219,7 @@ rend_cache_entry_free_(rend_cache_entry_t *e)
 static void
 rend_cache_entry_free_void(void *p)
 {
-  rend_cache_entry_free(p);
+  rend_cache_entry_free_(p);
 }
 
 /** Free all storage held by the service descriptor cache. */

diff --git a/src/or/rendcache.h b/src/or/rendcache.h
index 66b48e032003e3e740524eb666d5369dc55fbd6d..c1e9207a95d7c55775d7b487bd6c53bbc94eb2cf 100644 (file)
--- a/src/or/rendcache.h
+++ b/src/or/rendcache.h
@@ -94,11 +94,13 @@ STATIC void rend_cache_entry_free_(rend_cache_entry_t *e);
 #define rend_cache_entry_free(e) FREE_AND_NULL(rend_cache_entry, (e))
 STATIC void rend_cache_failure_intro_entry_free_(rend_cache_failure_intro_t
                                                  *entry);
-#define rend_cache_failure_intro_entry_free(e) \
-  FREE_AND_NULL(rend_cache_failure_intro_entry, (e))
+#define rend_cache_failure_intro_entry_free(e)                          \
+  FREE_AND_NULL_UNMATCHED(rend_cache_failure_intro_t,                   \
+                          rend_cache_failure_intro_entry_free_, (e))
 STATIC void rend_cache_failure_entry_free_(rend_cache_failure_t *entry);
-#define rend_cache_failure_entry_free(e) \
-  FREE_AND_NULL(rend_cache_failure_entry, (e))
+#define rend_cache_failure_entry_free(e)                        \
+  FREE_AND_NULL_UNMATCHED(rend_cache_failure_t,                 \
+                          rend_cache_failure_entry_free_, (e))
 STATIC int cache_failure_intro_lookup(const uint8_t *identity,
                                       const char *service_id,
                                       rend_cache_failure_intro_t

diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 568afe79b7532eefc0088256ebc357dc57f6bb59..3e318f73fe0d76d66ccbfb05428e123bfdbce16a 100644 (file)
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -174,7 +174,7 @@ rend_authorized_client_free_(rend_authorized_client_t *client)
 static void
 rend_authorized_client_strmap_item_free(void *authorized_client)
 {
-  rend_authorized_client_free(authorized_client);
+  rend_authorized_client_free_(authorized_client);
 }
 
 /** Release the storage held by <b>service</b>.
@@ -3726,7 +3726,7 @@ upload_service_descriptor(rend_service_t *service)
       }
       /* Free memory for descriptors. */
       for (i = 0; i < smartlist_len(descs); i++)
-        rend_encoded_v2_service_descriptor_free(smartlist_get(descs, i));
+        rend_encoded_v2_service_descriptor_free_(smartlist_get(descs, i));
       smartlist_clear(descs);
       /* Update next upload time. */
       if (seconds_valid - REND_TIME_PERIOD_OVERLAPPING_V2_DESCS
@@ -3757,7 +3757,7 @@ upload_service_descriptor(rend_service_t *service)
         }
         /* Free memory for descriptors. */
         for (i = 0; i < smartlist_len(descs); i++)
-          rend_encoded_v2_service_descriptor_free(smartlist_get(descs, i));
+          rend_encoded_v2_service_descriptor_free_(smartlist_get(descs, i));
         smartlist_clear(descs);
       }
     }

diff --git a/src/or/router.h b/src/or/router.h
index cd953da7aa26f307a79df46eb02060f6f7ea1b5d..e282b9975d4f4569b897fffdaf62466b9c412806 100644 (file)
--- a/src/or/router.h
+++ b/src/or/router.h
@@ -37,7 +37,8 @@ int get_onion_key_grace_period(void);
 
 di_digest256_map_t *construct_ntor_key_map(void);
 void ntor_key_map_free_(di_digest256_map_t *map);
-#define ntor_key_map_free(map) FREE_AND_NULL(ntor_key_map, (map))
+#define ntor_key_map_free(map) \
+  FREE_AND_NULL_UNMATCHED(di_digest256_map_t, ntor_key_map_free_, (map))
 
 int router_initialize_tls_context(void);
 int init_keys(void);

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 877479969f383c8acbd4ec263171290fac34c508..0236761eb33e9910ea17fc0888a948df63ef0b07 100644 (file)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -3268,7 +3268,7 @@ signed_descriptor_from_routerinfo(routerinfo_t *ri)
 static void
 extrainfo_free_void(void *e)
 {
-  extrainfo_free(e);
+  extrainfo_free_(e);
 }
 
 /** Free all storage held by a routerlist <b>rl</b>. */

diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c
index ae904cfda3f382889840e86318b8db6688e1088e..8c5a497bb35a3d78f5f8b9065d205d3c9d3c886f 100644 (file)
--- a/src/or/shared_random_state.c
+++ b/src/or/shared_random_state.c
@@ -271,7 +271,7 @@ commit_add_to_state(sr_commit_t *commit, sr_state_t *state)
 static void
 commit_free_(void *p)
 {
-  sr_commit_free(p);
+  sr_commit_free_(p);
 }
 
 /* Free a state that was allocated with state_new(). */

diff --git a/src/test/test.c b/src/test/test.c
index 1c5b654df215e85020f743a17d53290b8cc9a588..c08967c02547761b03707e0e6bffe026104c0104 100644 (file)
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -616,7 +616,7 @@ test_rend_fns(void *arg)
  done:
   if (descs) {
     for (i = 0; i < smartlist_len(descs); i++)
-      rend_encoded_v2_service_descriptor_free(smartlist_get(descs, i));
+      rend_encoded_v2_service_descriptor_free_(smartlist_get(descs, i));
     smartlist_free(descs);
   }
   if (parsed)

diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 4cab307ce15aa0e41bad1108d2c44ab02dbccdf4..35e4fb44c87c0eff6baedf236fa668e6cf15c461 100644 (file)
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -567,7 +567,7 @@ test_dir_routerinfo_parsing(void *arg)
 static void
 routerinfo_free_wrapper_(void *arg)
 {
-  routerinfo_free(arg);
+  routerinfo_free_(arg);
 }
 
 static void