Permalink
Browse files

Low: tools: cl#5032 Fixes issue in iso8601 calculations.

The number of days in a month is not the same for every month.
This was not taken into account correctly when adding and subtracting
days.  This also fixes an off by one error in the do_sub_field macro.
  • Loading branch information...
1 parent e7e74bb commit 1c0eae5c2c4d68db1683a99e78538955f98bb392 @davidvossel committed Mar 8, 2012
Showing with 35 additions and 5 deletions.
  1. +35 −5 lib/common/iso8601_fields.c
@@ -43,6 +43,24 @@
crm_trace("Result: %d", atime->field); \
}
+#define do_add_days_field(atime, field, extra, overflow) \
+ { \
+ int __limit = days_per_month(a_time->months, a_time->years); \
+ crm_trace("Adding %d to %d (limit=%d)", \
+ extra, atime->field, __limit); \
+ atime->field += extra; \
+ if(__limit > 0) { \
+ while(__limit < atime->field) { \
+ crm_trace("Overflowing: %d", atime->field); \
+ overflow(atime, 1); \
+ __limit = days_per_month(a_time->months, a_time->years); \
+ atime->field -= __limit; \
+ } \
+ } \
+ atime->field = atime->field; \
+ crm_trace("Result: %d", atime->field); \
+ }
+
#define do_add_time_field(atime, field, extra, limit, overflow) \
{ \
crm_trace("Adding %d to %d (limit=%d)", \
@@ -64,14 +82,28 @@
crm_trace("Subtracting %d from %d (limit=%d)", \
extra, atime->field, limit); \
atime->field -= extra; \
- while(atime->field <= 1) { \
+ while(atime->field < 1) { \
crm_trace("Underflowing: %d", atime->field); \
atime->field += limit; \
overflow(atime, 1); \
} \
crm_trace("Result: %d", atime->field); \
}
+#define do_sub_days_field(atime, field, extra, overflow) \
+ { \
+ int __limit = days_per_month(a_time->months, a_time->years); \
+ crm_trace("Subtracting %d from %d (__limit=%d)", \
+ extra, atime->field, __limit); \
+ atime->field -= extra; \
+ while(atime->field < 1) { \
+ crm_trace("Underflowing: %d", atime->field); \
+ overflow(atime, 1); \
+ __limit = days_per_month(a_time->months, a_time->years); \
+ atime->field += __limit; \
+ } \
+ crm_trace("Result: %d", atime->field); \
+ }
#define do_sub_time_field(atime, field, extra, limit, overflow) \
{ \
crm_trace("Subtracting %d from %d (limit=%d)", \
@@ -125,8 +157,7 @@ add_days(ha_time_t * a_time, int extra)
if (extra < 0) {
sub_days(a_time, -extra);
} else {
- do_add_field(a_time, days, extra,
- days_per_month(a_time->months, a_time->years), add_months);
+ do_add_days_field(a_time, days, extra, add_months);
@beekhof

beekhof Mar 8, 2012

What's this if not accounting for the number of days in a month?
What happens if you remove everything except the off-by-one fix?

@davidvossel

davidvossel Mar 8, 2012

Owner

This accounts for the days in the current month... if you add or subtract enough days the limit changes. The do_add_field macro did not account for this as it thinks the limit is always consistent.... This is not the case with days in a month. The do_add_days_field macro dynamically changes the limit based on the month as the month changes.

@davidvossel

davidvossel Mar 8, 2012

Owner

If we remove everything except the off by one fix, we still have the problem with the days in the month not being accurate in the do_add_field and do_subtract_field macros.

}
convert_from_gregorian(a_time);
@@ -280,8 +311,7 @@ sub_days(ha_time_t * a_time, int extra)
if (extra < 0) {
add_days(a_time, -extra);
} else {
- do_sub_field(a_time, days, extra,
- days_per_month(a_time->months, a_time->years), sub_months);
+ do_sub_days_field(a_time, days, extra, sub_months);
}
convert_from_gregorian(a_time);

0 comments on commit 1c0eae5

Please sign in to comment.