commit dad5eb7e1f3c92974c7bcaaea325a05233a27494 Author: Karsten Loesing <karsten.loesing@gmx.net> Date: Fri Feb 13 11:32:35 2015 +0100 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 | 19 +++++----- src/common/util.c | 10 +++--- src/test/test_util.c | 86 ++++++++++++++++++++++---------------------- 3 files changed, 56 insertions(+), 59 deletions(-) diff --git a/changes/laplace-edge-cases b/changes/laplace-edge-cases index 266b119..6c8c77b 100644 --- a/changes/laplace-edge-cases +++ b/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. + diff --git a/src/common/util.c b/src/common/util.c index 3e680d2..d8da8b1 100644 --- a/src/common/util.c +++ b/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, diff --git a/src/test/test_util.c b/src/test/test_util.c index 2f05ce8..3310316 100644 --- a/src/test/test_util.c +++ b/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));