free NULL cleanup
authorRich Salz <rsalz@openssl.org>
Tue, 24 Mar 2015 11:52:24 +0000 (07:52 -0400)
committerRich Salz <rsalz@openssl.org>
Tue, 24 Mar 2015 11:52:24 +0000 (07:52 -0400)
Start ensuring all OpenSSL "free" routines allow NULL, and remove
any if check before calling them.
This gets ASN1_OBJECT_free and ASN1_STRING_free.

Reviewed-by: Matt Caswell <matt@openssl.org>
26 files changed:
CHANGES
apps/cms.c
crypto/asn1/a_bitstr.c
crypto/asn1/a_int.c
crypto/asn1/a_utctm.c
crypto/asn1/asn1_lib.c
crypto/asn1/asn1_par.c
crypto/asn1/evp_asn1.c
crypto/asn1/p5_pbe.c
crypto/asn1/tasn_fre.c
crypto/asn1/x_algor.c
crypto/asn1/x_pkey.c
crypto/asn1/x_x509a.c
crypto/dh/dh_ameth.c
crypto/dh/dh_pmeth.c
crypto/dsa/dsa_ameth.c
crypto/ec/ec_asn1.c
crypto/ocsp/ocsp_lib.c
crypto/ocsp/v3_ocsp.c
crypto/rsa/rsa_ameth.c
crypto/rsa/rsa_saos.c
crypto/x509v3/pcy_data.c
crypto/x509v3/v3_conf.c
crypto/x509v3/v3_pci.c
doc/crypto/ASN1_OBJECT_new.pod
doc/crypto/ASN1_STRING_new.pod

diff --git a/CHANGES b/CHANGES
index 5dd7d8d..ab5b482 100644 (file)
--- a/CHANGES
+++ b/CHANGES
         Remove all but one '#ifdef undef' which is to be looked at.
      [Rich Salz]
 
+  *) Clean up calling of xxx_free routines.
+        Just like free(), fix most of the xxx_free routines to accept
+        NULL.  Remove the non-null checks from callers.  Save much code.
+     [Rich Salz]
+
   *) Experimental support for a new, fast, unbiased prime candidate generator,
      bn_probable_prime_dh_coprime(). Not currently used by any prime generator.
      [Felix Laurie von Massenbach <felix@erbridge.co.uk>]
index d983e28..0877426 100644 (file)
@@ -1152,8 +1152,7 @@ int MAIN(int argc, char **argv)
         OPENSSL_free(secret_keyid);
     if (pwri_tmp)
         OPENSSL_free(pwri_tmp);
-    if (econtent_type)
-        ASN1_OBJECT_free(econtent_type);
+    ASN1_OBJECT_free(econtent_type);
     if (rr)
         CMS_ReceiptRequest_free(rr);
     if (rr_to)
index 5a5cc23..4078be4 100644 (file)
@@ -177,7 +177,7 @@ ASN1_BIT_STRING *c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
     return (ret);
  err:
     ASN1err(ASN1_F_C2I_ASN1_BIT_STRING, i);
-    if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+    if ((a == NULL) || (*a != ret))
         ASN1_BIT_STRING_free(ret);
     return (NULL);
 }
index a33e3fd..65fac75 100644 (file)
@@ -265,7 +265,7 @@ ASN1_INTEGER *c2i_ASN1_INTEGER(ASN1_INTEGER **a, const unsigned char **pp,
     return (ret);
  err:
     ASN1err(ASN1_F_C2I_ASN1_INTEGER, i);
-    if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+    if ((a == NULL) || (*a != ret))
         ASN1_INTEGER_free(ret);
     return (NULL);
 }
@@ -334,7 +334,7 @@ ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp,
     return (ret);
  err:
     ASN1err(ASN1_F_D2I_ASN1_UINTEGER, i);
-    if ((ret != NULL) && ((a == NULL) || (*a != ret)))
+    if ((a == NULL) || (*a != ret))
         ASN1_INTEGER_free(ret);
     return (NULL);
 }
