Merge branch 'calloc2'

12 files changed, 77 insertions, 67 deletions

diff --git a/scripts/coccinelle/calloc.cocci b/scripts/coccinelle/calloc.cocci
index 8a295eb4fd..fbda88e538 100644
--- a/scripts/coccinelle/calloc.cocci
+++ b/scripts/coccinelle/calloc.cocci
@@ -1,16 +1,19 @@
 // Use calloc or realloc as appropriate instead of multiply-and-alloc
 
 @malloc_to_calloc@
-expression a,b;
+identifier f =~ "(tor_malloc|tor_malloc_zero)";
+expression a;
+constant b;
 @@
-- tor_malloc(a * b)
+- f(a * b)
 + tor_calloc(a, b)
 
-@malloc_zero_to_calloc@
-expression a, b;
+@calloc_arg_order@
+expression a;
+type t;
 @@
-- tor_malloc_zero(a * b)
-+ tor_calloc(a, b)
+- tor_calloc(sizeof(t), a)
++ tor_calloc(a, sizeof(t))
 
 @realloc_to_reallocarray@
 expression a, b;
diff --git a/src/common/compat.c b/src/common/compat.c
index e4758aaf88..8574bd04c9 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -1697,7 +1697,7 @@ log_credential_status(void)
 
   /* log supplementary groups */
   sup_gids_size = 64;
-  sup_gids = tor_calloc(sizeof(gid_t), 64);
+  sup_gids = tor_calloc(64, sizeof(gid_t));
   while ((ngids = getgroups(sup_gids_size, sup_gids)) < 0 &&
          errno == EINVAL &&
          sup_gids_size < NGROUPS_MAX) {
diff --git a/src/common/util.c b/src/common/util.c
index 2371ad3649..77102837db 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -195,33 +195,40 @@ tor_malloc_zero_(size_t size DMALLOC_PARAMS)
   return result;
 }
 
+/* The square root of SIZE_MAX + 1.  If a is less than this, and b is less
+ * than this, then a*b is less than SIZE_MAX.  (For example, if size_t is
+ * 32 bits, then SIZE_MAX is 0xffffffff and this value is 0x10000.  If a and
+ * b are less than this, then their product is at most (65535*65535) ==
+ * 0xfffe0001. */
+#define SQRT_SIZE_MAX_P1 (((size_t)1) << (sizeof(size_t)*4))
+
+/** Return non-zero if and only if the product of the arguments is exact. */
+static INLINE int
+size_mul_check(const size_t x, const size_t y)
+{
+  /* This first check is equivalent to
+     (x < SQRT_SIZE_MAX_P1 && y < SQRT_SIZE_MAX_P1)
+
+     Rationale: if either one of x or y is >= SQRT_SIZE_MAX_P1, then it
+     will have some bit set in its most significant half.
+   */
+  return ((x|y) < SQRT_SIZE_MAX_P1 ||
+          y == 0 ||
+          x <= SIZE_MAX / y);
+}
+
 /** Allocate a chunk of <b>nmemb</b>*<b>size</b> bytes of memory, fill
  * the memory with zero bytes, and return a pointer to the result.
  * Log and terminate the process on error.  (Same as
  * calloc(<b>nmemb</b>,<b>size</b>), but never returns NULL.)
- *
- * XXXX This implementation probably asserts in cases where it could
- * work, because it only tries dividing SIZE_MAX by size (according to
- * the calloc(3) man page, the size of an element of the nmemb-element
- * array to be allocated), not by nmemb (which could in theory be
- * smaller than size).  Don't do that then.
+ * The second argument (<b>size</b>) should preferably be non-zero
+ * and a compile-time constant.
  */
 void *
 tor_calloc_(size_t nmemb, size_t size DMALLOC_PARAMS)
 {
-  /* You may ask yourself, "wouldn't it be smart to use calloc instead of
-   * malloc+memset?  Perhaps libc's calloc knows some nifty optimization trick
-   * we don't!"  Indeed it does, but its optimizations are only a big win when
-   * we're allocating something very big (it knows if it just got the memory
-   * from the OS in a pre-zeroed state).  We don't want to use tor_malloc_zero
-   * for big stuff, so we don't bother with calloc. */
-  void *result;
-  size_t max_nmemb = (size == 0) ? SIZE_MAX : SIZE_MAX/size;
-
-  tor_assert(nmemb < max_nmemb);
-
-  result = tor_malloc_zero_((nmemb * size) DMALLOC_FN_ARGS);
-  return result;
+  tor_assert(size_mul_check(nmemb, size));
+  return tor_malloc_zero_((nmemb * size) DMALLOC_FN_ARGS);
 }
 
 /** Change the size of the memory block pointed to by <b>ptr</b> to <b>size</b>
@@ -264,7 +271,7 @@ tor_reallocarray_(void *ptr, size_t sz1, size_t sz2 DMALLOC_PARAMS)
 {
   /* XXXX we can make this return 0, but we would need to check all the
    * reallocarray users. */
