Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <55535F91.2010805@gmail.com>
Date: Wed, 13 May 2015 10:28:33 -0400
From: Jeff Trawick <trawick@gmail.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
To: dev@httpd.apache.org
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
References: <20150512185930.2215BAC09F8@hades.apache.org>
	<CAKQ1sVN2eEaoUR2se39e+WUqmcWmFhL8FkqVNwc26G0zbbFOqw@mail.gmail.com>
	<55526794.3030908@gmail.com>	<555344D2.7030609@gmail.com>
 <CAKQ1sVN2uejmKJO3O4REvqJCNJEyFn36TgaADPNzCkRx_Z6z3g@mail.gmail.com>
In-Reply-To: 
 <CAKQ1sVN2uejmKJO3O4REvqJCNJEyFn36TgaADPNzCkRx_Z6z3g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

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)
> --