index 2dac3b5..0e2f1b0 100644 (file)
@@ -193,11 +193,11 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t,
     int free_s = 0;
 
     if (s == NULL) {
-        free_s = 1;
         s = ASN1_UTCTIME_new();
+        if (s == NULL)
+            goto err;
+        free_s = 1;
     }
-    if (s == NULL)
-        goto err;
 
     ts = OPENSSL_gmtime(&t, &data);
     if (ts == NULL)
@@ -233,7 +233,7 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t,
 #endif
     return (s);
  err:
-    if (free_s && s)
+    if (free_s)
         ASN1_UTCTIME_free(s);
     return NULL;
 }
index fe63b62..2e36cff 100644 (file)
@@ -429,7 +429,9 @@ void ASN1_STRING_free(ASN1_STRING *a)
 
 void ASN1_STRING_clear_free(ASN1_STRING *a)
 {
-    if (a && a->data && !(a->flags & ASN1_STRING_FLAG_NDEF))
+    if (a == NULL)
+        return;
+    if (a->data && !(a->flags & ASN1_STRING_FLAG_NDEF))
         OPENSSL_cleanse(a->data, a->length);
     ASN1_STRING_free(a);
 }
index 20f3a88..574e8de 100644 (file)
@@ -276,10 +276,8 @@ static int asn1_parse2(BIO *bp, const unsigned char **pp, long length,
                         nl = 1;
                     }
                 }
-                if (os != NULL) {
-                    ASN1_OCTET_STRING_free(os);
-                    os = NULL;
-                }
+                ASN1_OCTET_STRING_free(os);
+                os = NULL;
             } else if (tag == V_ASN1_INTEGER) {
                 ASN1_INTEGER *bs;
                 int i;
@@ -356,10 +354,8 @@ static int asn1_parse2(BIO *bp, const unsigned char **pp, long length,
     }
     ret = 1;
  end:
-    if (o != NULL)
-        ASN1_OBJECT_free(o);
-    if (os != NULL)
-        ASN1_OCTET_STRING_free(os);
+    ASN1_OBJECT_free(o);
+    ASN1_OCTET_STRING_free(os);
     *pp = p;
     return (ret);
 }
index 3664576..e6a5b5f 100644 (file)
@@ -187,8 +187,7 @@ int ASN1_TYPE_get_int_octetstring(ASN1_TYPE *a, long *num,
  err:
         ASN1err(ASN1_F_ASN1_TYPE_GET_INT_OCTETSTRING, ASN1_R_DATA_IS_WRONG);
     }
-    if (os != NULL)
-        ASN1_OCTET_STRING_free(os);
+    ASN1_OCTET_STRING_free(os);
     if (ai != NULL)
         ASN1_INTEGER_free(ai);
     return (ret);
index bdbfdcd..d54b094 100644 (file)
@@ -118,8 +118,7 @@ int PKCS5_pbe_set0_algor(X509_ALGOR *algor, int alg, int iter,
  err:
     if (pbe != NULL)
         PBEPARAM_free(pbe);
-    if (pbe_str != NULL)
-        ASN1_STRING_free(pbe_str);
+    ASN1_STRING_free(pbe_str);
     return 0;
 }
 
