root
/
openssl
mirror of https://github.com/openssl/openssl.git
1
0
Fork 0
Code Issues Actions 7 Packages Projects Releases Wiki Activity

Ensure that empty or 1 element stacks are always sorted. 

Matt noticed "It's kind of weird that we are forced to call sort on
a newly created and empty stack. It feels like an empty stack should
have the "sorted" flag by default on creation"

I am incluined to agree. This change ensures tht empty or 1 element
stacks are marked as sorted, as per the existing comment in the
file.

Since this involved changing the various duplication routines to
also ensure that sorted was preserved for such stacks, I also
noticed the duplication code was largely duplicated. I
took the opportunity to deduplicate the duplication code while
making these changes.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/28509)
This commit is contained in:
Bob Beck 2025-09-10 11:43:01 -06:00 committed by Pauli
parent a5eaa0e1e3
commit d88c43a644
2 changed files with 88 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

97
crypto/stack/stack.c
View File

@ -38,41 +38,54 @@ OPENSSL_sk_compfunc OPENSSL_sk_set_cmp_func(OPENSSL_STACK *sk,
{ {
OPENSSL_sk_compfunc old = sk->comp; OPENSSL_sk_compfunc old = sk->comp;
if (sk->comp != c) if (sk->comp != c && sk->num > 1)
sk->sorted = 0; sk->sorted = 0;
sk->comp = c; sk->comp = c;
return old; return old;
} }
OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk) static OPENSSL_STACK *internal_copy(const OPENSSL_STACK *sk,
OPENSSL_sk_copyfunc copy_func,
OPENSSL_sk_freefunc free_func)
{ {
OPENSSL_STACK *ret; OPENSSL_STACK *ret;
int i;
if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL) if ((ret = OPENSSL_sk_new_null()) == NULL)
goto err; goto err;
if (sk == NULL) { if (sk == NULL)
ret->num = 0; goto done;
ret->sorted = 0;
ret->comp = NULL;
} else {
/* direct structure assignment */ /* direct structure assignment */
*ret = *sk; *ret = *sk;
}
if (sk == NULL || sk->num == 0) {
/* postpone |ret->data| allocation */
ret->data = NULL; ret->data = NULL;
ret->num_alloc = 0; ret->num_alloc = 0;
return ret;
}
/* duplicate |sk->data| content */ if (ret->num == 0)
ret->data = OPENSSL_malloc_array(sk->num_alloc, sizeof(*ret->data)); goto done; /* nothing to copy */
ret->num_alloc = ret->num > min_nodes ? ret->num : min_nodes;
ret->data = OPENSSL_calloc(ret->num_alloc, sizeof(*ret->data));
if (ret->data == NULL) if (ret->data == NULL)
goto err; goto err;
memcpy(ret->data, sk->data, sizeof(void *) * sk->num); if (copy_func == NULL) {
memcpy(ret->data, sk->data, sizeof(*ret->data) * ret->num);
} else {
for (i = 0; i < ret->num; ++i) {
if (sk->data[i] == NULL)
continue;
if ((ret->data[i] = copy_func(sk->data[i])) == NULL) {
while (--i >= 0)
if (ret->data[i] != NULL)
free_func((void *)ret->data[i]);
goto err;
}
}
}
done:
return ret; return ret;
err: err:
@ -84,48 +97,12 @@ OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk,
OPENSSL_sk_copyfunc copy_func, OPENSSL_sk_copyfunc copy_func,
OPENSSL_sk_freefunc free_func) OPENSSL_sk_freefunc free_func)
{ {
OPENSSL_STACK *ret; return internal_copy(sk, copy_func, free_func);
int i;
if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL)
goto err;
if (sk == NULL) {
ret->num = 0;
ret->sorted = 0;
ret->comp = NULL;
} else {
/* direct structure assignment */
*ret = *sk;
} }
if (sk == NULL || sk->num == 0) { OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk)
/* postpone |ret| data allocation */ {
ret->data = NULL; return internal_copy(sk, NULL, NULL);
ret->num_alloc = 0;
return ret;
}
ret->num_alloc = sk->num > min_nodes ? sk->num : min_nodes;
ret->data = OPENSSL_calloc(ret->num_alloc, sizeof(*ret->data));
if (ret->data == NULL)
goto err;
for (i = 0; i < ret->num; ++i) {
if (sk->data[i] == NULL)
continue;
if ((ret->data[i] = copy_func(sk->data[i])) == NULL) {
while (--i >= 0)
if (ret->data[i] != NULL)
free_func((void *)ret->data[i]);
goto err;
}
}
return ret;
err:
OPENSSL_sk_free(ret);
return NULL;
} }
OPENSSL_STACK *OPENSSL_sk_new_null(void) OPENSSL_STACK *OPENSSL_sk_new_null(void)
@ -232,6 +209,7 @@ OPENSSL_STACK *OPENSSL_sk_new_reserve(OPENSSL_sk_compfunc c, int n)
return NULL; return NULL;
st->comp = c; st->comp = c;
st->sorted = 1; /* empty or single-element stack is considered sorted */
if (n <= 0) if (n <= 0)
return st; return st;
@ -286,7 +264,7 @@ int OPENSSL_sk_insert(OPENSSL_STACK *st, const void *data, int loc)
st->data[loc] = data; st->data[loc] = data;
} }
st->num++; st->num++;
st->sorted = 0; st->sorted = st->num <= 1;
return st->num; return st->num;
} }
@ -298,6 +276,7 @@ static ossl_inline void *internal_delete(OPENSSL_STACK *st, int loc)
memmove(&st->data[loc], &st->data[loc + 1], memmove(&st->data[loc], &st->data[loc + 1],
sizeof(st->data[0]) * (st->num - loc - 1)); sizeof(st->data[0]) * (st->num - loc - 1));
st->num--; st->num--;
st->sorted = st->sorted || st->num <= 1;
return (void *)ret; return (void *)ret;
} }
@ -487,7 +466,7 @@ void *OPENSSL_sk_set(OPENSSL_STACK *st, int i, const void *data)
return NULL; return NULL;
} }
st->data[i] = data; st->data[i] = data;
st->sorted = 0; st->sorted = st->num <= 1;
return (void *)st->data[i]; return (void *)st->data[i];
} }

