Tweak teor's and dgoulet's #13192 patches.

 - Rewrite changes file.
 - Avoid float comparison with == and use <= instead.
 - Add teor's tor_llround(trunc(...)) back to silence clang warnings.
 - Replace tt_assert() with tt_i64_op() and friends.
 - Fix whitespace and a comment.

changes/laplace-edge-cases

@@ -1,11 +1,8 @@
-  o Minor bugfixes:
-    - Handle edge cases in the laplace functions:
-       * avoid division by zero
-       * avoid taking the log of zero
-       * silence clang type conversion warnings using round and trunc
-       * consistently check for overflow in round_*_to_next_multiple_of
-    - Add tests for laplace edge cases:
-       * check add_laplace_noise with maximal values
-       * check round_*_to_next_multiple_of with additional values
-       * check round_*_to_next_multiple_of with maximal values
-      Related to HS stats in #13192.
+  o Code simplifications and unit tests:
+    - Handle edge cases in the laplace functions: avoid division by zero,
+      avoid taking the log of zero, and silence clang type conversion
+      warnings using round and trunc.  Add unit tests for edge cases with
+      maximal values.
+    - Consistently check for overflow in round_*_to_next_multiple_of
+      functions, and add unit tests with additional and maximal values.
+

src/common/util.c

@@ -546,26 +546,26 @@ sample_laplace_distribution(double mu, double b, double p)
 
   /* This is the "inverse cumulative distribution function" from:
    * http://en.wikipedia.org/wiki/Laplace_distribution */
