[Bug 2985] bogus calculation in authkeys.c

 - implement 'auth_log2()' using integer bithack instead of float calculation

bk: 56937534dDSoJL__JyykImzIwQa0AQ

diff --git a/ChangeLog b/ChangeLog
index 304bd85ab7dc7c851bbe3b0704766c8fed26ba78..04b09fab39d503a3555568813b1b51986983fc26 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -47,6 +47,8 @@
               lots of clients. perlinger@ntp.org
 * [Bug 2971] ntpq bails on ^C: select fails: Interrupted system call
   - changed stacked/nested handling of CTRL-C. perlinger@ntp.org
+* [Bug 2985] bogus calculation in authkeys.c perlinger@ntp.org
+  - implement 'auth_log2()' using integer bithack instead of float calculation
 * Unity cleanup for FreeBSD-6.4.  Harlan Stenn.
 * Unity test cleanup.  Harlan Stenn.
 * Libevent autoconf pthread fixes for FreeBSD-10.  Harlan Stenn.

diff --git a/libntp/authkeys.c b/libntp/authkeys.c
index f7462a2463aea2c4091086991c2d6913a8d9a2f5..02725fb1ba533fc107c3f822af8e4c6f933e5b04 100644 (file)
--- a/libntp/authkeys.c
+++ b/libntp/authkeys.c
@@ -48,13 +48,13 @@ struct symkey_alloc_tag {
 symkey_alloc * authallocs;
 #endif /* DEBUG */
 
-static inline u_short  auth_log2(double x);
-static void            auth_resize_hashtable(void);
-static void            allocsymkey(symkey **, keyid_t, u_short,
-                                   u_short, u_long, u_short, u_char *);
-static void            freesymkey(symkey *, symkey **);
+static u_short auth_log2(size_t);
+static void    auth_resize_hashtable(void);
+static void    allocsymkey(symkey **, keyid_t, u_short,
+                           u_short, u_long, u_short, u_char *);
+static void    freesymkey(symkey *, symkey **);
 #ifdef DEBUG
-static void            free_auth_mem(void);
+static void    free_auth_mem(void);
 #endif
 
 symkey key_listhead;           /* list of all in-use keys */;
@@ -210,10 +210,33 @@ auth_prealloc_symkeys(
 }
 
 
-static inline u_short
-auth_log2(double x)
+static u_short
+auth_log2(size_t x)
 {
-       return (u_short)(log10(x) / log10(2));
+       /*
+       ** bithack to calculate floor(log2(x))
+       **
+       ** This assumes
+       **   - (sizeof(size_t) is a power of two
+       **   - CHAR_BITS is a power of two
+       **   - returning zero for arguments <= 0 is OK.
+       **
+       ** Does only shifts, masks and sums in integer arithmetic in
+       ** log2(CHAR_BIT*sizeof(size_t)) steps. (that is, 5/6 steps for
+       ** 32bit/64bit size_t)
+       */
+       int     s;
+       int     r = 0;
+       size_t  m = ~(size_t)0;
+
+       for (s = sizeof(size_t) / 2 * CHAR_BIT; s != 0; s >>= 1) {
+               m <<= s;
+               if (x & m)
+                       r += s;
+               else
+                       x <<= s;
+       }
+       return (u_short)r;
 }
 
 
@@ -234,7 +257,7 @@ auth_resize_hashtable(void)
        symkey *        sk;
 
        totalkeys = authnumkeys + authnumfreekeys;
-       hashbits = auth_log2(totalkeys / 4.0) + 1;
+       hashbits = auth_log2(totalkeys / 4) + 1;
        hashbits = max(4, hashbits);
        hashbits = min(15, hashbits);
 

diff --git a/tests/libntp/authkeys.c b/tests/libntp/authkeys.c
index 2ddbce59703bcf5d27b28f2b37cccb4478c79362..bea3fe539a316a5cdeea989767e0e699073b189a 100644 (file)
--- a/tests/libntp/authkeys.c
+++ b/tests/libntp/authkeys.c
@@ -13,6 +13,7 @@
 # include "openssl/rand.h"
 # include "openssl/evp.h"
 #endif
+#include <limits.h>
 
 u_long current_time = 4;
 int counter = 0;
@@ -27,6 +28,7 @@ void test_HaveKeyCorrect(void);
 void test_HaveKeyIncorrect(void);
 void test_AddWithAuthUseKey(void);
 void test_EmptyKey(void);
+void test_auth_log2(void);
 
 
 void
@@ -158,3 +160,39 @@ test_EmptyKey(void)
 
        return;
 }
