Hi, Yann and thanks for the review.

Le 21/08/2015 15:47, Yann Ylavic a écrit :
On Sun, Aug 16, 2015 at 12:05 AM,  <jaillet...@apache.org> wrote:
Author: jailletc36
Date: Sat Aug 15 22:05:08 2015
New Revision: 1696105

URL: http://svn.apache.org/r1696105
[]
Modified: httpd/httpd/trunk/modules/cache/mod_socache_memcache.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_memcache.c?rev=1696105&r1=1696104&r2=1696105&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_socache_memcache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_socache_memcache.c Sat Aug 15 22:05:08 
2015
[]
@@ -310,6 +319,35 @@ static const ap_socache_provider_t socac
[]
+static const char *socache_mc_set_ttl(cmd_parms *cmd, void *dummy,
+                                      const char *arg)
+{
+    socache_mc_svr_cfg *sconf = 
ap_get_module_config(cmd->server->module_config,
+                                                     &socache_memcache_module);
+    int i;
+
+    i = atoi(arg);
+
+    if ((i < 1) || (i > 1800)) {
Why a limit of 1800? Maybe the implicit INT_MAX is good enough.
Also the memcached may never close its connections by itself, or
always do, so -1 and 0 could also be interesting...
Obviously 0 should be allowed, but I don't see any reason for -1. What difference would you make between the 2?

INT_MAX on a 32 bits system is 2147483647. I considered that giving the TTL in sec was enough and "more standard". This has to be converted to sec. So, the maximum allowed value would be 2147. 1800 (half an hour) looked a more "logical" value for an end user. That's why I have chosen this limit.

Maybe up to 3600 (an hour) should also be allowed as it is accepted by apr_memcache_server_create.


+        return apr_pstrcat(cmd->pool, cmd->cmd->name,
+                           " must be a number between 1 and 1800.", NULL);
+    }
+
+    /* apr_memcache_server_create needs a ttl in usec. */
+    sconf->ttl = i * 1000 * 1000;
sconf->ttl = apr_time_from_sec(i) ?
Didn't do that because of the the different type (apr_time_t (i.e. apr_int64_t) vs apr_uint32_t) But using apr_time_from_sec looks good and self-document the code (i.e. ttl in usec)


+
+    return NULL;
+}
Regards,
Yann.



Reply via email to