-  if (p == 0.0) {
+  if (p <= 0.0) {
     /* Avoid taking log(0.0) == -INFINITY, as some processors or compiler
      * options can cause the program to trap. */
     return INT64_MIN;
   }
 
-  result =  mu - b * (p > 0.5 ? 1.0 : -1.0)
-                   * tor_mathlog(1.0 - 2.0 * fabs(p - 0.5));
+  result = mu - b * (p > 0.5 ? 1.0 : -1.0)
+                  * tor_mathlog(1.0 - 2.0 * fabs(p - 0.5));
   if (result >= INT64_MAX)
     return INT64_MAX;
   else if (result <= INT64_MIN)
     return INT64_MIN;
   else
-    return (int64_t) result;
+    return tor_llround(trunc(result));
 }
 
 /** Add random noise between INT64_MIN and INT64_MAX coming from a Laplace
  * distribution with mu = 0 and b = <b>delta_f</b>/<b>epsilon</b> to
  * <b>signal</b> based on the provided <b>random</b> value in [0.0, 1.0[.
- * The epislon value must be between ]0.0, 1.0]. delta_f must be greater
+ * The epsilon value must be between ]0.0, 1.0]. delta_f must be greater
  * than 0. */
 int64_t
 add_laplace_noise(int64_t signal, double random, double delta_f,

src/test/test_util.c

@@ -4055,11 +4055,11 @@ test_util_round_to_next_multiple_of(void *arg)
   tt_u64_op(round_uint64_to_next_multiple_of(99,7), ==, 105);
   tt_u64_op(round_uint64_to_next_multiple_of(99,9), ==, 99);
 
-  tt_assert(round_uint64_to_next_multiple_of(UINT64_MAX,2) ==
+  tt_u64_op(round_uint64_to_next_multiple_of(UINT64_MAX,2), ==,
             UINT64_MAX-UINT64_MAX%2);
 
-  tt_assert(round_int64_to_next_multiple_of(0,1) == 0);
-  tt_assert(round_int64_to_next_multiple_of(0,7) == 0);
+  tt_i64_op(round_int64_to_next_multiple_of(0,1), ==, 0);
+  tt_i64_op(round_int64_to_next_multiple_of(0,7), ==, 0);
 
   tt_i64_op(round_int64_to_next_multiple_of(99,1), ==, 99);
   tt_i64_op(round_int64_to_next_multiple_of(99,7), ==, 105);
@@ -4073,24 +4073,24 @@ test_util_round_to_next_multiple_of(void *arg)
   tt_i64_op(round_int64_to_next_multiple_of(INT64_MAX,2), ==,
                                             INT64_MAX-INT64_MAX%2);
 
-  tt_assert(round_uint32_to_next_multiple_of(0,1) == 0);
-  tt_assert(round_uint32_to_next_multiple_of(0,7) == 0);
+  tt_int_op(round_uint32_to_next_multiple_of(0,1), ==, 0);
+  tt_int_op(round_uint32_to_next_multiple_of(0,7), ==, 0);
 
-  tt_assert(round_uint32_to_next_multiple_of(99,1) == 99);
-  tt_assert(round_uint32_to_next_multiple_of(99,7) == 105);
-  tt_assert(round_uint32_to_next_multiple_of(99,9) == 99);
+  tt_int_op(round_uint32_to_next_multiple_of(99,1), ==, 99);
+  tt_int_op(round_uint32_to_next_multiple_of(99,7), ==, 105);
+  tt_int_op(round_uint32_to_next_multiple_of(99,9), ==, 99);
 
-  tt_assert(round_uint32_to_next_multiple_of(UINT32_MAX,2) ==
+  tt_int_op(round_uint32_to_next_multiple_of(UINT32_MAX,2), ==,
             UINT32_MAX-UINT32_MAX%2);
 
-  tt_assert(round_to_next_multiple_of(0,1) == 0);
-  tt_assert(round_to_next_multiple_of(0,7) == 0);
+  tt_uint_op(round_to_next_multiple_of(0,1), ==, 0);
+  tt_uint_op(round_to_next_multiple_of(0,7), ==, 0);
 
-  tt_assert(round_to_next_multiple_of(99,1) == 99);
-  tt_assert(round_to_next_multiple_of(99,7) == 105);
-  tt_assert(round_to_next_multiple_of(99,9) == 99);
+  tt_uint_op(round_to_next_multiple_of(99,1), ==, 99);
+  tt_uint_op(round_to_next_multiple_of(99,7), ==, 105);
+  tt_uint_op(round_to_next_multiple_of(99,9), ==, 99);
 
-  tt_assert(round_to_next_multiple_of(UINT_MAX,2) ==
+  tt_uint_op(round_to_next_multiple_of(UINT_MAX,2), ==,
             UINT_MAX-UINT_MAX%2);
  done:
   ;
@@ -4125,12 +4125,12 @@ test_util_laplace(void *arg)
   tt_i64_op(INT64_MIN + 20, ==,
             add_laplace_noise(20, 0.0, delta_f, epsilon));
 
-  tt_assert(-60 == add_laplace_noise(20, 0.1, delta_f, epsilon));
-  tt_assert(-14 == add_laplace_noise(20, 0.25, delta_f, epsilon));
-  tt_assert(20 == add_laplace_noise(20, 0.5, delta_f, epsilon));
-  tt_assert(54 == add_laplace_noise(20, 0.75, delta_f, epsilon));
-  tt_assert(100 == add_laplace_noise(20, 0.9, delta_f, epsilon));
-  tt_assert(215 == add_laplace_noise(20, 0.99, delta_f, epsilon));
+  tt_i64_op(-60, ==, add_laplace_noise(20, 0.1, delta_f, epsilon));
+  tt_i64_op(-14, ==, add_laplace_noise(20, 0.25, delta_f, epsilon));
+  tt_i64_op(20, ==, add_laplace_noise(20, 0.5, delta_f, epsilon));
+  tt_i64_op(54, ==, add_laplace_noise(20, 0.75, delta_f, epsilon));
+  tt_i64_op(100, ==, add_laplace_noise(20, 0.9, delta_f, epsilon));
+  tt_i64_op(215, ==, add_laplace_noise(20, 0.99, delta_f, epsilon));
 
   /* Test extreme values of signal with maximally negative values of noise
    * 1.0000000000000002 is the smallest number > 1
@@ -4143,54 +4143,54 @@ test_util_laplace(void *arg)
    */
   const double noscale_df = 1.0, noscale_eps = 1.0;
 
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(0, 0.0, noscale_df, noscale_eps));
 
   /* is it clipped to INT64_MIN? */
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(-1, 0.0, noscale_df, noscale_eps));
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(INT64_MIN, 0.0,
                               noscale_df, noscale_eps));
   /* ... even when scaled? */
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(0, 0.0, delta_f, epsilon));
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(0, 0.0,
                               DBL_MAX, 1));
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(INT64_MIN, 0.0,
                               DBL_MAX, 1));
 
   /* does it play nice with INT64_MAX? */
