httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r1679032 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_private.h modules/ssl/ssl_util_stapling.c
Date Wed, 13 May 2015 14:28:33 GMT
On 05/13/2015 08:59 AM, Yann Ylavic wrote:
> On Wed, May 13, 2015 at 2:34 PM, Jeff Trawick <trawick@gmail.com> wrote:
>> Thanks again!
> You're welcome ;)
>
> WDYT of the following?
> (cosmetic only, but helps read/reuse-ability a bit)
>
> Index: modules/ssl/ssl_util_stapling.c
> ===================================================================
> --- modules/ssl/ssl_util_stapling.c    (revision 1679195)
> +++ modules/ssl/ssl_util_stapling.c    (working copy)
> @@ -250,13 +250,11 @@ static BOOL stapling_cache_response(server_rec *s,
>
>       i2d_OCSP_RESPONSE(rsp, &p);
>
> -    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
> -        stapling_cache_mutex_on(s);
> +    stapling_cache_mutex_on(s);
>       rv = mc->stapling_cache->store(mc->stapling_cache_context, s,
>                                      cinf->idx, sizeof(cinf->idx),
>                                      expiry, resp_der, stored_len, pool);
> -    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
> -        stapling_cache_mutex_off(s);
> +    stapling_cache_mutex_off(s);

At the moment I very slightly prefer seeing the reminder that there 
isn't always a mutex, but I won't care before long.  I prefer that this 
matches the implementation of the session cache mutex on where the 
socache flag is checked, but if it makes you happy and you change the 
session cache equivalent to match then go for it :)

>       if (rv != APR_SUCCESS) {
>           ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01929)
>                        "stapling_cache_response: OCSP response session
> store error!");
> @@ -277,13 +275,11 @@ static BOOL stapling_get_cached_response(server_re
>       const unsigned char *p;
>       unsigned int resp_derlen = MAX_STAPLING_DER;
>
> -    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
> -        stapling_cache_mutex_on(s);
> +    stapling_cache_mutex_on(s);
>       rv = mc->stapling_cache->retrieve(mc->stapling_cache_context, s,
>                                         cinf->idx, sizeof(cinf->idx),
>                                         resp_der, &resp_derlen, pool);
> -    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE)
> -        stapling_cache_mutex_off(s);
> +    stapling_cache_mutex_off(s);
>       if (rv != APR_SUCCESS) {
>           ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01930)
>                        "stapling_get_cached_response: cache miss");
> @@ -623,8 +619,11 @@ static int stapling_cache_mutex_on(server_rec *s)
>   {
>       SSLModConfigRec *mc = myModConfig(s);
>
> -    return stapling_mutex_on(s, mc->stapling_cache_mutex,
> -                             SSL_STAPLING_CACHE_MUTEX_TYPE);
> +    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE) {
> +        return stapling_mutex_on(s, mc->stapling_cache_mutex,
> +                                 SSL_STAPLING_CACHE_MUTEX_TYPE);
> +    }
> +    return TRUE;
>   }
>
>   static int stapling_cache_mutex_off(server_rec *s)
> @@ -631,8 +630,11 @@ static int stapling_cache_mutex_off(server_rec *s)
>   {
>       SSLModConfigRec *mc = myModConfig(s);
>
> -    return stapling_mutex_off(s, mc->stapling_cache_mutex,
> -                              SSL_STAPLING_CACHE_MUTEX_TYPE);
> +    if (mc->stapling_cache->flags & AP_SOCACHE_FLAG_NOTMPSAFE) {
> +        return stapling_mutex_off(s, mc->stapling_cache_mutex,
> +                                  SSL_STAPLING_CACHE_MUTEX_TYPE);
> +    }
> +    return TRUE;
>   }
>
>   static int stapling_refresh_mutex_on(server_rec *s)
> --


Mime
View raw message