gst...@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.