socache API tweaks based on chrisd's review:

* include/ap_socache.h (ap_socache_provider_t::store): Take a pool.
  (ap_socache_provider_t::retrieve): Guarantee APR_NOTFOUND for a
  "not found" result.
  (ap_socache_provider_t::remove): Return an apr_status_t.

* modules/cache/mod_socache_dc.c, modules/cache/mod_socache_dbm.c,
  modules/cache/mod_socache_shmcb,
  modules/cache/mod_socache_memcache.c: Adjust accordingly.

* modules/ssl/ssl_scache.c (ssl_scache_store): Pass pool to
  sesscache->store.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@726059 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/include/ap_socache.h b/include/ap_socache.h
index d87ce76d29ff3e7dcb31525d71c0037b3a1af66e..ceb26bb2c1fb9784859a8567a38fd78a747f98a6 100644 (file)
--- a/include/ap_socache.h
+++ b/include/ap_socache.h
@@ -116,11 +116,14 @@ typedef struct ap_socache_provider_t {
      * @param expiry Absolute time at which the object expires
      * @param data Data to store; binary blob
      * @param datalen Length of data blob
+     * @param pool Pool for temporary allocations.
+     * @return APR status value.
      */
     apr_status_t (*store)(ap_socache_instance_t *instance, server_rec *s, 
                           const unsigned char *id, unsigned int idlen, 
                           time_t expiry, 
-                          unsigned char *data, unsigned int datalen);
+                          unsigned char *data, unsigned int datalen,
+                          apr_pool_t *pool);
 
     /**
      * Retrieve a cached object.
@@ -132,6 +135,8 @@ typedef struct ap_socache_provider_t {
      * @param datalen On entry, length of data buffer; on exit, the
      * number of bytes written to the data buffer.
      * @param pool Pool for temporary allocations.
+     * @return APR status value; APR_NOTFOUND if the object was not
+     * found
      */
     apr_status_t (*retrieve)(ap_socache_instance_t *instance, server_rec *s,
                              const unsigned char *id, unsigned int idlen,
@@ -145,9 +150,9 @@ typedef struct ap_socache_provider_t {
      * @param idlen Length of id blob
      * @param pool Pool for temporary allocations.
      */
-    void (*remove)(ap_socache_instance_t *instance, server_rec *s,
-                   const unsigned char *id, unsigned int idlen,
-                   apr_pool_t *pool);
+    apr_status_t (*remove)(ap_socache_instance_t *instance, server_rec *s,
+                           const unsigned char *id, unsigned int idlen,
+                           apr_pool_t *pool);
 
     /** Dump the status of a cache instance for mod_status.  Will use
      * the ap_r* interfaces to produce appropriate status output.

diff --git a/modules/cache/mod_socache_dbm.c b/modules/cache/mod_socache_dbm.c
index da0f4dc02560d1ce5bb6b90446667bd39438e0c9..e7ffbbbfe2a3d00009cc7eb026b19876d08bd87c 100644 (file)
--- a/modules/cache/mod_socache_dbm.c
+++ b/modules/cache/mod_socache_dbm.c
@@ -71,9 +71,9 @@ struct ap_socache_instance_t {
 
 static void socache_dbm_expire(ap_socache_instance_t *ctx, server_rec *s);
 
-static void socache_dbm_remove(ap_socache_instance_t *ctx, server_rec *s, 
-                               const unsigned char *id, unsigned int idlen,
-                               apr_pool_t *p);
+static apr_status_t socache_dbm_remove(ap_socache_instance_t *ctx, 
+                                       server_rec *s, const unsigned char *id, 
+                                       unsigned int idlen, apr_pool_t *p);
 
 static const char *socache_dbm_create(ap_socache_instance_t **context, 
                                       const char *arg, 
@@ -176,10 +176,11 @@ static void socache_dbm_kill(ap_socache_instance_t *ctx, server_rec *s)
     return;
 }
 
-static apr_status_t socache_dbm_store(ap_socache_instance_t *ctx, server_rec *s,
-                                      const unsigned char *id, unsigned int idlen,
-                                      time_t expiry, unsigned char *ucaData, 
-                                      unsigned int nData)
+static apr_status_t socache_dbm_store(ap_socache_instance_t *ctx, 
+                                      server_rec *s, const unsigned char *id, 
+                                      unsigned int idlen, time_t expiry, 
+                                      unsigned char *ucaData, 
+                                      unsigned int nData, apr_pool_t *pool)
 {
     apr_dbm_t *dbm;
     apr_datum_t dbmkey;
@@ -315,9 +316,9 @@ static apr_status_t socache_dbm_retrieve(ap_socache_instance_t *ctx, server_rec
     return APR_SUCCESS;
 }
 
-static void socache_dbm_remove(ap_socache_instance_t *ctx, server_rec *s, 
-                               const unsigned char *id, unsigned int idlen,
-                               apr_pool_t *p)
+static apr_status_t socache_dbm_remove(ap_socache_instance_t *ctx, 
+                                       server_rec *s, const unsigned char *id,
+                                       unsigned int idlen, apr_pool_t *p)
 {
     apr_dbm_t *dbm;
     apr_datum_t dbmkey;
@@ -336,12 +337,12 @@ static void socache_dbm_remove(ap_socache_instance_t *ctx, server_rec *s,
                      "Cannot open SSLSessionCache DBM file `%s' for writing "
                      "(delete)",
                      ctx->data_file);
-        return;
+        return rv;
     }
     apr_dbm_delete(dbm, dbmkey);
     apr_dbm_close(dbm);
 
-    return;
+    return APR_SUCCESS;
 }
 
 static void socache_dbm_expire(ap_socache_instance_t *ctx, server_rec *s)

diff --git a/modules/cache/mod_socache_dc.c b/modules/cache/mod_socache_dc.c
index 490c88470fb1611ccb949adbd4f47b023e4ebe00..f7d78645a438b0a2c0dd771bb2fdd91e7c0e36f5 100644 (file)
--- a/modules/cache/mod_socache_dc.c
+++ b/modules/cache/mod_socache_dc.c
@@ -93,7 +93,8 @@ static void socache_dc_kill(ap_socache_instance_t *ctx, server_rec *s)
 static apr_status_t socache_dc_store(ap_socache_instance_t *ctx, server_rec *s, 
                                      const unsigned char *id, unsigned int idlen,
                                      time_t timeout,
-                                     unsigned char *der, unsigned int der_len)
+                                     unsigned char *der, unsigned int der_len,
+                                     apr_pool_t *p)
 {
     /* !@#$%^ - why do we deal with *absolute* time anyway??? */
     timeout -= time(NULL);
@@ -128,15 +129,17 @@ static apr_status_t socache_dc_retrieve(ap_socache_instance_t *ctx, server_rec *
     return APR_SUCCESS;
 }
 
-static void socache_dc_remove(ap_socache_instance_t *ctx, server_rec *s, 
-                              const unsigned char *id, unsigned int idlen, 
-                              apr_pool_t *p)
+static apr_status_t socache_dc_remove(ap_socache_instance_t *ctx, 
+                                      server_rec *s, const unsigned char *id, 
+                                      unsigned int idlen, apr_pool_t *p)
 {
     /* Remove any corresponding session from the distributed cache context */
     if (!DC_CTX_remove_session(ctx->dc, id, idlen)) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "distributed scache 'remove_session' MISS");
+        return APR_NOTFOUND;
     } else {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "distributed scache 'remove_session' HIT");
+        return APR_SUCCESS;
     }
 }
 

