[PATCH v1] test_schedule: Make all the counting variables unsigned

From: Frank Lichtenheld <[email protected]>

cppcheck reported that "bit_ceil_n <<= 1" is
potentially undefined behavior if bit_ceil_n is
signed. Making it unsigned caused a ripple whereby
all counts became unsigned to avoid weird casts.

Change-Id: I6123c33b1434d77a0bc33dd5ef28da643d086b4b
Signed-off-by: Frank Lichtenheld <[email protected]>
Acked-by: Razvan Cojocaru <[email protected]>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1870
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1870
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <[email protected]>

        
diff --git a/dev-tools/cppcheck-suppression b/dev-tools/cppcheck-suppression
index 8736c2f..59262160f 100644
--- a/dev-tools/cppcheck-suppression
+++ b/dev-tools/cppcheck-suppression
@@ -114,8 +114,6 @@
 # IGN: We reuse the same variable name due to macro usage
 shadowVariable:src/openvpn/options.c:1948
 shadowVariable:src/openvpn/options.c:1966
-# IGN: sure this is theoretically undefined, but works
-shiftNegativeLHS:tests/unit_tests/openvpn/test_schedule.c:183
 # FP: fun:tls_crypt_v2_wrap_unwrap_invalid: cppcheck is confused
 syntaxError:tests/unit_tests/openvpn/test_tls_crypt.c:684
 # FP: this file is never compiled on _WIN32
diff --git a/tests/unit_tests/openvpn/test_schedule.c b/tests/unit_tests/openvpn/test_schedule.c
index 52cd415..2b53d28 100644
--- a/tests/unit_tests/openvpn/test_schedule.c
+++ b/tests/unit_tests/openvpn/test_schedule.c
@@ -78,15 +78,16 @@
  * Recursively check that the treap (btree) is
  * internally consistent.
  */
-int
-schedule_debug_entry(const struct schedule_entry *e, int depth, int *count, struct timeval *least,
+unsigned int
+schedule_debug_entry(const struct schedule_entry *e, unsigned int depth,
+                     unsigned int *count, struct timeval *least,
                      const struct timeval *min, const struct timeval *max)
 {
     struct gc_arena gc = gc_new();
-    int maxdepth = depth;
+    unsigned int maxdepth = depth;
     if (e)
     {
-        int d;
+        unsigned int d;
 
         assert_ptr_not_equal(e, e->lt);
         assert_ptr_not_equal(e, e->gt);
@@ -138,8 +139,8 @@
     return maxdepth;
 }
 
-int
-schedule_debug(struct schedule *s, int *count, struct timeval *least)
+unsigned int
+schedule_debug(struct schedule *s, unsigned int *count, struct timeval *least)
 {
     struct timeval min;
     struct timeval max;
@@ -164,20 +165,20 @@
 }
 
 void
-schedule_verify(struct schedule *s, int n)
+schedule_verify(struct schedule *s, unsigned int n)
 {
     struct gc_arena gc = gc_new();
     struct timeval least;
 
     least.tv_sec = least.tv_usec = 0x7FFFFFFF;
 
-    int count = 0;
-    int maxlev = schedule_debug(s, &count, &least);
+    unsigned int count = 0;
+    unsigned int maxlev = schedule_debug(s, &count, &least);
 
     /* a stupid algorithm to do C23 stdc_bit_ceil_ui/stdc_bit_width
      * calculate roundup(log2 n) */
-    int bit_ceil_n = 1;
-    int log2n = 0;
+    unsigned int bit_ceil_n = 1;
+    unsigned int log2n = 0;
     while (bit_ceil_n < n)
     {
         bit_ceil_n <<= 1;
@@ -198,12 +199,11 @@
 }
 
 void
-schedule_randomize_array(struct schedule_entry **array, int size)
+schedule_randomize_array(struct schedule_entry **array, unsigned int size)
 {
-    int i;
-    for (i = 0; i < size; ++i)
+    for (unsigned int i = 0; i < size; ++i)
     {
-        const int src = rand() % size;
+        const unsigned int src = (unsigned int)rand() % size;
         struct schedule_entry *tmp = array[i];
         if (i != src)
         {
@@ -214,11 +214,10 @@
 }
 
 void
-schedule_print_work(struct schedule_entry *e, int indent)
+schedule_print_work(struct schedule_entry *e, unsigned int indent)
 {
     struct gc_arena gc = gc_new();
-    int i;
-    for (i = 0; i < indent; ++i)
+    for (unsigned int i = 0; i < indent; ++i)
     {
         printf(" ");
     }
@@ -248,17 +247,16 @@
 schedule_test(void **state)
 {
     struct gc_arena gc = gc_new();
-    int n = 1000;
-    int n_mod = 25;
+    unsigned int n = 1000;
+    unsigned int n_mod = 25;
 
-    int i, j;
     struct schedule_entry **array;
     struct schedule *s = schedule_init();
     struct schedule_entry *e;
 
     ALLOC_ARRAY(array, struct schedule_entry *, n);
 
-    for (i = 0; i < n; ++i)
+    for (unsigned int i = 0; i < n; ++i)
     {
         ALLOC_OBJ_CLEAR(array[i], struct schedule_entry);
         tv_randomize(&array[i]->tv);
@@ -272,11 +270,11 @@
     /*schedule_print (s);*/
     schedule_verify(s, n);
 
-    for (j = 1; j <= n_mod; ++j)
+    for (unsigned int j = 1; j <= n_mod; ++j)
     {
         /*printf("Modification Phase Pass %d\n", j);*/
 
-        for (i = 0; i < n; ++i)
+        for (unsigned int i = 0; i < n; ++i)
         {
             e = schedule_find_earliest_wakeup(s);
             /*printf ("BEFORE %s\n", tv_string (&e->tv, &gc));*/
@@ -300,7 +298,7 @@
     schedule_verify(s, 0);
     assert_null(s->root);
 
-    for (i = 0; i < n; ++i)
+    for (unsigned int i = 0; i < n; ++i)
     {
         free(array[i]);
     }