subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <danie...@elego.de>
Subject Re: svn commit: r1307923 - /subversion/trunk/subversion/libsvn_subr/crypto.c
Date Sun, 01 Apr 2012 07:36:48 GMT
gstein@apache.org wrote on Sat, Mar 31, 2012 at 22:24:00 -0000:
> @@ -137,9 +159,15 @@ svn_crypto__context_create(svn_crypto__c
>  
>    apr_err = apr_crypto_get_driver(&driver, "openssl", NULL, &apu_err,
>                                    result_pool);
> -  if (apr_err != APR_SUCCESS || driver == NULL)
> +  /* Potential bugs in get_driver() imply we might get APR_SUCCESS and NULL.
> +     Sigh. Just be a little more careful in error generation here.  */
> +  if (apr_err != APR_SUCCESS)
>      return svn_error_create(apr_err, err_from_apu_err(apr_err, apu_err),

The code still dereferences APU_ERR when APR_ERR != APR_SUCCESS..

>                              _("OpenSSL crypto driver error"));
> +  if (driver == NULL)
> +    return svn_error_create(APR_EGENERAL,
> +                            err_from_apu_err(APR_EGENERAL, apu_err),
> +                            _("Bad return value while loading"));

The error message doesn't say much when seen without context.  (Consider
the SASL error messages that we had to s/%s/SASL said: %s/.)  IMO it
should say something about "Creating crypto context" or such.

Mime
View raw message