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

[Jeff Trawick]

On 05/13/2015 08:59 AM, Yann Ylavic wrote:
On Wed, May 13, 2015 at 2:34 PM, Jeff Trawick <traw...@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)
--