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
