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 <rsalz@openssl.org>

diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index 25e2635fd73dfb705712290cbac58e2b470b4552..1d01936e003d739efa35f61cc8542d0a81d2dd64 100644 (file)
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -9,6 +9,7 @@
 
 #include <stdio.h>
 #include "internal/cryptlib.h"
+#include "internal/numbers.h"
 #include <openssl/stack.h>
 #include <openssl/objects.h>
 
@@ -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];