diff mbox series

[Openvpn-devel,1/3] Move code to free cd to a function CAPI_DATA_free()

Message ID 1516770381-29466-2-git-send-email-selva.nair@gmail.com
State Superseded
Headers show
Series Support-EC-certificates-with-cryptoapicert | expand

Commit Message

Selva Nair Jan. 23, 2018, 6:06 p.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

- Avoids code-repetition especially so when support
  for more key types are added.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/cryptoapi.c | 63 ++++++++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 35 deletions(-)

Comments

Steffan Karger Jan. 25, 2018, 11:26 p.m. UTC | #1 
Hi,

On 24-01-18 06:06, selva.nair@gmail.com wrote:
> From: Selva Nair <selva.nair@gmail.com>
> 
> - Avoids code-repetition especially so when support
>   for more key types are added.
> 
> Signed-off-by: Selva Nair <selva.nair@gmail.com>
> ---
>  src/openvpn/cryptoapi.c | 63 ++++++++++++++++++++++---------------------------
>  1 file changed, 28 insertions(+), 35 deletions(-)
> 
> diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
> index f155123..00e78b6 100644
> --- a/src/openvpn/cryptoapi.c
> +++ b/src/openvpn/cryptoapi.c
> @@ -108,6 +108,32 @@ typedef struct _CAPI_DATA {
>      BOOL free_crypt_prov;
>  } CAPI_DATA;
>  
> +static void
> +CAPI_DATA_free(CAPI_DATA *cd)
> +{
> +    if (!cd)
> +    {
> +        return;
> +    }
> +    if (cd->free_crypt_prov && cd->crypt_prov)
> +    {
> +        if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
> +        {
> +            NCryptFreeObject(cd->crypt_prov);
> +        }
> +        else
> +        {
> +            CryptReleaseContext(cd->crypt_prov, 0);
> +        }
> +    }
> +    if (cd->cert_context)
> +    {
> +        CertFreeCertificateContext(cd->cert_context);
> +    }
> +    free(cd);
> +}
> +
> +

This could have been a single newline.

>  static char *
>  ms_error_text(DWORD ms_err)
>  {
> @@ -363,22 +389,7 @@ finish(RSA *rsa)
>      {
>          return 0;
>      }
> -    if (cd->crypt_prov && cd->free_crypt_prov)
> -    {
> -        if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
> -        {
> -            NCryptFreeObject(cd->crypt_prov);
> -        }
> -        else
> -        {
> -            CryptReleaseContext(cd->crypt_prov, 0);
> -        }
> -    }
> -    if (cd->cert_context)
> -    {
> -        CertFreeCertificateContext(cd->cert_context);
> -    }
> -    free(cd);
> +    CAPI_DATA_free(cd);
>      RSA_meth_free((RSA_METHOD*) rsa_meth);
>      return 1;
>  }
> @@ -614,25 +625,7 @@ err:
>          {
>              free(my_rsa_method);
>          }
> -        if (cd)
> -        {
> -            if (cd->free_crypt_prov && cd->crypt_prov)
> -            {
> -                if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
> -                {
> -                    NCryptFreeObject(cd->crypt_prov);
> -                }
> -                else
> -                {
> -                    CryptReleaseContext(cd->crypt_prov, 0);
> -                }
> -            }
> -            if (cd->cert_context)
> -            {
> -                CertFreeCertificateContext(cd->cert_context);
> -            }
> -            free(cd);
> -        }
> +        CAPI_DATA_free(cd);
>      }
>      return 0;
>  }
> 

Other than that, looks good and works as expected.  So:

Acked-by: Steffan Karger <steffan.karger@fox-it.com>

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff mbox series

Patch

diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index f155123..00e78b6 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -108,6 +108,32 @@  typedef struct _CAPI_DATA {
     BOOL free_crypt_prov;
 } CAPI_DATA;
 
+static void
+CAPI_DATA_free(CAPI_DATA *cd)
+{
+    if (!cd)
+    {
+        return;
+    }
+    if (cd->free_crypt_prov && cd->crypt_prov)
+    {
+        if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
+        {
+            NCryptFreeObject(cd->crypt_prov);
+        }
+        else
+        {
+            CryptReleaseContext(cd->crypt_prov, 0);
+        }
+    }
+    if (cd->cert_context)
+    {
+        CertFreeCertificateContext(cd->cert_context);
+    }
+    free(cd);
+}
+
+
 static char *
 ms_error_text(DWORD ms_err)
 {
@@ -363,22 +389,7 @@  finish(RSA *rsa)
     {
         return 0;
     }
-    if (cd->crypt_prov && cd->free_crypt_prov)
-    {
-        if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
-        {
-            NCryptFreeObject(cd->crypt_prov);
-        }
-        else
-        {
-            CryptReleaseContext(cd->crypt_prov, 0);
-        }
-    }
-    if (cd->cert_context)
-    {
-        CertFreeCertificateContext(cd->cert_context);
-    }
-    free(cd);
+    CAPI_DATA_free(cd);
     RSA_meth_free((RSA_METHOD*) rsa_meth);
     return 1;
 }
@@ -614,25 +625,7 @@  err:
         {
             free(my_rsa_method);
         }
-        if (cd)
-        {
-            if (cd->free_crypt_prov && cd->crypt_prov)
-            {
-                if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
-                {
-                    NCryptFreeObject(cd->crypt_prov);
-                }
-                else
-                {
-                    CryptReleaseContext(cd->crypt_prov, 0);
-                }
-            }
-            if (cd->cert_context)
-            {
-                CertFreeCertificateContext(cd->cert_context);
-            }
-            free(cd);
-        }
+        CAPI_DATA_free(cd);
     }
     return 0;
 }