On 04/01/2012 03:46 PM, Alon Bar-Lev wrote:
> Cleanup of "Use the garbage collector when retrieving x509 fields"
> patch series.
>
> Discussed at [1].
>
> There should be an effort to produce common function prologue
> and epilogue, so that cleanups will be done at single point.
>
> [1] http://comments.gmane.org/gmane.network.openvpn.devel/5401
>
> Signed-off-by: Alon Bar-Lev <alon.bar...@gmail.com>
> ---
>  
>  int
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index 5783528..30fb05d 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c
> @@ -538,8 +538,9 @@ verify_cert_call_command(const char *verify_command, 
> struct env_set *es,
>  static result_t
>  verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert)
>  {
> +  result_t ret = FAILURE;
>    char fn[256];
> -  int fd;
> +  int fd = -1;
>    struct gc_arena gc = gc_new();
>  
>    char *serial = x509_get_serial(cert, &gc);
> @@ -547,26 +548,29 @@ verify_check_crl_dir(const char *crl_dir, 
> openvpn_x509_cert_t *cert)
>    if (!openvpn_snprintf(fn, sizeof(fn), "%s%c%s", crl_dir, 
> OS_SPECIFIC_DIRSEP, serial))
>      {
>        msg (D_HANDSHAKE, "VERIFY CRL: filename overflow");
> -      gc_free(&gc);
> -      return FAILURE;
> +      goto cleanup;
>      }
>    fd = platform_open (fn, O_RDONLY, 0);
>    if (fd >= 0)
>      {
>        msg (D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is 
> revoked", serial);
> -      close(fd);
> -      gc_free(&gc);
> -      return FAILURE;
> +      goto cleanup;
>      }
>  
> -  gc_free(&gc);
> +  ret = SUCCESS;
>  
> -  return SUCCESS;
> +cleanup:
> +
> +  if (fd != -1)
> +    close(fd);
> +  gc_free(&gc);
> +  return ret;
>  }
>  
Thanks, looks good. Does platform_open return -1 on failure on all
platforms? If so, ack, otherwise change that to < 0.

Adriaan

Reply via email to