From 9205ebeb8e448b2d6948b9e5d78ecf309c0ed33c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 8 Sep 2016 11:06:29 +0100 Subject: [PATCH] Convert num_alloc to a size_t in stack.c and tweak style We were casting num_alloc to size_t in lots of places, or just using it in a context where size_t makes more sense - so convert it. This simplifies the code a bit. Also tweak the style in stack.c a bit following on from the previous commit Reviewed-by: Rich Salz --- crypto/stack/stack.c | 57 ++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c index 25e2635fd7..1d01936e00 100644 --- a/crypto/stack/stack.c +++ b/crypto/stack/stack.c @@ -9,6 +9,7 @@ #include #include "internal/cryptlib.h" +#include "internal/numbers.h" #include #include @@ -16,7 +17,7 @@ struct stack_st { int num; const char **data; int sorted; - int num_alloc; + size_t num_alloc; OPENSSL_sk_compfunc comp; }; @@ -39,9 +40,9 @@ OPENSSL_sk_compfunc OPENSSL_sk_set_cmp_func(OPENSSL_STACK *sk, OPENSSL_sk_compfu OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk) { OPENSSL_STACK *ret; - if ( sk->num_alloc < 0 || sk->num < 0 ) { + + if (sk->num < 0) return NULL; - } if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL) return NULL; @@ -65,9 +66,8 @@ OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk, OPENSSL_STACK *ret; int i; - if ( sk->num < 0 ) { + if (sk->num < 0) return NULL; - } if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL) return NULL; @@ -75,7 +75,7 @@ OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk, /* direct structure assignment */ *ret = *sk; - ret->num_alloc = sk->num > MIN_NODES ? sk->num : MIN_NODES; + ret->num_alloc = sk->num > MIN_NODES ? (size_t)sk->num : MIN_NODES; ret->data = OPENSSL_zalloc(sizeof(*ret->data) * ret->num_alloc); if (ret->data == NULL) { OPENSSL_free(ret); @@ -120,55 +120,44 @@ OPENSSL_STACK *OPENSSL_sk_new(OPENSSL_sk_compfunc c) int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc) { - if ( st == NULL || st->num < 0 || st->num == INT_MAX || st->num_alloc < 0 ) { + if (st == NULL || st->num < 0 || st->num == INT_MAX) { return 0; } - if (st->num_alloc <= st->num + 1) { - - /* Overflow checks by Guido - * Cast to size_t to avoid triggering -ftrapv via overflow of st->num_alloc - */ - if ( (size_t)(st->num_alloc) * 2 < (size_t)(st->num_alloc) ) { - return 0; - } + if (st->num_alloc <= (size_t)(st->num + 1)) { + size_t doub_num_alloc = st->num_alloc * 2; - if ( (size_t)(st->num_alloc) * 2 > INT_MAX ) - { + /* Overflow checks */ + if (doub_num_alloc < st->num_alloc) return 0; - } - /* Avond overflow due to multiplication by sizeof(char*) */ - if ( (size_t)(st->num_alloc) * 2 > (~(size_t)0) / sizeof(char*) ) { + /* Avoid overflow due to multiplication by sizeof(char *) */ + if (doub_num_alloc > SIZE_MAX / sizeof(char *)) return 0; - } - /* Remove cast to unsigned int to avoid undersized allocations on > 32 bit. */ st->data = OPENSSL_realloc((char *)st->data, - sizeof(char *) * st->num_alloc * 2); + sizeof(char *) * doub_num_alloc); if (st->data == NULL) { - /* Reset these counters to prevent subsequent operations on + /* + * Reset these counters to prevent subsequent operations on * (now non-existing) heap memory - */ + */ st->num_alloc = 0; st->num = 0; - return (0); + return 0; } - st->num_alloc *= 2; + st->num_alloc = doub_num_alloc; } - if ((loc >= (int)st->num) || (loc < 0)) + if ((loc >= st->num) || (loc < 0)) { st->data[st->num] = data; - else { - if ( loc == INT_MAX ) { - return 0; - } + } else { memmove(&st->data[loc + 1], &st->data[loc], sizeof(st->data[0]) * (st->num - loc)); st->data[loc] = data; } st->num++; st->sorted = 0; - return (st->num); + return st->num; } void *OPENSSL_sk_delete_ptr(OPENSSL_STACK *st, const void *p) @@ -185,7 +174,7 @@ void *OPENSSL_sk_delete(OPENSSL_STACK *st, int loc) { const char *ret; - if (st == NULL || loc < 0 || loc >= st->num ) + if (st == NULL || loc < 0 || loc >= st->num) return NULL; ret = st->data[loc]; -- 2.25.1