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)};