+
+/* test the implementation of 'auth_log2' -- use a local copy of the code */
+
+static u_short
+auth_log2(
+       size_t x)
+{
+       int     s;
+       int     r = 0;
+       size_t  m = ~(size_t)0;
+
+       for (s = sizeof(size_t) / 2 * CHAR_BIT; s != 0; s >>= 1) {
+               m <<= s;
+               if (x & m)
+                       r += s;
+               else
+                       x <<= s;
+       }
+       return (u_short)r;
+}
+
+void
+test_auth_log2(void)
+{
+       int     l2;
+       size_t  tv;
+       
+       TEST_ASSERT_EQUAL_INT(0, auth_log2(0));
+       TEST_ASSERT_EQUAL_INT(0, auth_log2(1));
+       for (l2 = 1; l2 < sizeof(size_t)*CHAR_BIT; ++l2) {
+               tv = (size_t)1 << l2;
+               TEST_ASSERT_EQUAL_INT(l2, auth_log2(   tv   ));
+               TEST_ASSERT_EQUAL_INT(l2, auth_log2( tv + 1 ));
+               TEST_ASSERT_EQUAL_INT(l2, auth_log2(2*tv - 1));
+       }
+}

diff --git a/tests/libntp/run-authkeys.c b/tests/libntp/run-authkeys.c
index 6a2b67081f9ea84972e601d41879053327d0d55e..cc91876a8735fcd5086c6b061c04c153e691b96d 100644 (file)
--- a/tests/libntp/run-authkeys.c
+++ b/tests/libntp/run-authkeys.c
@@ -26,6 +26,7 @@
 #include "ntp.h"
 #include "ntp_stdlib.h"
 #include "ntp_calendar.h"
+#include <limits.h>
 
 //=======External Functions This Runner Calls=====
 extern void setUp(void);
@@ -36,6 +37,7 @@ extern void test_HaveKeyCorrect(void);
 extern void test_HaveKeyIncorrect(void);
 extern void test_AddWithAuthUseKey(void);
 extern void test_EmptyKey(void);
+extern void test_auth_log2(void);
 
 
 //=======Test Reset Option=====
@@ -54,12 +56,13 @@ int main(int argc, char *argv[])
 {
   progname = argv[0];
   UnityBegin("authkeys.c");
-  RUN_TEST(test_AddTrustedKeys, 24);
-  RUN_TEST(test_AddUntrustedKey, 25);
-  RUN_TEST(test_HaveKeyCorrect, 26);
-  RUN_TEST(test_HaveKeyIncorrect, 27);
-  RUN_TEST(test_AddWithAuthUseKey, 28);
-  RUN_TEST(test_EmptyKey, 29);
+  RUN_TEST(test_AddTrustedKeys, 25);
+  RUN_TEST(test_AddUntrustedKey, 26);
+  RUN_TEST(test_HaveKeyCorrect, 27);
+  RUN_TEST(test_HaveKeyIncorrect, 28);
+  RUN_TEST(test_AddWithAuthUseKey, 29);
+  RUN_TEST(test_EmptyKey, 30);
+  RUN_TEST(test_auth_log2, 31);
 
   return (UnityEnd());
 }