Fixed process_select and file_select time ranges on Windows

configure.ac

@@ -954,6 +954,11 @@ AC_CHECK_DECLS(fgetgrent, [], [], [[#include <grp.h>]])
 
 AC_CHECK_DECLS(isfinite, [], [], [[#include <math.h>]])
 
+AC_CHECK_DECL([SCNd64], [],
+  [AC_MSG_ERROR(Cannot find SCNd64 in inttypes.h)],
+  [[#include <inttypes.h>]]
+  )
+
 AC_CHECK_FUNCS(getpwent setpwent endpwent)
 
 AC_CHECK_FUNCS(fgetspent lckpwdf ulckpwdf)

libpromises/attributes.c

@@ -563,7 +563,7 @@ FileSelect GetSelectConstraints(const EvalContext *ctx, const Promise *pp)
         entries++;
     }
 
-    if (!IntegerRangeFromString(value, (long *) &s.min_size, (long *) &s.max_size))
+    if (!IntegerRangeFromString(value, &s.min_size, &s.max_size))
     {
         PromiseRef(LOG_LEVEL_ERR, pp);
         FatalError(ctx, "Could not make sense of integer range [%s]", value);
@@ -576,7 +576,7 @@ FileSelect GetSelectConstraints(const EvalContext *ctx, const Promise *pp)
         entries++;
     }
 
-    if (!IntegerRangeFromString(value, (long *) &s.min_ctime, (long *) &s.max_ctime))
+    if (!TimeRangeFromString(value, &s.min_ctime, &s.max_ctime))
     {
         PromiseRef(LOG_LEVEL_ERR, pp);
         FatalError(ctx, "Could not make sense of integer range [%s]", value);
@@ -589,7 +589,7 @@ FileSelect GetSelectConstraints(const EvalContext *ctx, const Promise *pp)
         entries++;
     }
 
-    if (!IntegerRangeFromString(value, (long *) &s.min_atime, (long *) &s.max_atime))
+    if (!TimeRangeFromString(value, &s.min_atime, &s.max_atime))
     {
         PromiseRef(LOG_LEVEL_ERR, pp);
         FatalError(ctx, "Could not make sense of integer range [%s]", value);
@@ -602,7 +602,7 @@ FileSelect GetSelectConstraints(const EvalContext *ctx, const Promise *pp)
         entries++;
     }
 
-    if (!IntegerRangeFromString(value, (long *) &s.min_mtime, (long *) &s.max_mtime))
+    if (!TimeRangeFromString(value, &s.min_mtime, &s.max_mtime))
     {
         PromiseRef(LOG_LEVEL_ERR, pp);
         FatalError(ctx, "Could not make sense of integer range [%s]", value);
@@ -1410,15 +1410,15 @@ ProcessSelect GetProcessFilterConstraints(const EvalContext *ctx, const Promise
 
     // get constraint cumulated CPU time
     value = PromiseGetConstraintAsRval(pp, "ttime_range", RVAL_TYPE_SCALAR);
-    if (!IntegerRangeFromString(value, (long *) &p.min_ttime, (long *) &p.max_ttime))
+    if (!TimeRangeFromString(value, &p.min_ttime, &p.max_ttime))
     {
         PromiseRef(LOG_LEVEL_ERR, pp);
         FatalError(ctx, "Could not make sense of integer range [%s]", value);
     }
 
     // get constraint start time
     value = PromiseGetConstraintAsRval(pp, "stime_range", RVAL_TYPE_SCALAR);
-    if (!IntegerRangeFromString(value, (long *) &p.min_stime, (long *) &p.max_stime))
+    if (!TimeRangeFromString(value, &p.min_stime, &p.max_stime))
     {
         PromiseRef(LOG_LEVEL_ERR, pp);
         FatalError(ctx, "Could not make sense of integer range [%s]", value);

libpromises/conversion.c

@@ -585,12 +585,18 @@ bool DoubleFromString(const char *s, double *value_out)
 /****************************************************************************/
 
 /**
+ * Parse a "min,max" range into 64-bit bounds.
+ *
+ * Shared implementation behind IntegerRangeFromString() and
+ * TimeRangeFromString(). Parsing is always done in 64 bits so that the typed
+ * wrappers, not this function, decide how wide the caller's storage is.
+ *
  * @return true if successful
  */
-bool IntegerRangeFromString(const char *intrange, long *min_out, long *max_out)
+static bool Int64RangeFromString(const char *intrange, int64_t *min_out, int64_t *max_out)
 {
     Item *split;
-    long lmax = CF_LOWINIT, lmin = CF_HIGHINIT;
+    int64_t lmax = CF_LOWINIT, lmin = CF_HIGHINIT;
 
 /* Numeric types are registered by range separated by comma str "min,max" */
 
@@ -603,15 +609,15 @@ bool IntegerRangeFromString(const char *intrange, long *min_out, long *max_out)
 
     split = SplitString(intrange, ',');
 
-    sscanf(split->name, "%ld", &lmin);
+    sscanf(split->name, "%" SCNd64, &lmin);
 
     if (strcmp(split->next->name, "inf") == 0)
     {
-        lmax = (long) CF_INFINITY;
+        lmax = (int64_t) CF_INFINITY;
     }
     else
     {
-        sscanf(split->next->name, "%ld", &lmax);
+        sscanf(split->next->name, "%" SCNd64, &lmax);
     }
 
     DeleteItemList(split);
@@ -626,6 +632,46 @@ bool IntegerRangeFromString(const char *intrange, long *min_out, long *max_out)
     return true;
 }
 
+/**
+ * @return true if successful
+ */
+bool IntegerRangeFromString(const char *intrange, long *min_out, long *max_out)
+{
+    int64_t min, max;
+
+    if (!Int64RangeFromString(intrange, &min, &max))
+    {
+        return false;
+    }
+
+    *min_out = min;
+    *max_out = max;
+    return true;
+}
+
+/**
+ * Like IntegerRangeFromString(), but for bounds stored as time_t.
+ *
+ * A separate entry point is needed because time_t is wider than long on LLP64
+ * platforms (Windows). Hence, writing a bound through a long * there would
+ * leave half of the caller's time_t untouched.
+ *
+ * @return true if successful
+ */
+bool TimeRangeFromString(const char *timerange, time_t *min_out, time_t *max_out)
+{
+    int64_t min, max;
+
+    if (!Int64RangeFromString(timerange, &min, &max))
+    {
+        return false;
+    }
+
+    *min_out = min;
+    *max_out = max;
+    return true;
+}
+
 AclMethod AclMethodFromString(const char *string)
 {
     static const char *const ACL_METHOD_TYPES[] =

libpromises/conversion.h

@@ -62,6 +62,7 @@ bool StringIsBoolean(const char *val);
 long IntFromString(const char *s);
 bool DoubleFromString(const char *s, double *value_out);
 bool IntegerRangeFromString(const char *intrange, long *min_out, long *max_out);
+bool TimeRangeFromString(const char *timerange, time_t *min_out, time_t *max_out);
 bool IsRealNumber(const char *s);
 
 

tests/unit/conversion_test.c

@@ -181,6 +181,75 @@ static void test_double_from_string(void)
     assert_true(val == old_val);
 }
 
+static void test_integer_range_from_string(void)
+{
+    long min, max;
+
+    assert_true(IntegerRangeFromString("0,0", &min, &max));
+    assert_int_equal(min, 0L);
+    assert_int_equal(max, 0L);
+
+    assert_true(IntegerRangeFromString("-20,19", &min, &max));
+    assert_int_equal(min, -20L);
+    assert_int_equal(max, 19L);
+
+    assert_true(IntegerRangeFromString("1,inf", &min, &max));
+    assert_int_equal(min, 1L);
+    assert_int_equal(max, CF_INFINITY);
+
+    /* An absent attribute is reported as CF_NOINT. */
+    assert_true(IntegerRangeFromString(NULL, &min, &max));
+    assert_int_equal(min, CF_NOINT);
+    assert_int_equal(max, CF_NOINT);
+}
+
+/**
+ * TimeRangeFromString() exists because time_t is wider than long on some
+ * platforms (notably Windows), where writing a bound through a long * fills
+ * only its low half and leaves the rest untouched. These cases check that
+ * bounds reach a time_t intact, at full width and with the correct sign.
+ */
+static void test_time_range_from_string(void)
+{
+    /* Pre-fill with a pattern that is wrong in both halves, so that a bound
+     * written only 4 bytes wide fails these assertions. */
+    const time_t poison = (time_t) 0x5A5A5A5A5A5A5A5A;
+    time_t min = poison, max = poison;
+
+    /* A bound that fits in 32 bits. */
+    assert_true(TimeRangeFromString("0,283824000", &min, &max));
+    assert_int_equal(min, 0);
+    assert_int_equal(max, 283824000);
+
+    /* A bound beyond INT32_MAX, which cannot survive a round trip through a
+     * 32-bit long. Policy reaches these values easily: this one is what
+     * irange(0, accumulated(99,0,0,0,0,0)) expands to, i.e. 99 years. */
+    min = poison;
+    max = poison;
+    assert_true(TimeRangeFromString("0,3122064000", &min, &max));
+    assert_int_equal(min, 0);
+    assert_int_equal(max, (time_t) 3122064000);
+
+    min = poison;
+    max = poison;
+    assert_true(TimeRangeFromString("-3122064000,-1", &min, &max));
+    assert_int_equal(min, (time_t) -3122064000);
+    assert_int_equal(max, (time_t) -1);
+
+    min = poison;
+    max = poison;
+    assert_true(TimeRangeFromString("0,inf", &min, &max));
+    assert_int_equal(min, 0);
+    assert_int_equal(max, CF_INFINITY);
+
+    /* An absent attribute is reported as CF_NOINT */
+    min = poison;
+    max = poison;
+    assert_true(TimeRangeFromString(NULL, &min, &max));
+    assert_int_equal(min, CF_NOINT);
+    assert_int_equal(max, CF_NOINT);
+}
+
 static void test_signal_from_string(void)
 {
     /* Lowercase (baseline -- should always work) */
@@ -309,6 +378,8 @@ int main()
         unit_test(test_boolean_from_string),
         unit_test(test_int_from_string),
         unit_test(test_double_from_string),
+        unit_test(test_integer_range_from_string),
+        unit_test(test_time_range_from_string),
         unit_test(test_CommandArg0_bound),
         unit_test(test_signal_from_string),
     };