Merge pull request #87 from zopefoundation/use-unsigned-hex-consts-in…

…-timestamp

Use unsigned constants for second arithmetic in C timestamp

CHANGES.rst

@@ -4,6 +4,11 @@
 4.3.1 (unreleased)
 ------------------
 
+- Use unsigned constants when doing arithmetic on C timestamps,
+  possibly avoiding some overflow issues with some compilers or
+  compiler settings. See `issue 86
+  <https://github.com/zopefoundation/persistent/issues/86>`_.
+
 - Change the default representation of ``Persistent`` objects to
   include the representation of their OID and jar, if set. Also add
   the ability for subclasses to implement ``_p_repr()`` instead of

persistent/_timestamp.c

@@ -27,9 +27,66 @@ static char TimeStampModule_doc[] =
 "$Id$\n";
 
 
+/* A magic constant having the value 0.000000013969839. When an
+   number of seconds between 0 and 59 is *divided* by this number, we get
+   a number between 0 (for 0), 71582786 (for 1) and 4223384393 (for 59),
+   all of which can be represented in a 32-bit unsigned integer, suitable
+   for packing into 4 bytes using `TS_PACK_UINT32_INTO_BYTES`.
+   To get (close to) the original seconds back, use
+   `TS_UNPACK_UINT32_FROM_BYTES` and *multiply* by this number.
+ */
+#define TS_SECOND_BYTES_BIAS ((double)60) / ((double)(0x10000)) / ((double)(0x10000))
+#define TS_BASE_YEAR 1900
+#define TS_MINUTES_PER_DAY 1440
+/* We pretend there are always 31 days in a month; this has us using
+   372 days in a year in some calculations */
+#define TS_DAYS_PER_MONTH 31
+#define TS_MONTHS_PER_YEAR 12
+#define TS_MINUTES_PER_MONTH (TS_DAYS_PER_MONTH * TS_MINUTES_PER_DAY)
+#define TS_MINUTES_PER_YEAR (TS_MINUTES_PER_MONTH * TS_MONTHS_PER_YEAR)
+
+/**
+ * Given an unsigned int *v*, pack it into the four
+ * unsigned char bytes beginning at *bytes*. If *v* is larger
+ * than 2^31 (i.e., it doesn't fit in 32 bits), the results will
+ * be invalid (the first byte will be 0.)
+ *
+ * The inverse is `TS_UNPACK_UINT32_FROM_BYTES`. This is a
+ * lossy operation and may lose some lower-order precision.
+ *
+ */
+#define TS_PACK_UINT32_INTO_BYTES(v, bytes) do { \
+    *(bytes) = v / 0x1000000;                      \
+    *(bytes + 1) = (v % 0x1000000) / 0x10000;      \
+    *(bytes + 2) = (v % 0x10000) / 0x100;          \
+    *(bytes + 3) = v % 0x100;                      \
+} while (0)
+
+/**
+ * Given a sequence of four unsigned chars beginning at *bytes*
+ * as produced by `TS_PACK_UINT32_INTO_BYTES`, return the
+ * original unsigned int.
+ *
+ * Remember this is a lossy operation, and the value you get back
+ * may not exactly match the original value. If the original value
+ * was greater than 2^31 it will definitely not match.
+ */
+#define TS_UNPACK_UINT32_FROM_BYTES(bytes) (*(bytes) * 0x1000000 + *(bytes + 1) * 0x10000 + *(bytes + 2) * 0x100 + *(bytes + 3))
+
 typedef struct
 {
     PyObject_HEAD
+    /*
+      The first four bytes of data store the year, month, day, hour, and
+      minute as the number of minutes since Jan 1 00:00.
+
+      The final four bytes store the seconds since 00:00 as
+      the number of microseconds.
+
+      Both are normalized into those four bytes the same way with
+      TS_[UN]PACK_UINT32_INTO|FROM_BYTES.
+    */
+
     unsigned char data[8];
 } TimeStamp;
 
@@ -49,8 +106,6 @@ static short joff[2][12] =
 
 static double gmoff=0;
 
-/* TODO:  May be better (faster) to store in a file static. */
-#define SCONV ((double)60) / ((double)(1<<16)) / ((double)(1<<16))
 
 static int
 leap(int year)