index 49c5793..bdc26f9 100644 (file)
@@ -85,6 +85,7 @@ static void asn1_item_combine_free(ASN1_VALUE **pval, const ASN1_ITEM *it,
     const ASN1_AUX *aux = it->funcs;
     ASN1_aux_cb *asn1_cb;
     int i;
+
     if (!pval)
         return;
     if ((it->itype != ASN1_ITYPE_PRIMITIVE) && !*pval)
@@ -116,6 +117,7 @@ static void asn1_item_combine_free(ASN1_VALUE **pval, const ASN1_ITEM *it,
         i = asn1_get_choice_selector(pval, it);
         if ((i >= 0) && (i < it->tcount)) {
             ASN1_VALUE **pchval;
+
             tt = it->templates + i;
             pchval = asn1_get_field_ptr(pval, tt);
             ASN1_template_free(pchval, tt);
@@ -170,35 +172,41 @@ static void asn1_item_combine_free(ASN1_VALUE **pval, const ASN1_ITEM *it,
 
 void ASN1_template_free(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt)
 {
-    int i;
     if (tt->flags & ASN1_TFLG_SK_MASK) {
         STACK_OF(ASN1_VALUE) *sk = (STACK_OF(ASN1_VALUE) *)*pval;
+        int i;
+
         for (i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
-            ASN1_VALUE *vtmp;
-            vtmp = sk_ASN1_VALUE_value(sk, i);
+            ASN1_VALUE *vtmp = sk_ASN1_VALUE_value(sk, i);
+
             asn1_item_combine_free(&vtmp, ASN1_ITEM_ptr(tt->item), 0);
         }
         sk_ASN1_VALUE_free(sk);
         *pval = NULL;
-    } else
+    } else {
         asn1_item_combine_free(pval, ASN1_ITEM_ptr(tt->item),
                                tt->flags & ASN1_TFLG_COMBINE);
+    }
 }
 
 void ASN1_primitive_free(ASN1_VALUE **pval, const ASN1_ITEM *it)
 {
     int utype;
+
+    /* Special case: if 'it' is a primitive with a free_func, use that. */
     if (it) {
-        const ASN1_PRIMITIVE_FUNCS *pf;
-        pf = it->funcs;
+        const ASN1_PRIMITIVE_FUNCS *pf = it->funcs;
+
         if (pf && pf->prim_free) {
             pf->prim_free(pval, it);
             return;
         }
     }
-    /* Special case: if 'it' is NULL free contents of ASN1_TYPE */
+
+    /* Special case: if 'it' is NULL, free contents of ASN1_TYPE */
     if (!it) {
         ASN1_TYPE *typ = (ASN1_TYPE *)*pval;
+
         utype = typ->type;
         pval = &typ->value.asn1_value;
         if (!*pval)
@@ -235,8 +243,8 @@ void ASN1_primitive_free(ASN1_VALUE **pval, const ASN1_ITEM *it)
 
     default:
         ASN1_STRING_free((ASN1_STRING *)*pval);
-        *pval = NULL;
         break;
     }
-    *pval = NULL;
+    if (*pval)
+        *pval = NULL;
 }
index 0aa3ded..30d6481 100644 (file)
@@ -86,8 +86,7 @@ int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval)
             return 0;
     }
     if (alg) {
-        if (alg->algorithm)
-            ASN1_OBJECT_free(alg->algorithm);
+        ASN1_OBJECT_free(alg->algorithm);
         alg->algorithm = aobj;
     }
     if (ptype == 0)
index cf5fd80..f4396e7 100644 (file)
@@ -143,8 +143,7 @@ void X509_PKEY_free(X509_PKEY *x)
 
     if (x->enc_algor != NULL)
         X509_ALGOR_free(x->enc_algor);
-    if (x->enc_pkey != NULL)
-        ASN1_OCTET_STRING_free(x->enc_pkey);
+    ASN1_OCTET_STRING_free(x->enc_pkey);
     if (x->dec_pkey != NULL)
         EVP_PKEY_free(x->dec_pkey);
     if ((x->key_data != NULL) && (x->key_free))
index 2a2ca87..8be50b5 100644 (file)
@@ -159,8 +159,7 @@ int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
     if (!objtmp || sk_ASN1_OBJECT_push(aux->trust, objtmp))
         return 1;
  err:
-    if (objtmp)
-        ASN1_OBJECT_free(objtmp);
+    ASN1_OBJECT_free(objtmp);
     return 0;
 }
 
index 2c77381..e7d56f1 100644 (file)
@@ -191,8 +191,7 @@ static int dh_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey)
  err:
     if (penc)
         OPENSSL_free(penc);
-    if (str)
-        ASN1_STRING_free(str);
+    ASN1_STRING_free(str);
 
     return 0;
 }
@@ -297,8 +296,7 @@ static int dh_priv_encode(PKCS8_PRIV_KEY_INFO *p8, const EVP_PKEY *pkey)
  err:
     if (dp != NULL)
         OPENSSL_free(dp);
