Changeset: 6aef6d691891 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=6aef6d691891
Modified Files:
        monetdb5/ChangeLog
        monetdb5/modules/atoms/mtime.c
        monetdb5/modules/atoms/mtime.h
Branch: default
Log Message:

Don't use bitfields in atoms that end up in BATs: their layout is not 
guaranteed.
Instead, use explicit shifting and masking.  The shifts and masks are
constructed in such a way that the fields automatically don't overlap.
No upgrade needed since these atoms (tzone and rule) are not used by
the SQL layer, so unlikely to be in a database (and the tzone was
corrupt beyond repair already in all previous versions anyway).


diffs (truncated from 733 to 300 lines):

diff --git a/monetdb5/ChangeLog b/monetdb5/ChangeLog
--- a/monetdb5/ChangeLog
+++ b/monetdb5/ChangeLog
@@ -1,3 +1,10 @@
 # ChangeLog file for MonetDB5
 # This file is updated with Maddlog
 
+* Thu Apr 18 2019 Sjoerd Mullender <[email protected]>
+- The layout of the bits in the tzone and rule atoms was changed.
+  Before they were bitfields for which the C language does not give a
+  guarantee (except that they are packed); now they are explicitly masked
+  and shifted.  These types are not used by the SQL layer and therefore
+  unlikely to be in any database.  No upgrade code has been created.
+
diff --git a/monetdb5/modules/atoms/mtime.c b/monetdb5/modules/atoms/mtime.c
--- a/monetdb5/modules/atoms/mtime.c
+++ b/monetdb5/modules/atoms/mtime.c
@@ -196,16 +196,30 @@
 extern char *strptime(const char *, const char *, struct tm *);
 #endif
 
-
-#define get_rule(r)    ((r).s.weekday | ((r).s.day<<4) | ((r).s.minutes<<10) | 
((r).s.month<<21))
-#define set_rule(r,i)                                                  \
-       do {                                                                    
        \
-               (r).s.empty = 0;                                                
\
-               (r).s.weekday = (i)&15;                                 \
-               (r).s.day = ((i)&(63<<4))>>4;                   \
-               (r).s.minutes = ((i)&(2047<<10))>>10;   \
-               (r).s.month = ((i)&(15<<21))>>21;               \
-       } while (0)
+#define rule_nil                       ((rule) int_nil)
+#define is_rule_nil(r)         ((r) == rule_nil)
+
+/* layout is based on the widths of the components */
+#define WEEKDAY_WIDTH  4
+#define DAY_WIDTH              6
+#define MINUTE_WIDTH   11
+#define MONTH_WIDTH            4
+
+#define WEEKDAY_SHIFT  0
+#define DAY_SHIFT              (WEEKDAY_SHIFT+WEEKDAY_WIDTH)
+#define MINUTE_SHIFT   (DAY_SHIFT+DAY_WIDTH)
+#define MONTH_SHIFT            (MINUTE_SHIFT+MINUTE_WIDTH)
+
+#define rule_month(r)          (((r) >> MONTH_SHIFT) & ((1<<MONTH_WIDTH)-1))
+#define rule_minutes(r)                (((r) >> MINUTE_SHIFT) & 
((1<<MINUTE_WIDTH)-1))
+#define rule_day(r)                    (((r) >> DAY_SHIFT) & 
((1<<DAY_WIDTH)-1))
+#define rule_weekday(r)                (((r) >> WEEKDAY_SHIFT) & 
((1<<WEEKDAY_WIDTH)-1))
+
+#define mkrule(month, minutes, day, weekday)   \
+       (((month) << MONTH_SHIFT) |                                     \
+        ((minutes) << MINUTE_SHIFT) |                          \
+        ((day) << DAY_SHIFT) |                                         \
+        ((weekday) << WEEKDAY_SHIFT))
 
 /* phony zero values, used to get negative numbers from unsigned
  * sub-integers in rule */
