[tor-commits] [tor/master] Another clang analyzer complaint wrt HT_GENERATE

nickm at torproject.org nickm at torproject.org
Tue Sep 2 16:50:20 UTC 2014


commit 00ffccd9a6d9b7f484c2e421be33aea3975a2879
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Sep 2 12:48:34 2014 -0400

    Another clang analyzer complaint wrt HT_GENERATE
    
    We're calling mallocfn() and reallocfn() in the HT_GENERATE macro
    with the result of a product.  But that makes any sane analyzer
    worry about overflow.
    
    This patch keeps HT_GENERATE having its old semantics, since we
    aren't the only project using ht.h.  Instead, define a HT_GENERATE2
    that takes a reallocarrayfn.
---
 src/common/container.c    |    8 ++++----
 src/common/sandbox.c      |    8 ++++----
 src/common/util_process.c |    4 ++--
 src/ext/ht.h              |   23 +++++++++++++++++++----
 src/or/channel.c          |    4 ++--
 src/or/circuitlist.c      |    6 +++---
 src/or/circuitmux.c       |    6 +++---
 src/or/dns.c              |    4 ++--
 src/or/fp_pair.c          |    6 +++---
 src/or/geoip.c            |    8 ++++----
 src/or/microdesc.c        |    4 ++--
 src/or/nodelist.c         |    4 ++--
 src/or/policies.c         |    4 ++--
 src/or/rephist.c          |    4 ++--
 14 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/src/common/container.c b/src/common/container.c
index 7481d31..f7dfc69 100644
--- a/src/common/container.c
+++ b/src/common/container.c
@@ -1052,13 +1052,13 @@ digestmap_entry_hash(const digestmap_entry_t *a)
 
 HT_PROTOTYPE(strmap_impl, strmap_entry_t, node, strmap_entry_hash,
              strmap_entries_eq)
-HT_GENERATE(strmap_impl, strmap_entry_t, node, strmap_entry_hash,
-            strmap_entries_eq, 0.6, malloc, realloc, free)
+HT_GENERATE2(strmap_impl, strmap_entry_t, node, strmap_entry_hash,
+             strmap_entries_eq, 0.6, tor_reallocarray_, tor_free_)
 
 HT_PROTOTYPE(digestmap_impl, digestmap_entry_t, node, digestmap_entry_hash,
              digestmap_entries_eq)
-HT_GENERATE(digestmap_impl, digestmap_entry_t, node, digestmap_entry_hash,
-            digestmap_entries_eq, 0.6, malloc, realloc, free)
+HT_GENERATE2(digestmap_impl, digestmap_entry_t, node, digestmap_entry_hash,
+             digestmap_entries_eq, 0.6, tor_reallocarray_, tor_free_)
 
 /** Constructor to create a new empty map from strings to void*'s.
  */
diff --git a/src/common/sandbox.c b/src/common/sandbox.c
index b414e31..c7e4dcd 100644
--- a/src/common/sandbox.c
+++ b/src/common/sandbox.c
@@ -1292,10 +1292,10 @@ static HT_HEAD(getaddrinfo_cache, cached_getaddrinfo_item_t)
 HT_PROTOTYPE(getaddrinfo_cache, cached_getaddrinfo_item_t, node,
              cached_getaddrinfo_item_hash,
              cached_getaddrinfo_items_eq);