-    if (params != NULL)
-        ASN1_STRING_free(params);
+    ASN1_STRING_free(params);
     if (prkey != NULL)
         ASN1_STRING_clear_free(prkey);
     return 0;
index 8975f44..668f5f3 100644 (file)
@@ -155,8 +155,7 @@ static void pkey_dh_cleanup(EVP_PKEY_CTX *ctx)
     if (dctx) {
         if (dctx->kdf_ukm)
             OPENSSL_free(dctx->kdf_ukm);
-        if (dctx->kdf_oid)
-            ASN1_OBJECT_free(dctx->kdf_oid);
+        ASN1_OBJECT_free(dctx->kdf_oid);
         OPENSSL_free(dctx);
     }
 }
@@ -245,8 +244,7 @@ static int pkey_dh_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *p2)
         return dctx->kdf_ukmlen;
 
     case EVP_PKEY_CTRL_DH_KDF_OID:
-        if (dctx->kdf_oid)
-            ASN1_OBJECT_free(dctx->kdf_oid);
+        ASN1_OBJECT_free(dctx->kdf_oid);
         dctx->kdf_oid = p2;
         return 1;
 
index d63c417..425144a 100644 (file)
@@ -166,8 +166,7 @@ static int dsa_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey)
  err:
     if (penc)
         OPENSSL_free(penc);
-    if (str)
-        ASN1_STRING_free(str);
+    ASN1_STRING_free(str);
 
     return 0;
 }
@@ -328,8 +327,7 @@ static int dsa_priv_encode(PKCS8_PRIV_KEY_INFO *p8, const EVP_PKEY *pkey)
  err:
     if (dp != NULL)
         OPENSSL_free(dp);
-    if (params != NULL)
-        ASN1_STRING_free(params);
+    ASN1_STRING_free(params);
     if (prkey != NULL)
         ASN1_STRING_clear_free(prkey);
     return 0;
index 87cc334..90de23b 100644 (file)
@@ -317,8 +317,7 @@ static int ec_asn1_group2fieldid(const EC_GROUP *group, X9_62_FIELDID *field)
         return 0;
 
     /* clear the old values (if necessary) */
-    if (field->fieldType != NULL)
-        ASN1_OBJECT_free(field->fieldType);
+    ASN1_OBJECT_free(field->fieldType);
     if (field->p.other != NULL)
         ASN1_TYPE_free(field->p.other);
 