@@ -213,9 +227,32 @@ extern char *strptime(const char *, cons
 #define DAY_ZERO       32
 #define OFFSET_ZERO    4096
 
-/* as the offset field got split in two, we need macros to get and set them */
-#define get_offset(z)  (((int) (((z)->off1 << 7) + (z)->off2)) - OFFSET_ZERO)
-#define set_offset(z,i)        do { (z)->off1 = (((i)+OFFSET_ZERO)&8064) >> 7; 
(z)->off2 = ((i)+OFFSET_ZERO)&127; } while (0)
+#define tzone_nil                      ((tzone) lng_nil)
+#define is_tzone_nil(z)                ((z) == tzone_nil)
+
+/* layout is based on the widths of the components; the width of the
+ * start and end rules comes from the number of bits used for them (see
+ * above) */
+#define END_WIDTH              
(WEEKDAY_WIDTH+DAY_WIDTH+MINUTE_WIDTH+MONTH_WIDTH)
+#define START_WIDTH            
(WEEKDAY_WIDTH+DAY_WIDTH+MINUTE_WIDTH+MONTH_WIDTH)
+#define OFF_WIDTH              13
+#define        DST_WIDTH               1
+
+#define END_SHIFT              0
+#define START_SHIFT            (END_SHIFT+END_WIDTH)
+#define OFF_SHIFT              (START_SHIFT+START_WIDTH)
+#define DST_SHIFT              (OFF_SHIFT+OFF_WIDTH)
+
+#define tzone_dst(z)           (((z) >> DST_SHIFT) & ((1<<DST_WIDTH)-1))
+#define tzone_off(z)           ((int) (((z) >> OFF_SHIFT) & 
((1<<OFF_WIDTH)-1)) - OFFSET_ZERO)
+#define tzone_start(z)         (((z) >> START_SHIFT) & ((1<<START_WIDTH)-1))
+#define tzone_end(z)           (((z) >> END_SHIFT) & ((1<<END_WIDTH)-1))
+
+#define mktzone(dst, off, start, end)                                          
        \
+                       (((uint64_t) (dst) << DST_SHIFT)                        
                \
+                        | ((uint64_t) ((off) + OFFSET_ZERO) << OFF_SHIFT)      
\
+                        | ((uint64_t) (start) << START_SHIFT)                  
        \
+                        | ((uint64_t) (end) << END_SHIFT))
 
 tzone tzone_local;
 
@@ -260,12 +297,7 @@ static union {
        timestamp ts;
        lng nilval;
 } ts_nil;
-static union {
-       tzone tz;
-       lng nilval;
-} tz_nil;
 timestamp *timestamp_nil = NULL;
-static tzone *tzone_nil = NULL;
 
 int TYPE_date;
 int TYPE_daytime;