diff --git a/modules/cache/mod_socache_memcache.c b/modules/cache/mod_socache_memcache.c
index c8d2f5012b8987a1da6b0e57fcf449f5bf02ff46..0ac1c0019abe3005f3845950541c47675e09759d 100644 (file)
--- a/modules/cache/mod_socache_memcache.c
+++ b/modules/cache/mod_socache_memcache.c
@@ -201,7 +201,8 @@ static int socache_mc_id2key(ap_socache_instance_t *ctx,
 static apr_status_t socache_mc_store(ap_socache_instance_t *ctx, server_rec *s, 
                                      const unsigned char *id, unsigned int idlen,
                                      time_t timeout,
-                                     unsigned char *ucaData, unsigned int nData)
+                                     unsigned char *ucaData, unsigned int nData,
+                                     apr_pool_t *p)
 {
     char buf[MC_KEY_LEN];
     apr_status_t rv;
@@ -222,8 +223,7 @@ static apr_status_t socache_mc_store(ap_socache_instance_t *ctx, server_rec *s,
     return APR_SUCCESS;
 }
 
-static apr_status_t socache_mc_retrieve(ap_socache_instance_t *ctx, 
-                                        server_rec *s, 
+static apr_status_t socache_mc_retrieve(ap_socache_instance_t *ctx, server_rec *s, 
                                         const unsigned char *id, unsigned int idlen,
                                         unsigned char *dest, unsigned int *destlen,
                                         apr_pool_t *p)
@@ -259,15 +259,15 @@ static apr_status_t socache_mc_retrieve(ap_socache_instance_t *ctx,
     return APR_SUCCESS;
 }
 
-static void socache_mc_remove(ap_socache_instance_t *ctx, server_rec *s, 
-                              const unsigned char *id, unsigned int idlen,
-                              apr_pool_t *p)
+static apr_status_t socache_mc_remove(ap_socache_instance_t *ctx, server_rec *s, 
+                                      const unsigned char *id, 
+                                      unsigned int idlen, apr_pool_t *p)
 {
     char buf[MC_KEY_LEN];
     apr_status_t rv;
 
     if (socache_mc_id2key(ctx, id, idlen, buf, sizeof buf)) {
-        return;
+        return APR_EINVAL;
     }
 
     rv = apr_memcache_delete(ctx->mc, buf, 0);
@@ -276,8 +276,9 @@ static void socache_mc_remove(ap_socache_instance_t *ctx, server_rec *s,
         ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, s,
                      "scache_mc: error deleting key '%s' ",
                      buf);
-        return;
     }
+
+    return rv;
 }
 
 static void socache_mc_status(ap_socache_instance_t *ctx, request_rec *r, int flags)

diff --git a/modules/cache/mod_socache_shmcb.c b/modules/cache/mod_socache_shmcb.c
index a973f2d492f0b1b95fb0e34debb029326f587864..549e305e92ed3dd7b956a28ec4a749040b783ae7 100644 (file)
--- a/modules/cache/mod_socache_shmcb.c
+++ b/modules/cache/mod_socache_shmcb.c
@@ -447,11 +447,11 @@ static void socache_shmcb_kill(ap_socache_instance_t *ctx, server_rec *s)
 }
 
 static apr_status_t socache_shmcb_store(ap_socache_instance_t *ctx, 
-                                        server_rec *s, 
-                                        const unsigned char *id, unsigned int idlen,
-                                        time_t timeout, 
+                                        server_rec *s, const unsigned char *id, 
+                                        unsigned int idlen, time_t timeout, 
                                         unsigned char *encoded,
-                                        unsigned int len_encoded)
+                                        unsigned int len_encoded,
+                                        apr_pool_t *p)
 {
     SHMCBHeader *header = ctx->header;
     SHMCBSubcache *subcache = SHMCB_MASK(header, id);
@@ -503,12 +503,13 @@ static apr_status_t socache_shmcb_retrieve(ap_socache_instance_t *ctx,
     return rv == 0 ? APR_SUCCESS : APR_EGENERAL;
 }
 
-static void socache_shmcb_remove(ap_socache_instance_t *ctx, server_rec *s, 
-                                 const unsigned char *id, unsigned int idlen,
-                                 apr_pool_t *p)
+static apr_status_t socache_shmcb_remove(ap_socache_instance_t *ctx, 
+                                         server_rec *s, const unsigned char *id,
+                                         unsigned int idlen, apr_pool_t *p)
 {
     SHMCBHeader *header = ctx->header;
     SHMCBSubcache *subcache = SHMCB_MASK(header, id);
+    apr_status_t rv;
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "socache_shmcb_remove (0x%02x -> subcache %d)",
@@ -516,14 +517,19 @@ static void socache_shmcb_remove(ap_socache_instance_t *ctx, server_rec *s,
     if (idlen < 4) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "unusably short session_id provided "
                 "(%u bytes)", idlen);
-        return;
+        return APR_EINVAL;
     }
-    if (shmcb_subcache_remove(s, header, subcache, id, idlen))
+    if (shmcb_subcache_remove(s, header, subcache, id, idlen) == 0) {
         header->stat_removes_hit++;
-    else
+        rv = APR_SUCCESS;
+    } else {
         header->stat_removes_miss++;
+        rv = APR_NOTFOUND;
+    }
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "leaving socache_shmcb_remove successfully");
+
+    return rv;
 }
 
 static void socache_shmcb_status(ap_socache_instance_t *ctx, 

diff --git a/modules/ssl/ssl_scache.c b/modules/ssl/ssl_scache.c
index 7148cd91045b2d6189f6674323e081a2ff9dfe20..640b145fe8fc8f40e88460795fbf4719f8a31af9 100644 (file)
--- a/modules/ssl/ssl_scache.c
+++ b/modules/ssl/ssl_scache.c
@@ -114,7 +114,7 @@ BOOL ssl_scache_store(server_rec *s, UCHAR *id, int idlen,
     }
     
     rv = mc->sesscache->store(mc->sesscache_context, s, id, idlen, 
-                              expiry, encoded, len);
+                              expiry, encoded, len, p);
 
     if (mc->sesscache->flags & AP_SOCACHE_FLAG_NOTMPSAFE) {
         ssl_mutex_off(s);