-HT_GENERATE(getaddrinfo_cache, cached_getaddrinfo_item_t, node,
-            cached_getaddrinfo_item_hash,
-            cached_getaddrinfo_items_eq,
-            0.6, tor_malloc_, tor_realloc_, tor_free_);
+HT_GENERATE2(getaddrinfo_cache, cached_getaddrinfo_item_t, node,
+             cached_getaddrinfo_item_hash,
+             cached_getaddrinfo_items_eq,
+             0.6, tor_reallocarray_, tor_free_)
 
 int
 sandbox_getaddrinfo(const char *name, const char *servname,
diff --git a/src/common/util_process.c b/src/common/util_process.c
index d6ef590..a6a2a9d 100644
--- a/src/common/util_process.c
+++ b/src/common/util_process.c
@@ -62,8 +62,8 @@ static HT_HEAD(process_map, waitpid_callback_t) process_map = HT_INITIALIZER();
 
 HT_PROTOTYPE(process_map, waitpid_callback_t, node, process_map_entry_hash_,
              process_map_entries_eq_);
-HT_GENERATE(process_map, waitpid_callback_t, node, process_map_entry_hash_,
-            process_map_entries_eq_, 0.6, malloc, realloc, free);
+HT_GENERATE2(process_map, waitpid_callback_t, node, process_map_entry_hash_,
+             process_map_entries_eq_, 0.6, tor_reallocarray_, tor_free_);
 
 /**
  * Begin monitoring the child pid <b>pid</b> to see if we get a SIGCHLD for
diff --git a/src/ext/ht.h b/src/ext/ht.h
index 8387107..61e9719 100644
--- a/src/ext/ht.h
+++ b/src/ext/ht.h
@@ -302,8 +302,8 @@ ht_string_hash(const char *s)
     }                                                                   \
   }
 
-#define HT_GENERATE(name, type, field, hashfn, eqfn, load, mallocfn,    \
-                    reallocfn, freefn)                                  \
+#define HT_GENERATE2(name, type, field, hashfn, eqfn, load, reallocarrayfn, \
+                     freefn)                                            \
   /* Primes that aren't too far from powers of two. We stop at */       \
   /* P=402653189 because P*sizeof(void*) is less than SSIZE_MAX */      \
   /* even on a 32-bit platform. */                                      \
@@ -336,7 +336,7 @@ ht_string_hash(const char *s)
       new_load_limit = (unsigned)(load*new_len);                        \
     } while (new_load_limit <= size &&                                  \
              prime_idx < (int)name##_N_PRIMES);                         \
-    if ((new_table = mallocfn(new_len*sizeof(struct type*)))) {         \
+    if ((new_table = reallocarrayfn(NULL, new_len, sizeof(struct type*)))) { \
       unsigned b;                                                       \
       memset(new_table, 0, new_len*sizeof(struct type*));               \
       for (b = 0; b < head->hth_table_length; ++b) {                    \
@@ -356,7 +356,7 @@ ht_string_hash(const char *s)
       head->hth_table = new_table;                                      \
     } else {                                                            \
       unsigned b, b2;                                                   \
-      new_table = reallocfn(head->hth_table, new_len*sizeof(struct type*)); \
+      new_table = reallocarrayfn(head->hth_table, new_len, sizeof(struct type*)); \
       if (!new_table) return -1;                                        \
       memset(new_table + head->hth_table_length, 0,                     \
              (new_len - head->hth_table_length)*sizeof(struct type*));  \
@@ -427,6 +427,21 @@ ht_string_hash(const char *s)
     return 0;                                                           \
   }
 
+#define HT_GENERATE(name, type, field, hashfn, eqfn, load, mallocfn,    \
+                    reallocfn, freefn)                                  \
+  static void *                                                         \
+  name##_reallocarray(void *arg, size_t a, size_t b)                    \
+  {                                                                     \
+    if ((b) && (a) > SIZE_MAX / (b))                                    \
+      return NULL;                                                      \
+    if (arg)                                                            \
+      return reallocfn((arg),(a)*(b));                                  \
+    else                                                                \
+      return mallocfn((a)*(b));                                         \
+  }                                                                     \
+  HT_GENERATE2(name, type, field, hashfn, eqfn, load,                   \
+               name##_reallocarray, freefn)
+
 /** Implements an over-optimized "find and insert if absent" block;
  * not meant for direct usage by typical code, or usage outside the critical
  * path.*/
diff --git a/src/or/channel.c b/src/or/channel.c
index ffd6849..4129839 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -108,8 +108,8 @@ channel_idmap_eq(const channel_idmap_entry_t *a,
 
 HT_PROTOTYPE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash,
              channel_idmap_eq);
-HT_GENERATE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash,
-            channel_idmap_eq, 0.5, tor_malloc, tor_realloc, tor_free_);
+HT_GENERATE2(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash,
+             channel_idmap_eq, 0.5,  tor_reallocarray_, tor_free_);
 
 static cell_queue_entry_t * cell_queue_entry_dup(cell_queue_entry_t *q);
 static void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off);
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 35c5236..9d72ea1 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -94,9 +94,9 @@ static HT_HEAD(chan_circid_map, chan_circid_circuit_map_t)
      chan_circid_map = HT_INITIALIZER();
 HT_PROTOTYPE(chan_circid_map, chan_circid_circuit_map_t, node,
              chan_circid_entry_hash_, chan_circid_entries_eq_)
-HT_GENERATE(chan_circid_map, chan_circid_circuit_map_t, node,
-            chan_circid_entry_hash_, chan_circid_entries_eq_, 0.6,
-            malloc, realloc, free)
+HT_GENERATE2(chan_circid_map, chan_circid_circuit_map_t, node,
+             chan_circid_entry_hash_, chan_circid_entries_eq_, 0.6,
+             tor_reallocarray_, tor_free_)
 
 /** The most recently returned entry from circuit_get_by_circid_chan;
  * used to improve performance when many cells arrive in a row from the
diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c
index e4571ff..3ca33b0 100644
--- a/src/or/circuitmux.c
+++ b/src/or/circuitmux.c
@@ -363,9 +363,9 @@ HT_HEAD(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t);
 /* Emit a bunch of hash table stuff */
 HT_PROTOTYPE(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node,
              chanid_circid_entry_hash, chanid_circid_entries_eq);
-HT_GENERATE(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node,
-            chanid_circid_entry_hash, chanid_circid_entries_eq, 0.6,
-            malloc, realloc, free);
+HT_GENERATE2(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node,
+             chanid_circid_entry_hash, chanid_circid_entries_eq, 0.6,
+             tor_reallocarray_, tor_free_)
 
 /*
  * Circuitmux alloc/free functions
diff --git a/src/or/dns.c b/src/or/dns.c
index a9c4318..362b970 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -244,8 +244,8 @@ cached_resolve_hash(cached_resolve_t *a)
 
 HT_PROTOTYPE(cache_map, cached_resolve_t, node, cached_resolve_hash,
              cached_resolves_eq)
-HT_GENERATE(cache_map, cached_resolve_t, node, cached_resolve_hash,
-            cached_resolves_eq, 0.6, malloc, realloc, free)
+HT_GENERATE2(cache_map, cached_resolve_t, node, cached_resolve_hash,
+             cached_resolves_eq, 0.6, tor_reallocarray_, tor_free_)
 
 /** Initialize the DNS cache. */
 static void
diff --git a/src/or/fp_pair.c b/src/or/fp_pair.c
index 55e4c89..1be1696 100644
--- a/src/or/fp_pair.c
+++ b/src/or/fp_pair.c
@@ -42,9 +42,9 @@ fp_pair_map_entry_hash(const fp_pair_map_entry_t *a)
 
 HT_PROTOTYPE(fp_pair_map_impl, fp_pair_map_entry_s, node,
              fp_pair_map_entry_hash, fp_pair_map_entries_eq)
-HT_GENERATE(fp_pair_map_impl, fp_pair_map_entry_s, node,
-            fp_pair_map_entry_hash, fp_pair_map_entries_eq,
-            0.6, tor_malloc, tor_realloc, tor_free)
+HT_GENERATE2(fp_pair_map_impl, fp_pair_map_entry_s, node,
+             fp_pair_map_entry_hash, fp_pair_map_entries_eq,
+             0.6, tor_reallocarray_, tor_free_)
 
 /** Constructor to create a new empty map from fp_pair_t to void *
  */
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 108385e..cdf2797 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -506,8 +506,8 @@ clientmap_entries_eq(const clientmap_entry_t *a, const clientmap_entry_t *b)
 
 HT_PROTOTYPE(clientmap, clientmap_entry_t, node, clientmap_entry_hash,
              clientmap_entries_eq);
-HT_GENERATE(clientmap, clientmap_entry_t, node, clientmap_entry_hash,
-            clientmap_entries_eq, 0.6, malloc, realloc, free);
+HT_GENERATE2(clientmap, clientmap_entry_t, node, clientmap_entry_hash,
+             clientmap_entries_eq, 0.6, tor_reallocarray_, tor_free_)
 
 /** Free all storage held by <b>ent</b>. */
 static void
@@ -720,8 +720,8 @@ dirreq_map_ent_hash(const dirreq_map_entry_t *entry)
 
 HT_PROTOTYPE(dirreqmap, dirreq_map_entry_t, node, dirreq_map_ent_hash,
              dirreq_map_ent_eq);
-HT_GENERATE(dirreqmap, dirreq_map_entry_t, node, dirreq_map_ent_hash,
-            dirreq_map_ent_eq, 0.6, malloc, realloc, free);
+HT_GENERATE2(dirreqmap, dirreq_map_entry_t, node, dirreq_map_ent_hash,
+             dirreq_map_ent_eq, 0.6, tor_reallocarray_, tor_free_)
 
 /** Helper: Put <b>entry</b> into map of directory requests using
  * <b>type</b> and <b>dirreq_id</b> as key parts. If there is
diff --git a/src/or/microdesc.c b/src/or/microdesc.c
index fdb549a..ed586f3 100644
--- a/src/or/microdesc.c
+++ b/src/or/microdesc.c
@@ -57,9 +57,9 @@ microdesc_eq_(microdesc_t *a, microdesc_t *b)
 
 HT_PROTOTYPE(microdesc_map, microdesc_t, node,
              microdesc_hash_, microdesc_eq_);
-HT_GENERATE(microdesc_map, microdesc_t, node,
+HT_GENERATE2(microdesc_map, microdesc_t, node,
              microdesc_hash_, microdesc_eq_, 0.6,
-             malloc, realloc, free);
+             tor_reallocarray_, tor_free_)
 
 /** Write the body of <b>md</b> into <b>f</b>, with appropriate annotations.
  * On success, return the total number of bytes written, and set
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index 21e4ec2..d7deac1 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -53,8 +53,8 @@ node_id_eq(const node_t *node1, const node_t *node2)
 }
 
 HT_PROTOTYPE(nodelist_map, node_t, ht_ent, node_id_hash, node_id_eq);
-HT_GENERATE(nodelist_map, node_t, ht_ent, node_id_hash, node_id_eq,
-            0.6, malloc, realloc, free);
+HT_GENERATE2(nodelist_map, node_t, ht_ent, node_id_hash, node_id_eq,
+             0.6, tor_reallocarray_, tor_free_)
 
 /** The global nodelist. */
 static nodelist_t *the_nodelist=NULL;
diff --git a/src/or/policies.c b/src/or/policies.c
index 4dbb43e..adc286a 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -629,8 +629,8 @@ policy_hash(const policy_map_ent_t *ent)
 
 HT_PROTOTYPE(policy_map, policy_map_ent_t, node, policy_hash,
              policy_eq)
-HT_GENERATE(policy_map, policy_map_ent_t, node, policy_hash,
-            policy_eq, 0.6, malloc, realloc, free)
+HT_GENERATE2(policy_map, policy_map_ent_t, node, policy_hash,
+             policy_eq, 0.6, tor_reallocarray_, tor_free_)
 
 /** Given a pointer to an addr_policy_t, return a copy of the pointer to the
  * "canonical" copy of that addr_policy_t; the canonical copy is a single
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 2f8ebcb..cd92b0a 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -2724,8 +2724,8 @@ bidi_map_ent_hash(const bidi_map_entry_t *entry)
 
 HT_PROTOTYPE(bidimap, bidi_map_entry_t, node, bidi_map_ent_hash,
              bidi_map_ent_eq);
-HT_GENERATE(bidimap, bidi_map_entry_t, node, bidi_map_ent_hash,
-            bidi_map_ent_eq, 0.6, malloc, realloc, free);
+HT_GENERATE2(bidimap, bidi_map_entry_t, node, bidi_map_ent_hash,
+             bidi_map_ent_eq, 0.6, tor_reallocarray_, tor_free_)
 
 /* DOCDOC bidi_map_free */
 static void



More information about the tor-commits mailing list