@@ -468,16 +500,16 @@ date_dayofweek(date v)
 static date
 compute_rule(const rule *val, int y)
 {
-       int m = val->s.month, cnt = abs(val->s.day - DAY_ZERO);
+       int m = rule_month(*val), cnt = abs(rule_day(*val) - DAY_ZERO);
        date d = todate(1, m, y);
        int dayofweek = date_dayofweek(d);
-       int w = abs(val->s.weekday - WEEKDAY_ZERO);
-
-       if (val->s.weekday == WEEKDAY_ZERO || w == WEEKDAY_ZERO) {
+       int w = abs(rule_weekday(*val) - WEEKDAY_ZERO);
+
+       if (rule_weekday(*val) == WEEKDAY_ZERO || w == WEEKDAY_ZERO) {
                /* cnt-th of month */
                d += cnt - 1;
-       } else if (val->s.day > DAY_ZERO) {
-               if (val->s.weekday < WEEKDAY_ZERO) {
+       } else if (rule_day(*val) > DAY_ZERO) {
+               if (rule_weekday(*val) < WEEKDAY_ZERO) {
                        /* first weekday on or after cnt-th of month */
                        SKIP_DAYS(d, dayofweek, cnt - 1);
                        cnt = 1;
@@ -488,7 +520,7 @@ compute_rule(const rule *val, int y)
                        d++;
                }
        } else {
-               if (val->s.weekday > WEEKDAY_ZERO) {
+               if (rule_weekday(*val) > WEEKDAY_ZERO) {
                        /* cnt-last weekday from end of month */
                        SKIP_DAYS(d, dayofweek, MONTHDAYS(m, y) - 1);
                } else {
@@ -511,20 +543,20 @@ static int
 timestamp_inside(timestamp *ret, const timestamp *t, const tzone *z, lng 
offset)
 {
        /* starts with GMT time t, and returns whether it is in the DST for z */
-       lng add = (offset != (lng) 0) ? offset : (get_offset(z)) * (lng) 60000;
+       lng add = (offset != 0) ? offset : tzone_off(*z) * (lng) 60000;
        int start_days, start_msecs, end_days, end_msecs, year;
        rule start, end;
 
        MTIMEtimestamp_add(ret, t, &add);
 
-       if (is_timestamp_nil(*ret) || z->dst == 0) {
+       if (is_timestamp_nil(*ret) || tzone_dst(*z) == 0) {
                return 0;
        }
-       set_rule(start, z->dst_start);
-       set_rule(end, z->dst_end);
-
-       start_msecs = start.s.minutes * 60000;
-       end_msecs = end.s.minutes * 60000;
+       start = tzone_start(*z);
+       end = tzone_end(*z);
+
+       start_msecs = rule_minutes(start) * 60000;
+       end_msecs = rule_minutes(end) * 60000;
 
        fromdate(ret->days, NULL, NULL, &year);
        start_days = compute_rule(&start, year);
@@ -779,7 +811,7 @@ daytime_tz_fromstr(const char *buf, size
                s += pos;
        } else {
                /* if no tzone is specified; work with the local */
-               offset = get_offset(&tzone_local) * (lng) -60000;
+               offset = tzone_off(tzone_local) * (lng) -60000;
        }
        val = **ret + offset;
        if (val < 0)
@@ -878,7 +910,7 @@ timestamp_fromstr(const char *buf, size_
                        /* if no tzone is specified; work with the local */
                        timestamp tmp = **ret;
 
-                       offset = get_offset(&tzone_local) * (lng) -60000;
+                       offset = tzone_off(tzone_local) * (lng) -60000;
                        if (timestamp_inside(&tmp, &tmp, &tzone_local, (lng) 
-3600000)) {
                                **ret = tmp;
                        }
@@ -916,7 +948,7 @@ timestamp_tz_fromstr(const char *buf, si
                s += pos;
        } else {
                /* if no tzone is specified; work with the local */
-               offset = get_offset(&tzone_local) * (lng) -60000;
+               offset = tzone_off(tzone_local) * (lng) -60000;
        }
        MTIMEtimestamp_add(*ret, *ret, &offset);
        return (ssize_t) (s - buf);
@@ -930,7 +962,7 @@ timestamp_tz_tostr(str *buf, size_t *len
        size_t big = 128;
        char buf1[128], buf2[128], *s = *buf, *s1 = buf1, *s2 = buf2;
        if (timezone != NULL) {
-               /* int off = get_offset(timezone); */
+               /* int off = tzone_off(*timezone); */
                timestamp tmp = *val;
 
                if (!is_timestamp_nil(tmp) && timestamp_inside(&tmp, val, 
timezone, (lng) 0)) {
@@ -1002,8 +1034,8 @@ count1(int i)
 ssize_t
 rule_tostr(str *buf, size_t *len, const rule *r, bool external)
 {
-       int hours = r->s.minutes / 60;
-       int minutes = r->s.minutes % 60;
+       int hours = rule_minutes(*r) / 60;
+       int minutes = rule_minutes(*r) % 60;
 
        if (*len < 64 || *buf == NULL) {
                GDKfree(*buf);
@@ -1011,30 +1043,30 @@ rule_tostr(str *buf, size_t *len, const 
                if( *buf == NULL)
                        return -1;
        }
-       if (is_int_nil(r->asint)) {
+       if (is_rule_nil(*r)) {
                if (external)
                        strcpy(*buf, "nil");
                else
                        strcpy(*buf, str_nil);
-       } else if (r->s.weekday == WEEKDAY_ZERO) {
+       } else if (rule_weekday(*r) == WEEKDAY_ZERO) {
                sprintf(*buf, "%s %d@%02d:%02d",
-                               MONTHS[r->s.month], r->s.day - DAY_ZERO, hours, 
minutes);
-       } else if (r->s.weekday > WEEKDAY_ZERO && r->s.day > DAY_ZERO) {
+                               MONTHS[rule_month(*r)], rule_day(*r) - 
DAY_ZERO, hours, minutes);
+       } else if (rule_weekday(*r) > WEEKDAY_ZERO && rule_day(*r) > DAY_ZERO) {
                sprintf(*buf, "%s %s from start of %s@%02d:%02d",
-                               count1(r->s.day - DAY_ZERO), DAYS[r->s.weekday 
- WEEKDAY_ZERO],
-                               MONTHS[r->s.month], hours, minutes);
-       } else if (r->s.weekday > WEEKDAY_ZERO && r->s.day < DAY_ZERO) {
+                               count1(rule_day(*r) - DAY_ZERO), 
DAYS[rule_weekday(*r) - WEEKDAY_ZERO],
+                               MONTHS[rule_month(*r)], hours, minutes);
+       } else if (rule_weekday(*r) > WEEKDAY_ZERO && rule_day(*r) < DAY_ZERO) {
                sprintf(*buf, "%s %s from end of %s@%02d:%02d",
-                               count1(DAY_ZERO - r->s.day), DAYS[r->s.weekday 
- WEEKDAY_ZERO],
-                               MONTHS[r->s.month], hours, minutes);
-       } else if (r->s.day > DAY_ZERO) {
+                               count1(DAY_ZERO - rule_day(*r)), 
DAYS[rule_weekday(*r) - WEEKDAY_ZERO],
+                               MONTHS[rule_month(*r)], hours, minutes);
+       } else if (rule_day(*r) > DAY_ZERO) {
                sprintf(*buf, "first %s on or after %s %d@%02d:%02d",
-                               DAYS[WEEKDAY_ZERO - r->s.weekday], 
MONTHS[r->s.month],
-                               r->s.day - DAY_ZERO, hours, minutes);
+                               DAYS[WEEKDAY_ZERO - rule_weekday(*r)], 
MONTHS[rule_month(*r)],
+                               rule_day(*r) - DAY_ZERO, hours, minutes);
        } else {
                sprintf(*buf, "last %s on or before %s %d@%02d:%02d",
-                               DAYS[WEEKDAY_ZERO - r->s.weekday], 
MONTHS[r->s.month],
-                               DAY_ZERO - r->s.day, hours, minutes);
+                               DAYS[WEEKDAY_ZERO - rule_weekday(*r)], 
MONTHS[rule_month(*r)],
+                               DAY_ZERO - rule_day(*r), hours, minutes);
        }
        return (ssize_t) strlen(*buf);
 }
@@ -1052,7 +1084,7 @@ rule_fromstr(const char *buf, size_t *le
                if( *d == NULL)
                        return -1;
        }
-       (*d)->asint = int_nil;
+       **d = rule_nil;
        if (strcmp(buf, str_nil) == 0)
                return 1;
        if (external && strncmp(buf, "nil", 3) == 0)
@@ -1129,11 +1161,10 @@ rule_fromstr(const char *buf, size_t *le
        if (day >= 1 && day <= LEAPDAYS[month] &&
                hours >= 0 && hours < 60 &&
                minutes >= 0 && minutes < 60) {
-               (*d)->s.empty = 0;
-               (*d)->s.month = month;
-               (*d)->s.weekday = WEEKDAY_ZERO + (neg_weekday ? -weekday : 
weekday);
-               (*d)->s.day = DAY_ZERO + (neg_day ? -day : day);
-               (*d)->s.minutes = hours * 60 + minutes;
+               **d = mkrule(month,
+                                        hours * 60 + minutes,
+                                        DAY_ZERO + (neg_day ? -day : day),
+                                        WEEKDAY_ZERO + (neg_weekday ? -weekday 
: weekday));
        }
        return (ssize_t) (cur - buf);
 }
@@ -1146,17 +1177,17 @@ tzone_fromstr(const char *buf, size_t *l
 {
        int hours = 0, minutes = 0, neg_offset = 0;
        ssize_t pos = 0;
-       rule r1, *rp1 = &r1, r2, *rp2 = &r2;
+       rule r1, r2;
        const char *cur = buf;
 
-       rp1->asint = rp2->asint = 0;
+       r1 = r2 = 0;
        if (*len < (int) sizeof(tzone) || *d == NULL) {
                GDKfree(*d);
                *d = (tzone *) GDKmalloc(*len = sizeof(tzone));
                if( *d == NULL)
                        return -1;
        }
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list

Reply via email to