param: Optionally pass a duration unit to timeouts

Allowing units is much more convenient and keeping them optional
maintains compatibility. On the other hand, changing the defaults
for default_ttl and default_grace has no impact on the generated
RST.

Keeping track of the user input could help since default values
are "washed" like user input, without resorting to a dynamic
default.

Refs #3756

bin/varnishd/mgt/mgt_param_tweak.c

@@ -52,6 +52,8 @@ const char * const JSON_FMT = (const char *)&JSON_FMT;
  * Generic handling of double typed parameters
  */
 
+typedef double parse_double_f(const char *, const char **);
+
 static double
 parse_decimal(const char *p, const char **err)
 {
@@ -67,7 +69,7 @@ parse_decimal(const char *p, const char **err)
 
 static int
 tweak_generic_double(struct vsb *vsb, const char *arg, const struct parspec *pp,
-    const char *fmt)
+    parse_double_f parse, const char *fmt)
 {
 	volatile double u, minv = VRT_DECIMAL_MIN, maxv = VRT_DECIMAL_MAX;
 	volatile double *dest = pp->priv;
@@ -75,21 +77,21 @@ tweak_generic_double(struct vsb *vsb, const char *arg, const struct parspec *pp,
 
 	if (arg != NULL && arg != JSON_FMT) {
 		if (pp->min != NULL) {
-			minv = parse_decimal(pp->min, &err);
+			minv = parse(pp->min, &err);
 			if (errno) {
 				VSB_printf(vsb, "Min: %s (%s)\n", err, pp->min);
 				return (-1);
 			}
 		}
 		if (pp->max != NULL) {
-			maxv = parse_decimal(pp->max, &err);
+			maxv = parse(pp->max, &err);
 			if (errno) {
 				VSB_printf(vsb, "Max: %s (%s)\n", err, pp->max);
 				return (-1);
 			}
 		}
 
-		u = parse_decimal(arg, &err);
+		u = parse(arg, &err);
 		if (errno) {
 			VSB_printf(vsb, "%s (%s)\n", err, arg);
 			return (-1);
@@ -113,11 +115,29 @@ tweak_generic_double(struct vsb *vsb, const char *arg, const struct parspec *pp,
 
 /*--------------------------------------------------------------------*/
 
+static double
+parse_duration(const char *p, const char **err)
+{
+	double v, r;
+
+	v = SF_Parse_Decimal(&p, 0, err);
+	if (*p == '\0')
+		return (v);
+
+	r = VNUM_duration_unit(v, p, NULL);
+	if (isnan(r)) {
+		errno = EINVAL;
+		*err = "Invalid duration unit";
+	}
+
+	return (r);
+}
+
 int v_matchproto_(tweak_t)
 tweak_timeout(struct vsb *vsb, const struct parspec *par, const char *arg)
 {
 
-	return (tweak_generic_double(vsb, arg, par, "%.3f"));
+	return (tweak_generic_double(vsb, arg, par, parse_duration, "%.3f"));
 }
 
 /*--------------------------------------------------------------------*/
@@ -126,7 +146,7 @@ int v_matchproto_(tweak_t)
 tweak_double(struct vsb *vsb, const struct parspec *par, const char *arg)
 {
 
-	return (tweak_generic_double(vsb, arg, par, "%g"));
+	return (tweak_generic_double(vsb, arg, par, parse_decimal, "%g"));
 }
 
 /*--------------------------------------------------------------------*/
@@ -447,7 +467,8 @@ tweak_poolparam(struct vsb *vsb, const struct parspec *par, const char *arg)
 			pt.priv = &px.max_age;
 			pt.min = "0";
 			pt.max = "1000000";
-			retval = tweak_generic_double(vsb, av[3], &pt, "%.0f");
+			retval = tweak_generic_double(vsb, av[3], &pt,
+			    parse_decimal, "%.0f");
 			if (retval)
 				break;
 			if (px.min_pool > px.max_pool) {

bin/varnishtest/tests/b00042.vtc

@@ -4,6 +4,10 @@ varnish v1 -vcl {backend be none;} -start
 
 
 varnish v1 -clierr 106	"param.set default_ttl -1"
+varnish v1 -clierr 106	"param.set default_ttl 1x"
+varnish v1 -cliok	"param.set default_ttl 1s"
+varnish v1 -clierr 106	{param.set default_ttl "1 x"}
+varnish v1 -cliok	{param.set default_ttl "1 s"}
 varnish v1 -clierr 106	{param.set acceptor_sleep_decay "0.42 is not a number"}
 varnish v1 -clierr 106	"param.set acceptor_sleep_max 20"
 varnish v1 -cliok	"param.set prefer_ipv6 off"

include/tbl/params.h

@@ -385,13 +385,16 @@ PARAM_SIMPLE(
 	/* type */	timeout,
 	/* min */	"0.000",
 	/* max */	NULL,
-	/* def */	"10.000",
+	/* def */	"10s",
 	/* units */	"seconds",
 	/* descr */
 	"Default grace period.  We will deliver an object this long after "
 	"it has expired, provided another thread is attempting to get a "
 	"new copy.",
-	/* flags */	OBJ_STICKY
+	/* flags */	OBJ_STICKY,
+	/* dyn_min_reason */	NULL,
+	/* dyn_max_reason */	NULL,
+	/* dyn_def_reason */	"10s"
 )
 
 PARAM_SIMPLE(
@@ -399,14 +402,17 @@ PARAM_SIMPLE(
 	/* type */	timeout,
 	/* min */	"0.000",
 	/* max */	NULL,
-	/* def */	"0.000",
+	/* def */	"0s",
 	/* units */	"seconds",
 	/* descr */
 	"Default keep period.  We will keep a useless object around this "
 	"long, making it available for conditional backend fetches.  That "
 	"means that the object will be removed from the cache at the end "
 	"of ttl+grace+keep.",
-	/* flags */	OBJ_STICKY
+	/* flags */	OBJ_STICKY,
+	/* dyn_min_reason */	NULL,
+	/* dyn_max_reason */	NULL,
+	/* dyn_def_reason */	"0s"
 )
 
 PARAM_SIMPLE(
@@ -414,12 +420,15 @@ PARAM_SIMPLE(
 	/* type */	timeout,
 	/* min */	"0.000",
 	/* max */	NULL,
-	/* def */	"120.000",
+	/* def */	"2m",
 	/* units */	"seconds",
 	/* descr */
 	"The TTL assigned to objects if neither the backend nor the VCL "
 	"code assigns one.",
-	/* flags */	OBJ_STICKY
+	/* flags */	OBJ_STICKY,
+	/* dyn_min_reason */	NULL,
+	/* dyn_max_reason */	NULL,
+	/* dyn_def_reason */	"2m"
 )
 
 PARAM_SIMPLE(