50
test/stack_test.c
View File

@ -50,6 +50,15 @@ static int int_compare(const int *const *a, const int *const *b)
return 0; return 0;
} }
static int int_compare_backward(const int *const *a, const int *const *b)
{
if (**a > **b)
return -1;
if (**a < **b)
return 1;
return 0;
}
static int test_int_stack(int reserve) static int test_int_stack(int reserve)
{ {
static int v[] = { 1, 2, -4, 16, 999, 1, -173, 1, 9 }; static int v[] = { 1, 2, -4, 16, 999, 1, -173, 1, 9 };
@ -90,6 +99,9 @@ static int test_int_stack(int reserve)
/* Check push and num */ /* Check push and num */
for (i = 0; i < n; i++) { for (i = 0; i < n; i++) {
/* Empty or single element stack should be sorted */
if ((i == 0 || i == 1) && !TEST_true(sk_sint_is_sorted(s)))
goto end;
if (!TEST_int_eq(sk_sint_num(s), i)) { if (!TEST_int_eq(sk_sint_num(s), i)) {
TEST_info("int stack size %d", i); TEST_info("int stack size %d", i);
goto end; goto end;
@ -134,9 +146,18 @@ static int test_int_stack(int reserve)
/* sorting */ /* sorting */
if (!TEST_false(sk_sint_is_sorted(s))) if (!TEST_false(sk_sint_is_sorted(s)))
goto end; goto end;
(void)sk_sint_set_cmp_func(s, &int_compare); (void)sk_sint_set_cmp_func(s, &int_compare_backward);
sk_sint_sort(s); sk_sint_sort(s);
if (!TEST_true(sk_sint_is_sorted(s))) if (!TEST_true(sk_sint_is_sorted(s))) /* should be sorted */
goto end;
(void)sk_sint_set_cmp_func(s, &int_compare_backward);
if (!TEST_true(sk_sint_is_sorted(s))) /* should still be sorted */
goto end;
(void)sk_sint_set_cmp_func(s, &int_compare);
if (!TEST_false(sk_sint_is_sorted(s))) /* should no longer be sorted */
goto end;
sk_sint_sort(s);
if (!TEST_true(sk_sint_is_sorted(s))) /* now should be sorted again */
goto end; goto end;
/* find sorted -- the value is matched so we don't need to locate it */ /* find sorted -- the value is matched so we don't need to locate it */
@ -178,7 +199,7 @@ static int test_uchar_stack(int reserve)
{ {
static const unsigned char v[] = { 1, 3, 7, 5, 255, 0 }; static const unsigned char v[] = { 1, 3, 7, 5, 255, 0 };
const int n = OSSL_NELEM(v); const int n = OSSL_NELEM(v);
STACK_OF(uchar) *s = sk_uchar_new(&uchar_compare), *r = NULL; STACK_OF(uchar) *s = sk_uchar_new(&uchar_compare), *r = NULL, *q = NULL;
int i; int i;
int testresult = 0; int testresult = 0;
@ -205,19 +226,40 @@ static int test_uchar_stack(int reserve)
r = sk_uchar_dup(s); r = sk_uchar_dup(s);
if (!TEST_int_eq(sk_uchar_num(r), n)) if (!TEST_int_eq(sk_uchar_num(r), n))
goto end; goto end;
q = sk_uchar_dup(s);
if (!TEST_int_eq(sk_uchar_num(q), n))
goto end;
sk_uchar_sort(r); sk_uchar_sort(r);
sk_uchar_sort(q);
/* pop */ /* pop */
for (i = 0; i < n; i++) for (i = 0; i < n; i++) {
if (!TEST_ptr_eq(sk_uchar_pop(s), v + i)) { if (!TEST_ptr_eq(sk_uchar_pop(s), v + i)) {
TEST_info("uchar pop %d", i); TEST_info("uchar pop %d", i);
goto end; goto end;
} }
/* Previously unsorted stack of more than 1 element remains unsorted */
if (i < n - 2 && !TEST_false(sk_uchar_is_sorted(s)))
goto end;
/* A single or zero element stack should be sorted */
if (i > n - 2 && !TEST_true(sk_uchar_is_sorted(s)))
goto end;
}
/* free -- we rely on the debug malloc to detect leakage here */ /* free -- we rely on the debug malloc to detect leakage here */
sk_uchar_free(s); sk_uchar_free(s);
s = NULL; s = NULL;
/* pop */
for (i = 0; i < n; i++) {
/* A sorted stack should remain sorted */
if (!TEST_true(sk_uchar_is_sorted(q)))
goto end;
}
/* free -- we rely on the debug malloc to detect leakage here */
sk_uchar_free(q);
q = NULL;
/* dup again */ /* dup again */
if (!TEST_int_eq(sk_uchar_num(r), n)) if (!TEST_int_eq(sk_uchar_num(r), n))
goto end; goto end;