On 10/15/2008 07:39 AM, Mladen Turk wrote:
> Ruediger Pluem wrote:
>>
>> I guess this leaves us to the question whether we need to be able to set
>> connectiontimeouts below one second.
>
> Right now we are using a simple atoi for parsing those
> config values. Some more advanced function for parsing the
> time should be added thought.
> Eg.
> timeout=30 (defaults to seconds)
> timeout=30ms
> timeout=30us
> timeout=30s[ec]
> timeout=1min
> timeout=1h
>
> This would be similar what we have for parsing some
> file sizes across the conf (1024, 1K, 1M, ...)
>
> I'm not aware of any standard defining that, but
> since we underneath use apr_time anything
> from 1us should be valid time (weather it makes
> sense is a different story).
How about the following:
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c (Revision 704917)
+++ modules/proxy/mod_proxy.c (Arbeitskopie)
@@ -41,8 +41,60 @@
return sizeof(proxy_worker_stat);
}
+/**
+ * Parse a given timeout parameter string into an apr_interval_time_t value.
+ * The unit of the time interval is given as postfix string to the numeric
+ * string. Currently the following units are understood:
+ *
+ * s : seconds
+ * m[s] : milliseconds
+ *
+ * If no unit is contained in the given timeout parameter the default_time_unit
+ * will be used instead.
+ * @param timeout_parameter The string containing the timeout parameter.
+ * @param timeout The timeout value to be returned.
+ * @param default_time_unit The default time unit to use if none is specified
+ * in timeout_parameter.
+ * @return Status value indicating whether the parsing was successful or not.
+ */
+/*
+ * XXX: Once this function has its final status parameter wise it makes sense
+ * to move it to some of the util??? files under server/ as public API.
+ */
+static apr_status_t ap_timeout_parameter_parse(const char *timeout_parameter,
+ apr_interval_time_t *timeout,
+ const char *default_time_unit)
+{
+ char *endp;
+ const char *time_str;
+ apr_int64_t tout;
+ tout = apr_strtoi64(timeout_parameter, &endp, 10);
+ if (errno) {
+ return errno;
+ }
+ if (!endp || !*endp) {
+ time_str = default_time_unit;
+ }
+ else {
+ time_str = endp;
+ }
+ switch (*time_str) {
+ /* Time is in seconds */
+ case 's':
+ *timeout = (apr_interval_time_t) apr_time_from_sec(tout);
+ break;
+ /* Time is in miliseconds */
+ case 'm':
+ *timeout = (apr_interval_time_t) tout * 1000;
+ break;
+ default:
+ return APR_EGENERAL;
+ }
+ return APR_SUCCESS;
+}
+
/*
* A Web proxy module. Stages:
*
@@ -77,6 +129,8 @@
{
int ival;
+ apr_interval_time_t timeout;
+
if (!strcasecmp(key, "loadfactor")) {
/* Normalized load factor. Used with BalancerMamber,
* it is a number between 1 and 100.
@@ -267,12 +321,13 @@
worker->flush_wait = ival * 1000; /* change to microseconds */
}
else if (!strcasecmp(key, "ping")) {
- /* Ping/Pong timeout in seconds.
+ /* Ping/Pong timeout in given unit (default is second).
*/
- ival = atoi(val);
- if (ival < 1)
- return "Ping/Pong timeout must be at least one second";
- worker->ping_timeout = apr_time_from_sec(ival);
+ if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
+ return "Ping/Pong timeout has wrong format";
+ if (timeout < 1)
+ return "Ping/Pong timeout must be at least one millisecond";
+ worker->ping_timeout = timeout;
worker->ping_timeout_set = 1;
}
else if (!strcasecmp(key, "lbset")) {
@@ -282,13 +337,14 @@
worker->lbset = ival;
}
else if (!strcasecmp(key, "connectiontimeout")) {
- /* Request timeout in seconds.
+ /* Request timeout in given unit (default is second).
* Defaults to connection timeout
*/
- ival = atoi(val);
- if (ival < 1)
- return "Connectiontimeout must be at least one second.";
- worker->conn_timeout = apr_time_from_sec(ival);
+ if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
+ return "Connectiontimeout has wrong format";
+ if (timeout < 1)
+ return "Connectiontimeout must be at least one millisecond.";
+ worker->conn_timeout = timeout;
worker->conn_timeout_set = 1;
}
else {
Regards
Rüdiger