@@ -69,7 +124,7 @@ TimeStamp_yad(int y)
 {
     double d, s;
 
-    y -= 1900;
+    y -= TS_BASE_YEAR;
 
     d = (y - 1) * 365;
     if (y > 0) {
@@ -101,7 +156,7 @@ TimeStamp_init_gmoff(void)
         return -1;
     }
 
-    gmoff = TimeStamp_abst(t->tm_year+1900, t->tm_mon, t->tm_mday - 1,
+    gmoff = TimeStamp_abst(t->tm_year + TS_BASE_YEAR, t->tm_mon, t->tm_mday - 1,
                t->tm_hour * 60 + t->tm_min, t->tm_sec);
 
     return 0;
@@ -180,27 +235,26 @@ typedef struct
     int mi;
 } TimeStampParts;
 
+
 static void
 TimeStamp_unpack(TimeStamp *self, TimeStampParts *p)
 {
-    unsigned long v;
+    unsigned int minutes_since_base;
 
-    v = (self->data[0] * 16777216 + self->data[1] * 65536
-       + self->data[2] * 256 + self->data[3]);
-    p->y = v / 535680 + 1900;
-    p->m = (v % 535680) / 44640 + 1;
-    p->d = (v % 44640) / 1440 + 1;
-    p->mi = v % 1440;
+    minutes_since_base = TS_UNPACK_UINT32_FROM_BYTES(self->data);
+    p->y = minutes_since_base / TS_MINUTES_PER_YEAR + TS_BASE_YEAR;
+    p->m = (minutes_since_base % TS_MINUTES_PER_YEAR) / TS_MINUTES_PER_MONTH + 1;
+    p->d = (minutes_since_base % TS_MINUTES_PER_MONTH) / TS_MINUTES_PER_DAY + 1;
+    p->mi = minutes_since_base % TS_MINUTES_PER_DAY;
 }
 
 static double
 TimeStamp_sec(TimeStamp *self)
 {
     unsigned int v;
 
-    v = (self->data[4] * 16777216 + self->data[5] * 65536
-     + self->data[6] * 256 + self->data[7]);
-    return SCONV * v;
+    v = TS_UNPACK_UINT32_FROM_BYTES(self->data +4);
+    return TS_SECOND_BYTES_BIAS * v;
 }
 
 static PyObject *
@@ -423,13 +477,19 @@ PyObject *
 TimeStamp_FromDate(int year, int month, int day, int hour, int min,
            double sec)
 {
+
     TimeStamp *ts = NULL;
     int d;
+    unsigned int years_since_base;
+    unsigned int months_since_base;
+    unsigned int days_since_base;
+    unsigned int hours_since_base;
+    unsigned int minutes_since_base;
     unsigned int v;
 
-    if (year < 1900)
+    if (year < TS_BASE_YEAR)
         return PyErr_Format(PyExc_ValueError,
-                    "year must be greater than 1900: %d", year);
+                    "year must be greater than %d: %d", TS_BASE_YEAR, year);
     CHECK_RANGE(month, 1, 12);
     d = days_in_month(year, month - 1);
     if (day < 1 || day > d)
@@ -444,19 +504,19 @@ TimeStamp_FromDate(int year, int month, int day, int hour, int min,
                 "second must be between 0 and 59: %f", sec);
     */
     ts = (TimeStamp *)PyObject_New(TimeStamp, &TimeStamp_type);
-    v = (((year - 1900) * 12 + month - 1) * 31 + day - 1);
-    v = (v * 24 + hour) * 60 + min;
-    ts->data[0] = v / 16777216;
-    ts->data[1] = (v % 16777216) / 65536;
-    ts->data[2] = (v % 65536) / 256;
-    ts->data[3] = v % 256;
-    sec /= SCONV;
-    v = (unsigned int)sec;
-    ts->data[4] = v / 16777216;
-    ts->data[5] = (v % 16777216) / 65536;
-    ts->data[6] = (v % 65536) / 256;
-    ts->data[7] = v % 256;
+    /* months come in 1-based, hours and minutes come in 0-based */
+    /* The base time is Jan 1, 00:00 of TS_BASE_YEAR */
+    years_since_base = year - TS_BASE_YEAR;
+    months_since_base = years_since_base * TS_MONTHS_PER_YEAR + (month - 1);
+    days_since_base = months_since_base * TS_DAYS_PER_MONTH + (day - 1);
+    hours_since_base = days_since_base * 24 + hour;
+    minutes_since_base = hours_since_base * 60 + min;
 
+    TS_PACK_UINT32_INTO_BYTES(minutes_since_base, ts->data);
+
+    sec /= TS_SECOND_BYTES_BIAS;
+    v = (unsigned int)sec;
+    TS_PACK_UINT32_INTO_BYTES(v, ts->data + 4);
     return (PyObject *)ts;
 }
 

persistent/timestamp.py

@@ -61,12 +61,12 @@ def _makeUTC(y, mo, d, h, mi, s):
 
 _EPOCH = _makeUTC(1970, 1, 1, 0, 0, 0)
 
-_SCONV = 60.0 / (1<<16) / (1<<16)
+_TS_SECOND_BYTES_BIAS = 60.0 / (1<<16) / (1<<16)
 
 def _makeRaw(year, month, day, hour, minute, second):
     a = (((year - 1900) * 12 + month - 1) * 31 + day - 1)
     a = (a * 24 + hour) * 60 + minute
-    b = int(second / _SCONV) # Don't round() this; the C version does simple truncation
+    b = int(second / _TS_SECOND_BYTES_BIAS) # Don't round() this; the C version does simple truncation
     return struct.pack('>II', a, b)
 
 def _parseRaw(octets):
@@ -76,7 +76,7 @@ def _parseRaw(octets):
     day = a // (60 * 24) % 31 + 1
     month = a // (60 * 24 * 31) % 12 + 1
     year = a // (60 * 24 * 31 * 12) + 1900
-    second = b * _SCONV
+    second = b * _TS_SECOND_BYTES_BIAS
     return (year, month, day, hour, minute, second)