@@ -654,7 +653,7 @@ ECPKPARAMETERS *ec_asn1_group2pkparameters(const EC_GROUP *group,
             return NULL;
         }
     } else {
-        if (ret->type == 0 && ret->value.named_curve)
+        if (ret->type == 0)
             ASN1_OBJECT_free(ret->value.named_curve);
         else if (ret->type == 1 && ret->value.parameters)
             ECPARAMETERS_free(ret->value.parameters);
index 8e87f49..34df9ac 100644 (file)
@@ -110,8 +110,7 @@ OCSP_CERTID *OCSP_cert_id_new(const EVP_MD *dgst,
         goto err;
 
     alg = cid->hashAlgorithm;
-    if (alg->algorithm != NULL)
-        ASN1_OBJECT_free(alg->algorithm);
+    ASN1_OBJECT_free(alg->algorithm);
     if ((nid = EVP_MD_type(dgst)) == NID_undef) {
         OCSPerr(OCSP_F_OCSP_CERT_ID_NEW, OCSP_R_UNKNOWN_NID);
         goto err;
index 6558116..7e502d7 100644 (file)
@@ -247,7 +247,7 @@ static void *d2i_ocsp_nonce(void *a, const unsigned char **pp, long length)
     return os;
 
  err:
-    if (os && (!pos || (*pos != os)))
+    if ((pos == NULL) || (*pos != os))
         ASN1_OCTET_STRING_free(os);
     OCSPerr(OCSP_F_D2I_OCSP_NONCE, ERR_R_MALLOC_FAILURE);
     return NULL;
index 6f4c104..071dbb8 100644 (file)
@@ -484,8 +484,7 @@ static int rsa_md_to_mgf1(X509_ALGOR **palg, const EVP_MD *mgf1md)
     X509_ALGOR_set0(*palg, OBJ_nid2obj(NID_mgf1), V_ASN1_SEQUENCE, stmp);
     stmp = NULL;
  err:
-    if (stmp)
-        ASN1_STRING_free(stmp);
+    ASN1_STRING_free(stmp);
     if (algtmp)
         X509_ALGOR_free(algtmp);
     if (*palg)
@@ -576,8 +575,7 @@ static ASN1_STRING *rsa_ctx_to_pss(EVP_PKEY_CTX *pkctx)
         RSA_PSS_PARAMS_free(pss);
     if (rv)
         return os;
-    if (os)
-        ASN1_STRING_free(os);
+    ASN1_STRING_free(os);
     return NULL;
 }
 
@@ -921,8 +919,7 @@ static int rsa_cms_encrypt(CMS_RecipientInfo *ri)
  err:
     if (oaep)
         RSA_OAEP_PARAMS_free(oaep);
-    if (os)
-        ASN1_STRING_free(os);
+    ASN1_STRING_free(os);
     return rv;
 }
 
index 6ebab3d..0f15f00 100644 (file)
@@ -138,8 +138,7 @@ int RSA_verify_ASN1_OCTET_STRING(int dtype,
     } else
         ret = 1;
  err:
-    if (sig != NULL)
-        ASN1_OCTET_STRING_free(sig);
+    ASN1_OCTET_STRING_free(sig);
     if (s != NULL) {
         OPENSSL_cleanse(s, (unsigned int)siglen);
         OPENSSL_free(s);
index 90e9970..3a8d432 100644 (file)
@@ -102,8 +102,7 @@ X509_POLICY_DATA *policy_data_new(POLICYINFO *policy,
     ret->expected_policy_set = sk_ASN1_OBJECT_new_null();
     if (!ret->expected_policy_set) {
         OPENSSL_free(ret);
-        if (id)
-            ASN1_OBJECT_free(id);
+        ASN1_OBJECT_free(id);
         return NULL;
     }
 
index 9631e57..eb9cfea 100644 (file)
@@ -212,8 +212,7 @@ static X509_EXTENSION *do_ext_i2d(const X509V3_EXT_METHOD *method,
     X509V3err(X509V3_F_DO_EXT_I2D, ERR_R_MALLOC_FAILURE);
     if (ext_der != NULL)
         OPENSSL_free(ext_der);
-    if (ext_oct != NULL)
-        ASN1_OCTET_STRING_free(ext_oct);
+    ASN1_OCTET_STRING_free(ext_oct);
     return NULL;
 
 }
index 5a93717..4139b34 100644 (file)
@@ -305,10 +305,7 @@ static PROXY_CERT_INFO_EXTENSION *r2i_pci(X509V3_EXT_METHOD *method,
     pathlen = NULL;
     goto end;
  err:
-    if (language) {
-        ASN1_OBJECT_free(language);
-        language = NULL;
-    }
+    ASN1_OBJECT_free(language);
     if (pathlen) {
         ASN1_INTEGER_free(pathlen);
         pathlen = NULL;
index 338b726..36fc571 100644 (file)
@@ -19,6 +19,7 @@ ASN1_OBJECT structure, which represents an ASN1 OBJECT IDENTIFIER.
 ASN1_OBJECT_new() allocates and initializes a ASN1_OBJECT structure.
 
 ASN1_OBJECT_free() frees up the B<ASN1_OBJECT> structure B<a>.
+If B<a> is NULL, nothing is done.
 
 =head1 NOTES
 
index 8ac2a03..6c0b303 100644 (file)
@@ -22,6 +22,7 @@ ASN1_STRING_type_new() returns an allocated B<ASN1_STRING> structure of
 type B<type>.
 
 ASN1_STRING_free() frees up B<a>.
+If B<a> is NULL nothing is done.
 
 =head1 NOTES