Harmonize use of sk_TYPE_find's return value.

In some cases it's about redundant check for return value, in some
cases it's about replacing check for -1 with comparison to 0.
Otherwise compiler might generate redundant check for <-1. [Even
formatting and readability fixes.]

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6860)

diff --git a/crypto/asn1/asn_mime.c b/crypto/asn1/asn_mime.c
index aa92a8e115217a1da830eb84e57466f0c482b3ab..dfd5be6347543b8d50c56301f9743290add1d6ef 100644 (file)
--- a/crypto/asn1/asn_mime.c
+++ b/crypto/asn1/asn_mime.c
@@ -883,8 +883,6 @@ static MIME_HEADER *mime_hdr_find(STACK_OF(MIME_HEADER) *hdrs, const char *name)
     htmp.params = NULL;
 
     idx = sk_MIME_HEADER_find(hdrs, &htmp);
-    if (idx < 0)
-        return NULL;
     return sk_MIME_HEADER_value(hdrs, idx);
 }
 
@@ -896,8 +894,6 @@ static MIME_PARAM *mime_param_find(MIME_HEADER *hdr, const char *name)
     param.param_name = (char *)name;
     param.param_value = NULL;
     idx = sk_MIME_PARAM_find(hdr->params, &param);
-    if (idx < 0)
-        return NULL;
     return sk_MIME_PARAM_value(hdr->params, idx);
 }
 

