Remove a pointer indirection in STACK_OF(T) comparisons
At the public API, the comparison functions are a double pointer to
match qsort. qsort comparators are passed pointers to the list elements,
and our list elements are T*. qsort needs this indirection because it
can sort values of arbitrary size. However, our type-erased versions
have no such constraints. Since we know our all elements are pointers,
we can skip the indirection.
Change-Id: Ibb102b51a5aaf0a68a7318bf14ec8f4f9c7a3daf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60506
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index 4f4f1df..a11533e 100644
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -297,8 +297,7 @@
if (!OPENSSL_sk_is_sorted(sk)) {
for (size_t i = 0; i < sk->num; i++) {
- const void *elem = sk->data[i];
- if (call_cmp_func(sk->comp, &p, &elem) == 0) {
+ if (call_cmp_func(sk->comp, p, sk->data[i]) == 0) {
if (out_index) {
*out_index = i;
}
@@ -317,8 +316,7 @@
// Bias |mid| towards |lo|. See the |r == 0| case below.
size_t mid = lo + (hi - lo - 1) / 2;
assert(lo <= mid && mid < hi);
- const void *elem = sk->data[mid];
- int r = call_cmp_func(sk->comp, &p, &elem);
+ int r = call_cmp_func(sk->comp, p, sk->data[mid]);
if (r > 0) {
lo = mid + 1; // |mid| is too low.
} else if (r < 0) {
@@ -403,7 +401,10 @@
static int sort_compare(void *ctx_v, const void *a, const void *b) {
struct sort_compare_ctx *ctx = ctx_v;
- return ctx->call_cmp_func(ctx->cmp_func, a, b);
+ // |a| and |b| point to |void*| pointers which contain the actual values.
+ const void *const *a_ptr = a;
+ const void *const *b_ptr = b;
+ return ctx->call_cmp_func(ctx->cmp_func, *a_ptr, *b_ptr);
}
#endif
diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index ce92be6..7d5d4d7 100644
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -279,9 +279,8 @@
// true types.
typedef void (*OPENSSL_sk_call_free_func)(OPENSSL_sk_free_func, void *);
typedef void *(*OPENSSL_sk_call_copy_func)(OPENSSL_sk_copy_func, const void *);
-typedef int (*OPENSSL_sk_call_cmp_func)(OPENSSL_sk_cmp_func,
- const void *const *,
- const void *const *);
+typedef int (*OPENSSL_sk_call_cmp_func)(OPENSSL_sk_cmp_func, const void *,
+ const void *);
typedef int (*OPENSSL_sk_call_delete_if_func)(OPENSSL_sk_delete_if_func, void *,
void *);
@@ -421,13 +420,10 @@
} \
\
OPENSSL_INLINE int sk_##name##_call_cmp_func(OPENSSL_sk_cmp_func cmp_func, \
- const void *const *a, \
- const void *const *b) { \
- /* The data is actually stored as |void*| pointers, so read the pointer \
- * as |void*| and then pass the corrected type into the caller-supplied \
- * function, which expects |constptrtype*|. */ \
- constptrtype a_ptr = (constptrtype)*a; \
- constptrtype b_ptr = (constptrtype)*b; \
+ const void *a, const void *b) { \
+ constptrtype a_ptr = (constptrtype)a; \
+ constptrtype b_ptr = (constptrtype)b; \
+ /* |cmp_func| expects an extra layer of pointers to match qsort. */ \
return ((sk_##name##_cmp_func)cmp_func)(&a_ptr, &b_ptr); \
} \
\
