[M] Change in openvpn[master]: test_schedule: Make all the counting variables unsigned

"flichtenheld \(Code Review\) via Openvpn-devel" <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <[email protected]>
Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1870?usp=email

to review the following change.


Change subject: test_schedule: Make all the counting variables unsigned
......................................................................

test_schedule: Make all the counting variables unsigned

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]>
---
M dev-tools/cppcheck-suppression
M tests/unit_tests/openvpn/test_schedule.c
2 files changed, 23 insertions(+), 27 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/70/1870/1

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]);
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1870?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6123c33b1434d77a0bc33dd5ef28da643d086b4b
Gerrit-Change-Number: 1870
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.