diff --git a/crypto/evp/evp_pbe.c b/crypto/evp/evp_pbe.c
index 0cebd2df1db083d5be9471e0c299f559a8b1ed7e..f80fc064bdc2b09e7023dd215167825ebb601c1b 100644 (file)
--- a/crypto/evp/evp_pbe.c
+++ b/crypto/evp/evp_pbe.c
@@ -217,10 +217,9 @@ int EVP_PBE_find(int type, int pbe_nid,
     pbelu.pbe_type = type;
     pbelu.pbe_nid = pbe_nid;
 
-    if (pbe_algs) {
+    if (pbe_algs != NULL) {
         i = sk_EVP_PBE_CTL_find(pbe_algs, &pbelu);
-        if (i != -1)
-            pbetmp = sk_EVP_PBE_CTL_value(pbe_algs, i);
+        pbetmp = sk_EVP_PBE_CTL_value(pbe_algs, i);
     }
     if (pbetmp == NULL) {
         pbetmp = OBJ_bsearch_pbe2(&pbelu, builtin_pbe, OSSL_NELEM(builtin_pbe));

diff --git a/crypto/objects/obj_xref.c b/crypto/objects/obj_xref.c
index 42d204ca4c336463405f52a0603805de15d4da33..faf59eb20c8307c9de5e13c435b83aee76ef6f79 100644 (file)
--- a/crypto/objects/obj_xref.c
+++ b/crypto/objects/obj_xref.c
@@ -46,10 +46,9 @@ int OBJ_find_sigid_algs(int signid, int *pdig_nid, int *ppkey_nid)
     const nid_triple *rv = NULL;
     tmp.sign_id = signid;
 
-    if (sig_app) {
+    if (sig_app != NULL) {
         int idx = sk_nid_triple_find(sig_app, &tmp);
-        if (idx >= 0)
-            rv = sk_nid_triple_value(sig_app, idx);
+        rv = sk_nid_triple_value(sig_app, idx);
     }
 #ifndef OBJ_XREF_TEST2
     if (rv == NULL) {

diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
index 9d5a571c594161b396034c1e19c98224ec76edb5..11ac52ce3c55782ee173e5a11afdc5ad7471009b 100644 (file)
--- a/crypto/x509/by_dir.c
+++ b/crypto/x509/by_dir.c
@@ -329,10 +329,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
          */
         CRYPTO_THREAD_write_lock(ctx->lock);
         j = sk_X509_OBJECT_find(xl->store_ctx->objs, &stmp);
-        if (j != -1)
-            tmp = sk_X509_OBJECT_value(xl->store_ctx->objs, j);
-        else
-            tmp = NULL;
+        tmp = sk_X509_OBJECT_value(xl->store_ctx->objs, j);
         CRYPTO_THREAD_unlock(ctx->lock);
 
         /* If a CRL, update the last file suffix added for this */
@@ -343,11 +340,10 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
              * Look for entry again in case another thread added an entry
              * first.
              */
-            if (!hent) {
+            if (hent == NULL) {
                 htmp.hash = h;
                 idx = sk_BY_DIR_HASH_find(ent->hashes, &htmp);
-                if (idx >= 0)
-                    hent = sk_BY_DIR_HASH_value(ent->hashes, idx);
+                hent = sk_BY_DIR_HASH_value(ent->hashes, idx);
             }
             if (hent == NULL) {
                 hent = OPENSSL_malloc(sizeof(*hent));

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index e7b1b8521cc28d7af845e366218abefab1e90f60..be39015b0d0126ae1c7a39420770741b9285a8d5 100644 (file)
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -619,17 +619,18 @@ STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm)
 X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
                                         X509_OBJECT *x)
 {
-    int idx, i;
+    int idx, i, num;
     X509_OBJECT *obj;
+
     idx = sk_X509_OBJECT_find(h, x);
-    if (idx == -1)
+    if (idx < 0)
         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++) {
+    for (i = idx, num = sk_X509_OBJECT_num(h); i < num; i++) {
         obj = sk_X509_OBJECT_value(h, i);
-        if (x509_object_cmp
-            ((const X509_OBJECT **)&obj, (const X509_OBJECT **)&x))
+        if (x509_object_cmp((const X509_OBJECT **)&obj,
+                            (const X509_OBJECT **)&x))
             return NULL;
         if (x->type == X509_LU_X509) {
             if (!X509_cmp(obj->data.x509, x->data.x509))

diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c
index a9bb88d1e1b12717681b7c70edce54360d62f6f5..d2b0a8af633146ed53f40c28f670342ad1050325 100644 (file)
--- a/crypto/x509/x509_trs.c
+++ b/crypto/x509/x509_trs.c
@@ -98,13 +98,14 @@ int X509_TRUST_get_by_id(int id)
 {
     X509_TRUST tmp;
     int idx;
+
     if ((id >= X509_TRUST_MIN) && (id <= X509_TRUST_MAX))
         return id - X509_TRUST_MIN;
-    tmp.trust = id;
-    if (!trtable)
+    if (trtable == NULL)
         return -1;
+    tmp.trust = id;
     idx = sk_X509_TRUST_find(trtable, &tmp);
-    if (idx == -1)
+    if (idx < 0)
         return -1;
     return idx + X509_TRUST_COUNT;
 }

diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c
index 16d27f91b5c6743bae2d2c15d4c7f186fdc31286..aea186295c2a026d857f99a1faf0f34b92f4cf9e 100644 (file)
--- a/crypto/x509/x509_vpm.c
+++ b/crypto/x509/x509_vpm.c
@@ -555,10 +555,9 @@ int X509_VERIFY_PARAM_add0_table(X509_VERIFY_PARAM *param)
             return 0;
     } else {
         idx = sk_X509_VERIFY_PARAM_find(param_table, param);
-        if (idx != -1) {
-            ptmp = sk_X509_VERIFY_PARAM_value(param_table, idx);
+        if (idx >= 0) {
+            ptmp = sk_X509_VERIFY_PARAM_delete(param_table, idx);
             X509_VERIFY_PARAM_free(ptmp);
-            (void)sk_X509_VERIFY_PARAM_delete(param_table, idx);
         }
     }
     if (!sk_X509_VERIFY_PARAM_push(param_table, param))
@@ -588,9 +587,9 @@ const X509_VERIFY_PARAM *X509_VERIFY_PARAM_lookup(const char *name)
     X509_VERIFY_PARAM pm;
 
     pm.name = (char *)name;
-    if (param_table) {
+    if (param_table != NULL) {
         idx = sk_X509_VERIFY_PARAM_find(param_table, &pm);
-        if (idx != -1)
+        if (idx >= 0)
             return sk_X509_VERIFY_PARAM_value(param_table, idx);
     }
     return OBJ_bsearch_table(&pm, default_table, OSSL_NELEM(default_table));

diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index c0099e0fec3723d2b270c7493ab0e492c791c018..10733b58bca289d08ca5c13809cb2925d0c3af0e 100644 (file)
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -383,8 +383,11 @@ static int def_crl_lookup(X509_CRL *crl,
                           X509_NAME *issuer)
 {
     X509_REVOKED rtmp, *rev;
-    int idx;
-    rtmp.serialNumber = *serial;
+    int idx, num;
+
+    if (crl->crl.revoked == NULL)
+        return 0;
+
     /*
      * Sort revoked into serial number order if not already sorted. Do this
      * under a lock to avoid race condition.
@@ -394,11 +397,12 @@ static int def_crl_lookup(X509_CRL *crl,
         sk_X509_REVOKED_sort(crl->crl.revoked);
         CRYPTO_THREAD_unlock(crl->lock);
     }
+    rtmp.serialNumber = *serial;
     idx = sk_X509_REVOKED_find(crl->crl.revoked, &rtmp);
     if (idx < 0)
         return 0;
     /* Need to look for matching name */
-    for (; idx < sk_X509_REVOKED_num(crl->crl.revoked); idx++) {
+    for (num = sk_X509_REVOKED_num(crl->crl.revoked); idx < num; idx++) {
         rev = sk_X509_REVOKED_value(crl->crl.revoked, idx);
         if (ASN1_INTEGER_cmp(&rev->serialNumber, serial))
             return 0;

diff --git a/crypto/x509v3/pcy_cache.c b/crypto/x509v3/pcy_cache.c
index 7873a4246c9361fb92d069f8643539a112b2dbbf..623870b1f6f5c00109233d7e394aaf5eccb50d7b 100644 (file)
--- a/crypto/x509v3/pcy_cache.c
+++ b/crypto/x509v3/pcy_cache.c
@@ -26,19 +26,19 @@ static int policy_cache_set_int(long *out, ASN1_INTEGER *value);
 static int policy_cache_create(X509 *x,
                                CERTIFICATEPOLICIES *policies, int crit)
 {
-    int i, ret = 0;
+    int i, num, ret = 0;
     X509_POLICY_CACHE *cache = x->policy_cache;
     X509_POLICY_DATA *data = NULL;
     POLICYINFO *policy;
 
-    if (sk_POLICYINFO_num(policies) == 0)
+    if ((num = sk_POLICYINFO_num(policies)) <= 0)
         goto bad_policy;
     cache->data = sk_X509_POLICY_DATA_new(policy_data_cmp);
     if (cache->data == NULL) {
         X509V3err(X509V3_F_POLICY_CACHE_CREATE, ERR_R_MALLOC_FAILURE);
         goto just_cleanup;
     }
-    for (i = 0; i < sk_POLICYINFO_num(policies); i++) {
+    for (i = 0; i < num; i++) {
         policy = sk_POLICYINFO_value(policies, i);
         data = policy_data_new(policy, NULL, crit);
         if (data == NULL) {
@@ -54,7 +54,7 @@ static int policy_cache_create(X509 *x,
                 goto bad_policy;
             }
             cache->anyPolicy = data;
-        } else if (sk_X509_POLICY_DATA_find(cache->data, data) != -1) {
+        } else if (sk_X509_POLICY_DATA_find(cache->data, data) >=0 ) {
             ret = -1;
             goto bad_policy;
         } else if (!sk_X509_POLICY_DATA_push(cache->data, data)) {
@@ -204,8 +204,6 @@ X509_POLICY_DATA *policy_cache_find_data(const X509_POLICY_CACHE *cache,
     X509_POLICY_DATA tmp;
     tmp.valid_policy = (ASN1_OBJECT *)id;
     idx = sk_X509_POLICY_DATA_find(cache->data, &tmp);
-    if (idx == -1)
-        return NULL;
     return sk_X509_POLICY_DATA_value(cache->data, idx);
 }
 

diff --git a/crypto/x509v3/pcy_node.c b/crypto/x509v3/pcy_node.c
index db4d71f819f17a6ed9970d964346ea8da4fdef4b..1ffe98498bdb4ad84a68452dcd37e6ab4ab28571 100644 (file)
--- a/crypto/x509v3/pcy_node.c
+++ b/crypto/x509v3/pcy_node.c
@@ -36,9 +36,6 @@ X509_POLICY_NODE *tree_find_sk(STACK_OF(X509_POLICY_NODE) *nodes,
     l.data = &n;
 
     idx = sk_X509_POLICY_NODE_find(nodes, &l);
-    if (idx == -1)
-        return NULL;
-
     return sk_X509_POLICY_NODE_value(nodes, idx);
 
 }

diff --git a/crypto/x509v3/pcy_tree.c b/crypto/x509v3/pcy_tree.c
index 9a1e2088859e73143794ab5efde1fc9d09541c7f..87f51d001bbbc1d52526c4f64e300463e173a3be 100644 (file)
--- a/crypto/x509v3/pcy_tree.c
+++ b/crypto/x509v3/pcy_tree.c
@@ -442,7 +442,7 @@ static int tree_add_auth_node(STACK_OF(X509_POLICY_NODE) **pnodes,
     if (*pnodes == NULL &&
         (*pnodes = policy_node_cmp_new()) == NULL)
         return 0;
-    if (sk_X509_POLICY_NODE_find(*pnodes, pcy) != -1)
+    if (sk_X509_POLICY_NODE_find(*pnodes, pcy) >= 0)
         return 1;
     return sk_X509_POLICY_NODE_push(*pnodes, pcy) != 0;
 }

diff --git a/crypto/x509v3/v3_lib.c b/crypto/x509v3/v3_lib.c
index f51aa9624c4b3f2aac7153219c5078cf2cfbc0f1..e11e51b6e08a7c20be2791e8c9e25125fbcf57df 100644 (file)
--- a/crypto/x509v3/v3_lib.c
+++ b/crypto/x509v3/v3_lib.c
@@ -64,8 +64,6 @@ const X509V3_EXT_METHOD *X509V3_EXT_get_nid(int nid)
     if (!ext_list)
         return NULL;
     idx = sk_X509V3_EXT_METHOD_find(ext_list, &tmp);
-    if (idx == -1)
-        return NULL;
     return sk_X509V3_EXT_METHOD_value(ext_list, idx);
 }
 

diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c
index 1ba1ce8373bb0b72afea0d332f46e2efc03a14ac..b42151295f23a6b3116949a7a9efbdfdc76d8329 100644 (file)
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -133,13 +133,14 @@ int X509_PURPOSE_get_by_id(int purpose)
 {
     X509_PURPOSE tmp;
     int idx;
+
     if ((purpose >= X509_PURPOSE_MIN) && (purpose <= X509_PURPOSE_MAX))
         return purpose - X509_PURPOSE_MIN;
-    tmp.purpose = purpose;
-    if (!xptable)
+    if (xptable == NULL)
         return -1;
+    tmp.purpose = purpose;
     idx = sk_X509_PURPOSE_find(xptable, &tmp);
-    if (idx == -1)
+    if (idx < 0)
         return -1;
     return idx + X509_PURPOSE_COUNT;
 }

diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index 9011e42fa895c7310287b59109ec65b1b157a92e..b60cc79a2f53ac81472832a604ad71cc7e0c3745 100644 (file)
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -505,10 +505,7 @@ int ssl_cipher_get_evp(const SSL_SESSION *s, const EVP_CIPHER **enc,
         ctmp.id = s->compress_meth;
         if (ssl_comp_methods != NULL) {
             i = sk_SSL_COMP_find(ssl_comp_methods, &ctmp);
-            if (i >= 0)
-                *comp = sk_SSL_COMP_value(ssl_comp_methods, i);
-            else
-                *comp = NULL;
+            *comp = sk_SSL_COMP_value(ssl_comp_methods, i);
         }
         /* If were only interested in comp then return success */
         if ((enc == NULL) && (md == NULL))