Const-correct sk_FOO_cmp_func
This is a double-pointer and both layers should be const. This matches
OpenSSL 1.1.1, so in addition to being more const-correct, we're more
OpenSSL-compatible.
Update-Note: Anything that defines a comparison function would need to
fix the type signature. I found only one external caller, Envoy, that
defines it. https://github.com/envoyproxy/envoy/pull/25051 fixes it.
(That we hadn't run into the upstream incompatibility suggests this is
just not a feature folks use outside the library much.)
Bumping BORINGSSL_API_VERSION, in case I missed any, and there's some
caller where we can't just use C++14 generic lambdas to smooth it over.
Fixed: 498
Change-Id: I8f07ff42215172aa65ad8819acf69b63d6d8e54c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56190
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/stack/stack_test.cc b/crypto/stack/stack_test.cc
index 1ff44b9..7900fc9 100644
--- a/crypto/stack/stack_test.cc
+++ b/crypto/stack/stack_test.cc
@@ -210,7 +210,7 @@
static uint64_t g_compare_count = 0;
-static int compare(const TEST_INT **a, const TEST_INT **b) {
+static int compare(const TEST_INT *const *a, const TEST_INT *const *b) {
g_compare_count++;
if (**a < **b) {
return -1;
@@ -221,7 +221,7 @@
return 0;
}
-static int compare_reverse(const TEST_INT **a, const TEST_INT **b) {
+static int compare_reverse(const TEST_INT *const *a, const TEST_INT *const *b) {
return -compare(a, b);
}
diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
index 0283a0d..a9236fe 100644
--- a/crypto/x509/by_dir.c
+++ b/crypto/x509/by_dir.c
@@ -152,7 +152,8 @@
static void by_dir_hash_free(BY_DIR_HASH *hash) { OPENSSL_free(hash); }
-static int by_dir_hash_cmp(const BY_DIR_HASH **a, const BY_DIR_HASH **b) {
+static int by_dir_hash_cmp(const BY_DIR_HASH *const *a,
+ const BY_DIR_HASH *const *b) {
if ((*a)->hash > (*b)->hash) {
return 1;
}
diff --git a/crypto/x509/policy.c b/crypto/x509/policy.c
index 47ac2d8..abcc586 100644
--- a/crypto/x509/policy.c
+++ b/crypto/x509/policy.c
@@ -121,8 +121,8 @@
return node;
}
-static int x509_policy_node_cmp(const X509_POLICY_NODE **a,
- const X509_POLICY_NODE **b) {
+static int x509_policy_node_cmp(const X509_POLICY_NODE *const *a,
+ const X509_POLICY_NODE *const *b) {
return OBJ_cmp((*a)->policy, (*b)->policy);
}
@@ -201,7 +201,8 @@
return 1;
}
-static int policyinfo_cmp(const POLICYINFO **a, const POLICYINFO **b) {
+static int policyinfo_cmp(const POLICYINFO *const *a,
+ const POLICYINFO *const *b) {
return OBJ_cmp((*a)->policyid, (*b)->policyid);
}
@@ -312,13 +313,13 @@
return ret;
}
-static int compare_issuer_policy(const POLICY_MAPPING **a,
- const POLICY_MAPPING **b) {
+static int compare_issuer_policy(const POLICY_MAPPING *const *a,
+ const POLICY_MAPPING *const *b) {
return OBJ_cmp((*a)->issuerDomainPolicy, (*b)->issuerDomainPolicy);
}
-static int compare_subject_policy(const POLICY_MAPPING **a,
- const POLICY_MAPPING **b) {
+static int compare_subject_policy(const POLICY_MAPPING *const *a,
+ const POLICY_MAPPING *const *b) {
return OBJ_cmp((*a)->subjectDomainPolicy, (*b)->subjectDomainPolicy);
}
@@ -651,7 +652,8 @@
return 0;
}
-static int asn1_object_cmp(const ASN1_OBJECT **a, const ASN1_OBJECT **b) {
+static int asn1_object_cmp(const ASN1_OBJECT *const *a,
+ const ASN1_OBJECT *const *b) {
return OBJ_cmp(*a, *b);
}
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 2ec8971..cd4ed12 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -140,25 +140,25 @@
return ctx->method->get_by_subject(ctx, type, name, ret) > 0;
}
-static int x509_object_cmp(const X509_OBJECT **a, const X509_OBJECT **b) {
- int ret;
-
- ret = ((*a)->type - (*b)->type);
+static int x509_object_cmp(const X509_OBJECT *a, const X509_OBJECT *b) {
+ int ret = a->type - b->type;
if (ret) {
return ret;
}
- switch ((*a)->type) {
+ switch (a->type) {
case X509_LU_X509:
- ret = X509_subject_name_cmp((*a)->data.x509, (*b)->data.x509);
- break;
+ return X509_subject_name_cmp(a->data.x509, b->data.x509);
case X509_LU_CRL:
- ret = X509_CRL_cmp((*a)->data.crl, (*b)->data.crl);
- break;
+ return X509_CRL_cmp(a->data.crl, b->data.crl);
default:
// abort();
return 0;
}
- return ret;
+}
+
+static int x509_object_cmp_sk(const X509_OBJECT *const *a,
+ const X509_OBJECT *const *b) {
+ return x509_object_cmp(*a, *b);
}
X509_STORE *X509_STORE_new(void) {
@@ -169,7 +169,7 @@
}
OPENSSL_memset(ret, 0, sizeof(*ret));
CRYPTO_MUTEX_init(&ret->objs_lock);
- ret->objs = sk_X509_OBJECT_new(x509_object_cmp);
+ ret->objs = sk_X509_OBJECT_new(x509_object_cmp_sk);
if (ret->objs == NULL) {
goto err;
}
@@ -424,12 +424,10 @@
if (pnmatch != NULL) {
int tidx;
- const X509_OBJECT *tobj, *pstmp;
*pnmatch = 1;
- pstmp = &stmp;
for (tidx = idx + 1; tidx < (int)sk_X509_OBJECT_num(h); tidx++) {
- tobj = sk_X509_OBJECT_value(h, tidx);
- if (x509_object_cmp(&tobj, &pstmp)) {
+ const X509_OBJECT *tobj = sk_X509_OBJECT_value(h, tidx);
+ if (x509_object_cmp(tobj, &stmp)) {
break;
}
(*pnmatch)++;
@@ -542,19 +540,17 @@
X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
X509_OBJECT *x) {
- size_t idx, i;
- X509_OBJECT *obj;
-
sk_X509_OBJECT_sort(h);
+ size_t idx;
if (!sk_X509_OBJECT_find(h, &idx, x)) {
return NULL;
}
if ((x->type != X509_LU_X509) && (x->type != X509_LU_CRL)) {
return sk_X509_OBJECT_value(h, idx);
}
- for (i = idx; i < sk_X509_OBJECT_num(h); i++) {
- obj = sk_X509_OBJECT_value(h, i);
- if (x509_object_cmp((const X509_OBJECT **)&obj, (const X509_OBJECT **)&x)) {
+ for (size_t i = idx; i < sk_X509_OBJECT_num(h); i++) {
+ X509_OBJECT *obj = sk_X509_OBJECT_value(h, i);
+ if (x509_object_cmp(obj, x)) {
return NULL;
}
if (x->type == X509_LU_X509) {
diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c
index 553a0ae..13e5eca 100644
--- a/crypto/x509/x509_trs.c
+++ b/crypto/x509/x509_trs.c
@@ -63,7 +63,7 @@
#include "internal.h"
-static int tr_cmp(const X509_TRUST **a, const X509_TRUST **b);
+static int tr_cmp(const X509_TRUST *const *a, const X509_TRUST *const *b);
static void trtable_free(X509_TRUST *p);
static int trust_1oidany(X509_TRUST *trust, X509 *x, int flags);
@@ -97,7 +97,7 @@
static STACK_OF(X509_TRUST) *trtable = NULL;
-static int tr_cmp(const X509_TRUST **a, const X509_TRUST **b) {
+static int tr_cmp(const X509_TRUST *const *a, const X509_TRUST *const *b) {
return (*a)->trust - (*b)->trust;
}
diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c
index f14a531..8ff8038 100644
--- a/crypto/x509/x509_vpm.c
+++ b/crypto/x509/x509_vpm.c
@@ -549,7 +549,8 @@
static STACK_OF(X509_VERIFY_PARAM) *param_table = NULL;
-static int param_cmp(const X509_VERIFY_PARAM **a, const X509_VERIFY_PARAM **b) {
+static int param_cmp(const X509_VERIFY_PARAM *const *a,
+ const X509_VERIFY_PARAM *const *b) {
return strcmp((*a)->name, (*b)->name);
}
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 9c4c116..4a645ba 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -71,7 +71,8 @@
#include "../internal.h"
#include "internal.h"
-static int X509_REVOKED_cmp(const X509_REVOKED **a, const X509_REVOKED **b);
+static int X509_REVOKED_cmp(const X509_REVOKED *const *a,
+ const X509_REVOKED *const *b);
static int setup_idp(X509_CRL *crl, ISSUING_DIST_POINT *idp);
ASN1_SEQUENCE(X509_REVOKED) = {
@@ -361,7 +362,8 @@
IMPLEMENT_ASN1_FUNCTIONS(X509_CRL)
IMPLEMENT_ASN1_DUP_FUNCTION(X509_CRL)
-static int X509_REVOKED_cmp(const X509_REVOKED **a, const X509_REVOKED **b) {
+static int X509_REVOKED_cmp(const X509_REVOKED *const *a,
+ const X509_REVOKED *const *b) {
return ASN1_STRING_cmp((*a)->serialNumber, (*b)->serialNumber);
}
diff --git a/crypto/x509v3/v3_lib.c b/crypto/x509v3/v3_lib.c
index 5a3c3e8..d33b51f 100644
--- a/crypto/x509v3/v3_lib.c
+++ b/crypto/x509v3/v3_lib.c
@@ -72,8 +72,8 @@
static void ext_list_free(X509V3_EXT_METHOD *ext);
-static int ext_stack_cmp(const X509V3_EXT_METHOD **a,
- const X509V3_EXT_METHOD **b) {
+static int ext_stack_cmp(const X509V3_EXT_METHOD *const *a,
+ const X509V3_EXT_METHOD *const *b) {
return ((*a)->ext_nid - (*b)->ext_nid);
}
diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c
index 8c254e8..34ce33e 100644
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -95,7 +95,7 @@
static int no_check(const X509_PURPOSE *xp, const X509 *x, int ca);
static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca);
-static int xp_cmp(const X509_PURPOSE **a, const X509_PURPOSE **b);
+static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b);
static void xptable_free(X509_PURPOSE *p);
static X509_PURPOSE xstandard[] = {
@@ -126,7 +126,7 @@
static STACK_OF(X509_PURPOSE) *xptable = NULL;
-static int xp_cmp(const X509_PURPOSE **a, const X509_PURPOSE **b) {
+static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b) {
return (*a)->purpose - (*b)->purpose;
}
diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index 9281e30..183cf6a 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -75,7 +75,7 @@
static char *strip_spaces(char *name);
-static int sk_strcmp(const char **a, const char **b);
+static int sk_strcmp(const char *const *a, const char *const *b);
static STACK_OF(OPENSSL_STRING) *get_email(const X509_NAME *name,
const GENERAL_NAMES *gens);
static void str_free(OPENSSL_STRING str);
@@ -551,7 +551,9 @@
return name[len] == '\0' || name[len] == '.';
}
-static int sk_strcmp(const char **a, const char **b) { return strcmp(*a, *b); }
+static int sk_strcmp(const char *const *a, const char *const *b) {
+ return strcmp(*a, *b);
+}
STACK_OF(OPENSSL_STRING) *X509_get1_email(X509 *x) {
GENERAL_NAMES *gens;
diff --git a/include/openssl/base.h b/include/openssl/base.h
index a1a4309..fb1815f 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -193,7 +193,7 @@
// A consumer may use this symbol in the preprocessor to temporarily build
// against multiple revisions of BoringSSL at the same time. It is not
// recommended to do so for longer than is necessary.
-#define BORINGSSL_API_VERSION 18
+#define BORINGSSL_API_VERSION 19
#if defined(BORINGSSL_SHARED_LIBRARY)
diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index 72da30d..59b1c5e 100644
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -127,10 +127,8 @@
// respectively. Note the extra indirection - the function is given a pointer
// to a pointer to the element. This is the |qsort|/|bsearch| comparison
// function applied to an array of |SAMPLE*|.
-//
-// TODO(https://crbug.com/boringssl/498): The parameters should be
-// |const SAMPLE *const *|.
-typedef int (*sk_SAMPLE_cmp_func)(const SAMPLE **a, const SAMPLE **b);
+typedef int (*sk_SAMPLE_cmp_func)(const SAMPLE *const *a,
+ const SAMPLE *const *b);
// sk_SAMPLE_new creates a new, empty stack with the given comparison function,
// which may be NULL. It returns the new stack or NULL on allocation failure.
@@ -264,13 +262,10 @@
// the extra indirection - the function is given a pointer to a pointer to the
// element. This differs from the usual qsort/bsearch comparison function.
//
-// Note its actual type is |int (*)(const T **a, const T **b)|. Low-level |sk_*|
-// functions will be passed a type-specific wrapper to call it correctly.
-//
-// TODO(https://crbug.com/boringssl/498): This type should be
-// |const T *const *|. It is already fixed in OpenSSL 1.1.1, so hopefully we can
-// fix this compatibly.
-typedef int (*OPENSSL_sk_cmp_func)(const void **a, const void **b);
+// Note its actual type is |int (*)(const T *const *a, const T *const *b)|.
+// Low-level |sk_*| functions will be passed a type-specific wrapper to call it
+// correctly.
+typedef int (*OPENSSL_sk_cmp_func)(const void *const *a, const void *const *b);
// OPENSSL_sk_delete_if_func is the generic version of
// |sk_SAMPLE_delete_if_func|.
@@ -387,7 +382,8 @@
\
typedef void (*sk_##name##_free_func)(ptrtype); \
typedef ptrtype (*sk_##name##_copy_func)(constptrtype); \
- typedef int (*sk_##name##_cmp_func)(constptrtype *, constptrtype *); \
+ typedef int (*sk_##name##_cmp_func)(constptrtype const *, \
+ constptrtype const *); \
typedef int (*sk_##name##_delete_if_func)(ptrtype, void *); \
\
OPENSSL_INLINE void sk_##name##_call_free_func( \
diff --git a/ssl/ssl_file.cc b/ssl/ssl_file.cc
index d867cf1..9e06ec8 100644
--- a/ssl/ssl_file.cc
+++ b/ssl/ssl_file.cc
@@ -124,7 +124,7 @@
#include "internal.h"
-static int xname_cmp(const X509_NAME **a, const X509_NAME **b) {
+static int xname_cmp(const X509_NAME *const *a, const X509_NAME *const *b) {
return X509_NAME_cmp(*a, *b);
}
@@ -143,7 +143,7 @@
struct RestoreCmpFunc {
~RestoreCmpFunc() { sk_X509_NAME_set_cmp_func(stack, old_cmp); }
STACK_OF(X509_NAME) *stack;
- int (*old_cmp)(const X509_NAME **, const X509_NAME **);
+ int (*old_cmp)(const X509_NAME *const *, const X509_NAME *const *);
};
RestoreCmpFunc restore = {out, sk_X509_NAME_set_cmp_func(out, xname_cmp)};