-  tt_assert((INT64_MIN + INT64_MAX) ==
+  tt_i64_op((INT64_MIN + INT64_MAX), ==,
             add_laplace_noise(INT64_MAX, 0.0,
                               noscale_df, noscale_eps));
 
   /* do near-zero fractional values work? */
   const double min_dbl_error = 0.0000000000000002;
 
-  tt_assert(-35 ==
+  tt_i64_op(-35, ==,
             add_laplace_noise(0, min_dbl_error,
                               noscale_df, noscale_eps));
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(INT64_MIN, min_dbl_error,
                               noscale_df, noscale_eps));
-  tt_assert((-35 + INT64_MAX) ==
+  tt_i64_op((-35 + INT64_MAX), ==,
             add_laplace_noise(INT64_MAX, min_dbl_error,
                               noscale_df, noscale_eps));
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(0, min_dbl_error,
                               DBL_MAX, 1));
-  tt_assert((INT64_MAX + INT64_MIN) ==
+  tt_i64_op((INT64_MAX + INT64_MIN), ==,
             add_laplace_noise(INT64_MAX, min_dbl_error,
                               DBL_MAX, 1));
-  tt_assert(INT64_MIN ==
+  tt_i64_op(INT64_MIN, ==,
             add_laplace_noise(INT64_MIN, min_dbl_error,
                               DBL_MAX, 1));
 
   /* does it play nice with INT64_MAX? */
-  tt_assert((INT64_MAX - 35) ==
+  tt_i64_op((INT64_MAX - 35), ==,
             add_laplace_noise(INT64_MAX, min_dbl_error,
                               noscale_df, noscale_eps));
 
@@ -4205,31 +4205,31 @@ test_util_laplace(void *arg)
   const double max_dbl_lt_one = 0.9999999999999998;
 
   /* do near-one fractional values work? */
-  tt_assert(35 ==
+  tt_i64_op(35, ==,
             add_laplace_noise(0, max_dbl_lt_one, noscale_df, noscale_eps));
 
   /* is it clipped to INT64_MAX? */
-  tt_assert(INT64_MAX ==
+  tt_i64_op(INT64_MAX, ==,
             add_laplace_noise(INT64_MAX - 35, max_dbl_lt_one,
                               noscale_df, noscale_eps));
-  tt_assert(INT64_MAX ==
+  tt_i64_op(INT64_MAX, ==,
             add_laplace_noise(INT64_MAX - 34, max_dbl_lt_one,
                               noscale_df, noscale_eps));
-  tt_assert(INT64_MAX ==
+  tt_i64_op(INT64_MAX, ==,
             add_laplace_noise(INT64_MAX, max_dbl_lt_one,
                               noscale_df, noscale_eps));
   /* ... even when scaled? */
-  tt_assert(INT64_MAX ==
+  tt_i64_op(INT64_MAX, ==,
             add_laplace_noise(INT64_MAX, max_dbl_lt_one,
                               delta_f, epsilon));
-  tt_assert((INT64_MIN + INT64_MAX) ==
+  tt_i64_op((INT64_MIN + INT64_MAX), ==,
             add_laplace_noise(INT64_MIN, max_dbl_lt_one,
                               DBL_MAX, 1));
-  tt_assert(INT64_MAX ==
+  tt_i64_op(INT64_MAX, ==,
             add_laplace_noise(INT64_MAX, max_dbl_lt_one,
                               DBL_MAX, 1));
   /* does it play nice with INT64_MIN? */
-  tt_assert((INT64_MIN + 35) ==
+  tt_i64_op((INT64_MIN + 35), ==,
             add_laplace_noise(INT64_MIN, max_dbl_lt_one,
                               noscale_df, noscale_eps));