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/+/1119?usp=email
to review the following change.
Change subject: list: Make types of hash elements consistent
......................................................................
list: Make types of hash elements consistent
Really no use in having the indices and limits in int.
Change-Id: I3334465738fb1fbf508dfd719b6a238b500cc0ae
Signed-off-by: Frank Lichtenheld <fr...@lichtenheld.com>
---
M src/openvpn/integer.h
M src/openvpn/list.c
M src/openvpn/list.h
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/options.c
M src/openvpn/options.h
M tests/unit_tests/openvpn/test_misc.c
8 files changed, 63 insertions(+), 50 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1119/1
diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index c568036..e6106c9 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -132,6 +132,27 @@
}
}
+static inline unsigned int
+constrain_uint(unsigned int x, unsigned int min, unsigned int max)
+{
+ if (min > max)
+ {
+ return min;
+ }
+ if (x < min)
+ {
+ return min;
+ }
+ else if (x > max)
+ {
+ return max;
+ }
+ else
+ {
+ return x;
+ }
+}
+
/*
* Functions used for circular buffer index arithmetic.
*/
diff --git a/src/openvpn/list.c b/src/openvpn/list.c
index 1f33b5f..3971d62 100644
--- a/src/openvpn/list.c
+++ b/src/openvpn/list.c
@@ -35,23 +35,22 @@
#include "memdbg.h"
struct hash *
-hash_init(const int n_buckets,
+hash_init(const uint32_t n_buckets,
const uint32_t iv,
uint32_t (*hash_function)(const void *key, uint32_t iv),
bool (*compare_function)(const void *key1, const void *key2))
{
struct hash *h;
- int i;
ASSERT(n_buckets > 0);
ALLOC_OBJ_CLEAR(h, struct hash);
- h->n_buckets = (int) adjust_power_of_2(n_buckets);
+ h->n_buckets = (uint32_t)adjust_power_of_2(n_buckets);
h->mask = h->n_buckets - 1;
h->hash_function = hash_function;
h->compare_function = compare_function;
h->iv = iv;
ALLOC_ARRAY(h->buckets, struct hash_bucket, h->n_buckets);
- for (i = 0; i < h->n_buckets; ++i)
+ for (uint32_t i = 0; i < h->n_buckets; ++i)
{
struct hash_bucket *b = &h->buckets[i];
b->list = NULL;
@@ -62,8 +61,7 @@
void
hash_free(struct hash *hash)
{
- int i;
- for (i = 0; i < hash->n_buckets; ++i)
+ for (uint32_t i = 0; i < hash->n_buckets; ++i)
{
struct hash_bucket *b = &hash->buckets[i];
struct hash_element *he = b->list;
@@ -222,15 +220,15 @@
void
hash_iterator_init_range(struct hash *hash,
struct hash_iterator *hi,
- int start_bucket,
- int end_bucket)
+ uint32_t start_bucket,
+ uint32_t end_bucket)
{
if (end_bucket > hash->n_buckets)
{
end_bucket = hash->n_buckets;
}
- ASSERT(start_bucket >= 0 && start_bucket <= end_bucket);
+ ASSERT(start_bucket <= end_bucket);
hi->hash = hash;
hi->elem = NULL;
@@ -336,6 +334,9 @@
* the return value. Every 1-bit and 2-bit delta achieves avalanche.
* About 36+6len instructions.
*
+ * #define hashsize(n) ((uint32_t)1<<(n))
+ * #define hashmask(n) (hashsize(n)-1)
+ *
* The best hash table sizes are powers of 2. There is no need to do
* mod a prime (mod is sooo slow!). If you need less than 32 bits,
* use a bitmask. For example, if you need only 10 bits, do
diff --git a/src/openvpn/list.h b/src/openvpn/list.h
index 783570f..d2c5373 100644
--- a/src/openvpn/list.h
+++ b/src/openvpn/list.h
@@ -37,14 +37,11 @@
#include "basic.h"
#include "buffer.h"
-#define hashsize(n) ((uint32_t)1<<(n))
-#define hashmask(n) (hashsize(n)-1)
-
struct hash_element
{
void *value;
const void *key;
- unsigned int hash_value;
+ uint32_t hash_value;
struct hash_element *next;
};
@@ -55,16 +52,16 @@
struct hash
{
- int n_buckets;
- int n_elements;
- int mask;
+ uint32_t n_buckets;
+ uint32_t n_elements;
+ uint32_t mask;
uint32_t iv;
uint32_t (*hash_function)(const void *key, uint32_t iv);
bool (*compare_function)(const void *key1, const void *key2); /* return
true if equal */
struct hash_bucket *buckets;
};
-struct hash *hash_init(const int n_buckets,
+struct hash *hash_init(const uint32_t n_buckets,
const uint32_t iv,
uint32_t (*hash_function)(const void *key, uint32_t iv),
bool (*compare_function)(const void *key1, const void
*key2));
@@ -88,19 +85,19 @@
struct hash_iterator
{
struct hash *hash;
- int bucket_index;
+ uint32_t bucket_index;
struct hash_bucket *bucket;
struct hash_element *elem;
struct hash_element *last;
bool bucket_marked;
- int bucket_index_start;
- int bucket_index_end;
+ uint32_t bucket_index_start;
+ uint32_t bucket_index_end;
};
void hash_iterator_init_range(struct hash *hash,
struct hash_iterator *hi,
- int start_bucket,
- int end_bucket);
+ uint32_t start_bucket,
+ uint32_t end_bucket);
void hash_iterator_init(struct hash *hash, struct hash_iterator *iter);
@@ -118,13 +115,13 @@
return (*hash->hash_function)(key, hash->iv);
}
-static inline int
+static inline uint32_t
hash_n_elements(const struct hash *hash)
{
return hash->n_elements;
}
-static inline int
+static inline uint32_t
hash_n_buckets(const struct hash *hash)
{
return hash->n_buckets;
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index c0e8f9c..02157ac 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -175,19 +175,13 @@
static void
multi_reap_range(const struct multi_context *m,
- int start_bucket,
- int end_bucket)
+ uint32_t start_bucket,
+ uint32_t end_bucket)
{
struct gc_arena gc = gc_new();
struct hash_iterator hi;
struct hash_element *he;
- if (start_bucket < 0)
- {
- start_bucket = 0;
- end_bucket = hash_n_buckets(m->vhash);
- }
-
dmsg(D_MULTI_DEBUG, "MULTI: REAP range %d -> %d", start_bucket,
end_bucket);
hash_iterator_init_range(m->vhash, &hi, start_bucket, end_bucket);
while ((he = hash_iterator_next(&hi)) != NULL)
@@ -209,11 +203,11 @@
static void
multi_reap_all(const struct multi_context *m)
{
- multi_reap_range(m, -1, 0);
+ multi_reap_range(m, 0, hash_n_buckets(m->vhash));
}
static struct multi_reap *
-multi_reap_new(int buckets_per_pass)
+multi_reap_new(uint32_t buckets_per_pass)
{
struct multi_reap *mr;
ALLOC_OBJ(mr, struct multi_reap);
@@ -245,10 +239,10 @@
/*
* How many buckets in vhash to reap per pass.
*/
-static int
-reap_buckets_per_pass(int n_buckets)
+static uint32_t
+reap_buckets_per_pass(uint32_t n_buckets)
{
- return constrain_int(n_buckets / REAP_DIVISOR, REAP_MIN, REAP_MAX);
+ return constrain_uint(n_buckets / REAP_DIVISOR, REAP_MIN, REAP_MAX);
}
#ifdef ENABLE_MANAGEMENT
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 3c821d7..a622e8c 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -52,8 +52,8 @@
*/
struct multi_reap
{
- int bucket_base;
- int buckets_per_pass;
+ uint32_t bucket_base;
+ uint32_t buckets_per_pass;
time_t last_call;
};
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d3ca340..9ca2c4a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7588,11 +7588,11 @@
}
else if (streq(p[0], "hash-size") && p[1] && p[2] && !p[3])
{
- int real, virtual;
+ uint32_t real, virtual;
VERIFY_PERMISSION(OPT_P_GENERAL);
- real = atoi_warn(p[1], msglevel);
- virtual = atoi_warn(p[2], msglevel);
+ real = positive_atoi(p[1], msglevel);
+ virtual = positive_atoi(p[2], msglevel);
if (real < 1 || virtual < 1)
{
msg(msglevel, "--hash-size sizes must be >= 1 (preferably a power
of 2)");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 78b0d7c..9542d93 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -498,8 +498,8 @@
struct in6_addr ifconfig_ipv6_pool_base; /* IPv6 */
int ifconfig_ipv6_pool_netbits; /* IPv6 */
- int real_hash_size;
- int virtual_hash_size;
+ uint32_t real_hash_size;
+ uint32_t virtual_hash_size;
const char *client_connect_script;
const char *client_disconnect_script;
const char *learn_address_script;
diff --git a/tests/unit_tests/openvpn/test_misc.c
b/tests/unit_tests/openvpn/test_misc.c
index 3272f6b..b1e0bef 100644
--- a/tests/unit_tests/openvpn/test_misc.c
+++ b/tests/unit_tests/openvpn/test_misc.c
@@ -123,7 +123,7 @@
word_hash_function(const void *key, uint32_t iv)
{
const char *str = (const char *) key;
- const int len = strlen(str);
+ const uint32_t len = (uint32_t)strlen(str);
return hash_func((const uint8_t *)str, len, iv);
}
@@ -133,11 +133,11 @@
return strcmp((const char *)key1, (const char *)key2) == 0;
}
-static unsigned long
+static uint32_t
get_random(void)
{
/* rand() is not very random, but it's C99 and this is just for testing */
- return rand();
+ return (uint32_t)rand();
}
static struct hash_element *
@@ -172,7 +172,7 @@
struct hash *hash = hash_init(10000, get_random(), word_hash_function,
word_compare_function);
struct hash *nhash = hash_init(256, get_random(), word_hash_function,
word_compare_function);
- printf("hash_init n_buckets=%d mask=0x%08x\n", hash->n_buckets,
hash->mask);
+ printf("hash_init n_buckets=%u mask=0x%08x\n", hash->n_buckets,
hash->mask);
char wordfile[PATH_MAX] = { 0 };
openvpn_test_get_srcdir_dir(wordfile, PATH_MAX, "/../../../COPYRIGHT.GPL"
);
@@ -250,10 +250,10 @@
/* output contents of hash table */
{
- ptr_type inc = 0;
+ uint32_t inc = 0;
int count = 0;
- for (ptr_type base = 0; base < hash_n_buckets(hash); base += inc)
+ for (uint32_t base = 0; base < hash_n_buckets(hash); base += inc)
{
struct hash_iterator hi;
struct hash_element *he;
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1119?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3334465738fb1fbf508dfd719b6a238b500cc0ae
Gerrit-Change-Number: 1119
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