-  tor_assert(sz2 == 0 || sz1 < SIZE_T_CEILING / sz2);
+  tor_assert(size_mul_check(sz1, sz2));
 
   return tor_realloc(ptr, (sz1 * sz2) DMALLOC_FN_ARGS);
 }
@@ -3458,8 +3465,8 @@ format_win_cmdline_argument(const char *arg)
     smartlist_add(arg_chars, (void*)&backslash);
 
   /* Allocate space for argument, quotes (if needed), and terminator */
-  formatted_arg = tor_calloc(sizeof(char),
-                    (smartlist_len(arg_chars) + (need_quotes ? 2 : 0) + 1));
+  formatted_arg = tor_calloc((smartlist_len(arg_chars) + (need_quotes ? 2 : 0) + 1),
+                             sizeof(char));
 
   /* Add leading quote */
   i=0;
@@ -5097,7 +5104,7 @@ tor_check_port_forwarding(const char *filename,
        for each smartlist element (one for "-p" and one for the
        ports), and one for the final NULL. */
     args_n = 1 + 2*smartlist_len(ports_to_forward) + 1;
-    argv = tor_calloc(sizeof(char *), args_n);
+    argv = tor_calloc(args_n, sizeof(char *));
 
     argv[argv_index++] = filename;
     SMARTLIST_FOREACH_BEGIN(ports_to_forward, const char *, port) {
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 44946d389c..42c4870e87 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1548,7 +1548,7 @@ choose_good_exit_server_general(int need_uptime, int need_capacity)
    * -1 means "Don't use this router at all."
    */
   the_nodes = nodelist_get_list();
-  n_supported = tor_calloc(sizeof(int), smartlist_len(the_nodes));
+  n_supported = tor_calloc(smartlist_len(the_nodes), sizeof(int));
   SMARTLIST_FOREACH_BEGIN(the_nodes, const node_t *, node) {
     const int i = node_sl_idx;
     if (router_digest_is_me(node->identity)) {
diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 956c969db6..a136278e58 100644
--- a/src/or/circuitstats.c
+++ b/src/or/circuitstats.c
@@ -404,7 +404,7 @@ circuit_build_times_new_consensus_params(circuit_build_times_t *cbt,
          * distress anyway, so memory correctness here is paramount over
          * doing acrobatics to preserve the array.
          */
-        recent_circs = tor_calloc(sizeof(int8_t), num);
+        recent_circs = tor_calloc(num, sizeof(int8_t));
         if (cbt->liveness.timeouts_after_firsthop &&
             cbt->liveness.num_recent_circs > 0) {
           memcpy(recent_circs, cbt->liveness.timeouts_after_firsthop,
@@ -508,7 +508,7 @@ circuit_build_times_init(circuit_build_times_t *cbt)
     cbt->liveness.num_recent_circs =
       circuit_build_times_recent_circuit_count(NULL);
     cbt->liveness.timeouts_after_firsthop =
-      tor_calloc(sizeof(int8_t), cbt->liveness.num_recent_circs);
+      tor_calloc(cbt->liveness.num_recent_circs, sizeof(int8_t));
   } else {
     cbt->liveness.num_recent_circs = 0;
     cbt->liveness.timeouts_after_firsthop = NULL;
@@ -873,7 +873,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt,
   }
 
   /* build_time_t 0 means uninitialized */
-  loaded_times = tor_calloc(sizeof(build_time_t), state->TotalBuildTimes);
+  loaded_times = tor_calloc(state->TotalBuildTimes, sizeof(build_time_t));
 
   for (line = state->BuildtimeHistogram; line; line = line->next) {
     smartlist_t *args = smartlist_new();
diff --git a/src/or/config.c b/src/or/config.c
index 3f31e876dd..681955e7eb 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -4856,7 +4856,7 @@ parse_client_transport_line(const or_options_t *options,
     if (!validate_only && !is_useless_proxy) {
       proxy_argc = line_length-2;
       tor_assert(proxy_argc > 0);
-      proxy_argv = tor_calloc(sizeof(char *), (proxy_argc + 1));
+      proxy_argv = tor_calloc((proxy_argc + 1), sizeof(char *));
       tmp = proxy_argv;
       for (i=0;i<proxy_argc;i++) { /* store arguments */
         *tmp++ = smartlist_get(items, 2);
@@ -5143,7 +5143,7 @@ parse_server_transport_line(const or_options_t *options,
     if (!validate_only) {
       proxy_argc = line_length-2;
       tor_assert(proxy_argc > 0);
-      proxy_argv = tor_calloc(sizeof(char *), (proxy_argc + 1));
+      proxy_argv = tor_calloc((proxy_argc + 1), sizeof(char *));
       tmp = proxy_argv;
 
       for (i=0;i<proxy_argc;i++) { /* store arguments */
diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c
index 1fe472d18c..568d9e42d8 100644
--- a/src/or/cpuworker.c
+++ b/src/or/cpuworker.c
@@ -510,7 +510,7 @@ spawn_cpuworker(void)
   connection_t *conn;
   int err;
 
-  fdarray = tor_calloc(sizeof(tor_socket_t), 2);
+  fdarray = tor_calloc(2, sizeof(tor_socket_t));
   if ((err = tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fdarray)) < 0) {
     log_warn(LD_NET, "Couldn't construct socketpair for cpuworker: %s",
              tor_socket_strerror(-err));
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index ec9af07ecb..730f005a96 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -1369,18 +1369,18 @@ dirserv_compute_performance_thresholds(routerlist_t *rl,
    * sort them and use that to compute thresholds. */
   n_active = n_active_nonexit = 0;
   /* Uptime for every active router. */
-  uptimes = tor_calloc(sizeof(uint32_t), smartlist_len(rl->routers));
+  uptimes = tor_calloc(smartlist_len(rl->routers), sizeof(uint32_t));
   /* Bandwidth for every active router. */
-  bandwidths_kb = tor_calloc(sizeof(uint32_t), smartlist_len(rl->routers));
+  bandwidths_kb = tor_calloc(smartlist_len(rl->routers), sizeof(uint32_t));
   /* Bandwidth for every active non-exit router. */
   bandwidths_excluding_exits_kb =
-    tor_calloc(sizeof(uint32_t), smartlist_len(rl->routers));
+    tor_calloc(smartlist_len(rl->routers), sizeof(uint32_t));
   /* Weighted mean time between failure for each active router. */
-  mtbfs = tor_calloc(sizeof(double), smartlist_len(rl->routers));
+  mtbfs = tor_calloc(smartlist_len(rl->routers), sizeof(double));
   /* Time-known for each active router. */
-  tks = tor_calloc(sizeof(long), smartlist_len(rl->routers));
+  tks = tor_calloc(smartlist_len(rl->routers), sizeof(long));
   /* Weighted fractional uptime for each active router. */
-  wfus = tor_calloc(sizeof(double), smartlist_len(rl->routers));
+  wfus = tor_calloc(smartlist_len(rl->routers), sizeof(double));
 
   nodelist_assert_ok();
 
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index 76dce6c6be..39505a4f9e 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -611,7 +611,7 @@ dirvote_compute_params(smartlist_t *votes, int method, int total_authorities)
      between INT32_MIN and INT32_MAX inclusive.  This should be guaranteed by
      the parsing code. */
 
-  vals = tor_calloc(sizeof(int), n_votes);
+  vals = tor_calloc(n_votes, sizeof(int));
 
   SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, v) {
     if (!v->net_params)
@@ -1258,10 +1258,10 @@ networkstatus_compute_consensus(smartlist_t *votes,
     smartlist_t *chosen_flags = smartlist_new();
     smartlist_t *versions = smartlist_new();
     smartlist_t *exitsummaries = smartlist_new();
-    uint32_t *bandwidths_kb = tor_calloc(sizeof(uint32_t),
-                                         smartlist_len(votes));
-    uint32_t *measured_bws_kb = tor_calloc(sizeof(uint32_t),
-                                           smartlist_len(votes));
+    uint32_t *bandwidths_kb = tor_calloc(smartlist_len(votes),
+                                         sizeof(uint32_t));
+    uint32_t *measured_bws_kb = tor_calloc(smartlist_len(votes),
+                                           sizeof(uint32_t));
     int num_bandwidths;
     int num_mbws;
 
@@ -1281,13 +1281,13 @@ networkstatus_compute_consensus(smartlist_t *votes,
     memset(conflict, 0, sizeof(conflict));
     memset(unknown, 0xff, sizeof(conflict));
 
-    index = tor_calloc(sizeof(int), smartlist_len(votes));
-    size = tor_calloc(sizeof(int), smartlist_len(votes));
-    n_voter_flags = tor_calloc(sizeof(int), smartlist_len(votes));
-    n_flag_voters = tor_calloc(sizeof(int), smartlist_len(flags));
-    flag_map = tor_calloc(sizeof(int *), smartlist_len(votes));
-    named_flag = tor_calloc(sizeof(int), smartlist_len(votes));
-    unnamed_flag = tor_calloc(sizeof(int), smartlist_len(votes));
+    index = tor_calloc(smartlist_len(votes), sizeof(int));
+    size = tor_calloc(smartlist_len(votes), sizeof(int));
+    n_voter_flags = tor_calloc(smartlist_len(votes), sizeof(int));
+    n_flag_voters = tor_calloc(smartlist_len(flags), sizeof(int));
+    flag_map = tor_calloc(smartlist_len(votes), sizeof(int *));
+    named_flag = tor_calloc(smartlist_len(votes), sizeof(int));
+    unnamed_flag = tor_calloc(smartlist_len(votes), sizeof(int));
     for (i = 0; i < smartlist_len(votes); ++i)
       unnamed_flag[i] = named_flag[i] = -1;
 
@@ -1298,8 +1298,8 @@ networkstatus_compute_consensus(smartlist_t *votes,
      * that they're actually set before doing U64_LITERAL(1) << index with
      * them.*/
     SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, v) {
-      flag_map[v_sl_idx] = tor_calloc(sizeof(int),
-                                      smartlist_len(v->known_flags));
+      flag_map[v_sl_idx] = tor_calloc(smartlist_len(v->known_flags),
+                                      sizeof(int));
       if (smartlist_len(v->known_flags) > MAX_KNOWN_FLAGS_IN_VOTE) {
         log_warn(LD_BUG, "Somehow, a vote has %d entries in known_flags",
                  smartlist_len(v->known_flags));
@@ -1379,7 +1379,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
     );
 
     /* Now go through all the votes */
-    flag_counts = tor_calloc(sizeof(int), smartlist_len(flags));
+    flag_counts = tor_calloc(smartlist_len(flags), sizeof(int));
     while (1) {
       vote_routerstatus_t *rs;
       routerstatus_t rs_out;
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 75b26212d0..c02343d489 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -963,7 +963,7 @@ geoip_get_dirreq_history(dirreq_type_t type)
     /* We may have rounded 'completed' up.  Here we want to use the
      * real value. */
     complete = smartlist_len(dirreq_completed);
-    dltimes = tor_calloc(sizeof(uint32_t), complete);
+    dltimes = tor_calloc(complete, sizeof(uint32_t));
     SMARTLIST_FOREACH_BEGIN(dirreq_completed, dirreq_map_entry_t *, ent) {
       uint32_t bytes_per_second;
       uint32_t time_diff = (uint32_t) tv_mdiff(&ent->request_time,
@@ -1033,7 +1033,7 @@ geoip_get_client_history(geoip_client_action_t action,
   if (!geoip_is_loaded(AF_INET) && !geoip_is_loaded(AF_INET6))
     return -1;
 
-  counts = tor_calloc(sizeof(unsigned), n_countries);
+  counts = tor_calloc(n_countries, sizeof(unsigned));
   HT_FOREACH(ent, clientmap, &client_history) {
     int country;
     if ((*ent)->action != (int)action)
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 214b7c483f..d81dae4676 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1536,7 +1536,7 @@ dirserver_choose_by_weight(const smartlist_t *servers, double authority_weight)
   u64_dbl_t *weights;
   const dir_server_t *ds;
 
-  weights = tor_calloc(sizeof(u64_dbl_t), n);
+  weights = tor_calloc(n, sizeof(u64_dbl_t));
   for (i = 0; i < n; ++i) {
     ds = smartlist_get(servers, i);
     weights[i].dbl = ds->weight;
@@ -2045,7 +2045,7 @@ compute_weighted_bandwidths(const smartlist_t *sl,
   Web /= weight_scale;
   Wdb /= weight_scale;
 
-  bandwidths = tor_calloc(sizeof(u64_dbl_t), smartlist_len(sl));
+  bandwidths = tor_calloc(smartlist_len(sl), sizeof(u64_dbl_t));
 
   // Cycle through smartlist and total the bandwidth.
   SMARTLIST_FOREACH_BEGIN(sl, const node_t *, node) {
@@ -2192,7 +2192,7 @@ smartlist_choose_node_by_bandwidth(const smartlist_t *sl,
 
   /* First count the total bandwidth weight, and make a list
    * of each value.  We use UINT64_MAX to indicate "unknown". */
-  bandwidths = tor_calloc(sizeof(u64_dbl_t), smartlist_len(sl));
+  bandwidths = tor_calloc(smartlist_len(sl), sizeof(u64_dbl_t));
   fast_bits = bitarray_init_zero(smartlist_len(sl));
   exit_bits = bitarray_init_zero(smartlist_len(sl));
   guard_bits = bitarray_init_zero(smartlist_len(sl));
@@ -3584,9 +3584,9 @@ routerlist_remove_old_cached_routers_with_id(time_t now,
     n_extra = n - mdpr;
   }
 
-  lifespans = tor_calloc(sizeof(struct duration_idx_t), n);
-  rmv = tor_calloc(sizeof(uint8_t), n);
-  must_keep = tor_calloc(sizeof(uint8_t), n);
+  lifespans = tor_calloc(n, sizeof(struct duration_idx_t));
+  rmv = tor_calloc(n, sizeof(uint8_t));
+  must_keep = tor_calloc(n, sizeof(uint8_t));
   /* Set lifespans to contain the lifespan and index of each server. */
   /* Set rmv[i-lo]=1 if we're going to remove a server for being too old. */
   for (i = lo; i <= hi; ++i) {
diff --git a/src/test/test_pt.c b/src/test/test_pt.c
index 4ca2e32b7b..1be52ee5bd 100644
--- a/src/test/test_pt.c
+++ b/src/test/test_pt.c
@@ -196,7 +196,7 @@ test_pt_protocol(void *arg)
   (void)arg;
   mp->conf_state = PT_PROTO_LAUNCHED;
   mp->transports = smartlist_new();
-  mp->argv = tor_calloc(sizeof(char *), 2);
+  mp->argv = tor_calloc(2, sizeof(char *));
   mp->argv[0] = tor_strdup("<testcase>");
 
   /* various wrong protocol runs: */