Fix edge-case behavior of pg_next_dst_boundary().

tglsfdc · tglsfdc · commit b0f24b562672 · 2012-04-25T17:25:24.000-04:00
Due to rather sloppy thinking (on my part, I'm afraid) about the
appropriate behavior for boundary conditions, pg_next_dst_boundary() gave
undefined, platform-dependent results when the input time is exactly the
last recorded DST transition time for the specified time zone, as a result
of fetching values one past the end of its data arrays.

Change its specification to be that it always finds the next DST boundary
*after* the input time, and adjust code to match that.  The sole existing
caller, DetermineTimeZoneOffset, doesn't actually care about this
distinction, since it always uses a probe time earlier than the instant
that it does care about.  So it seemed best to me to change the API to make
the result=1 and result=0 cases more consistent, specifically to ensure
that the "before" outputs always describe the state at the given time,
rather than hacking the code to obey the previous API comment exactly.

Per bug #6605 from Sergey Burladyan.  Back-patch to all supported versions.
diff --git a/src/timezone/localtime.c b/src/timezone/localtime.c
@@ -1290,20 +1290,21 @@ increment_overflow(int *number, int delta)
 }
 
 /*
- * Find the next DST transition time at or after the given time
+ * Find the next DST transition time after the given time
  *
  * *timep is the input value, the other parameters are output values.
  *
  * When the function result is 1, *boundary is set to the time_t
- * representation of the next DST transition time at or after *timep,
+ * representation of the next DST transition time after *timep,
  * *before_gmtoff and *before_isdst are set to the GMT offset and isdst
- * state prevailing just before that boundary, and *after_gmtoff and
- * *after_isdst are set to the state prevailing just after that boundary.
+ * state prevailing just before that boundary (in particular, the state
+ * prevailing at *timep), and *after_gmtoff and *after_isdst are set to
+ * the state prevailing just after that boundary.
  *
- * When the function result is 0, there is no known DST transition at or
+ * When the function result is 0, there is no known DST transition
  * after *timep, but *before_gmtoff and *before_isdst indicate the GMT
  * offset and isdst state prevailing at *timep.  (This would occur in
- * DST-less time zones, for example.)
+ * DST-less time zones, or if a zone has permanently ceased using DST.)
  *
  * A function result of -1 indicates failure (this case does not actually
  * occur in our current implementation).
@@ -1383,16 +1384,16 @@ pg_next_dst_boundary(const pg_time_t *timep,
 		return result;
 	}
 
-	if (t > sp->ats[sp->timecnt - 1])
+	if (t >= sp->ats[sp->timecnt - 1])
 	{
-		/* No known transition >= t, so use last known segment's type */
+		/* No known transition > t, so use last known segment's type */
 		i = sp->types[sp->timecnt - 1];
 		ttisp = &sp->ttis[i];
 		*before_gmtoff = ttisp->tt_gmtoff;
 		*before_isdst = ttisp->tt_isdst;
 		return 0;
 	}
-	if (t <= sp->ats[0])
+	if (t < sp->ats[0])
 	{
 		/* For "before", use lowest-numbered standard type */
 		i = 0;
@@ -1413,10 +1414,10 @@ pg_next_dst_boundary(const pg_time_t *timep,
 		*after_isdst = ttisp->tt_isdst;
 		return 1;
 	}
-	/* Else search to find the containing segment */
+	/* Else search to find the boundary following t */
 	{
 		int			lo = 1;
-		int			hi = sp->timecnt;
+		int			hi = sp->timecnt - 1;
 
 		while (lo < hi)
 		{

Original file line number	Diff line number	Diff line change
`@@ -1290,20 +1290,21 @@ increment_overflow(int *number, int delta)`
`1290`	`1290`	`}`
`1291`	`1291`
`1292`	`1292`	`/*`
`1293`		`- * Find the next DST transition time at or after the given time`
	`1293`	`+ * Find the next DST transition time after the given time`
`1294`	`1294`	`*`
`1295`	`1295`	`* *timep is the input value, the other parameters are output values.`
`1296`	`1296`	`*`
`1297`	`1297`	`* When the function result is 1, *boundary is set to the time_t`
`1298`		`- * representation of the next DST transition time at or after *timep,`
	`1298`	`+ * representation of the next DST transition time after *timep,`
`1299`	`1299`	`* before_gmtoff and before_isdst are set to the GMT offset and isdst`
`1300`		`- * state prevailing just before that boundary, and *after_gmtoff and`
`1301`		`- * *after_isdst are set to the state prevailing just after that boundary.`
	`1300`	`+ * state prevailing just before that boundary (in particular, the state`
	`1301`	`+ * prevailing at timep), and after_gmtoff and *after_isdst are set to`
	`1302`	`+ * the state prevailing just after that boundary.`
`1302`	`1303`	`*`
`1303`		`- * When the function result is 0, there is no known DST transition at or`
	`1304`	`+ * When the function result is 0, there is no known DST transition`
`1304`	`1305`	`* after timep, but before_gmtoff and *before_isdst indicate the GMT`
`1305`	`1306`	`* offset and isdst state prevailing at *timep. (This would occur in`
`1306`		`- * DST-less time zones, for example.)`
	`1307`	`+ * DST-less time zones, or if a zone has permanently ceased using DST.)`
`1307`	`1308`	`*`
`1308`	`1309`	`* A function result of -1 indicates failure (this case does not actually`
`1309`	`1310`	`* occur in our current implementation).`
`@@ -1383,16 +1384,16 @@ pg_next_dst_boundary(const pg_time_t *timep,`
`1383`	`1384`	`return result;`
`1384`	`1385`	`}`
`1385`	`1386`
`1386`		`- if (t > sp->ats[sp->timecnt - 1])`
	`1387`	`+ if (t >= sp->ats[sp->timecnt - 1])`
`1387`	`1388`	`{`
`1388`		`- /* No known transition >= t, so use last known segment's type */`
	`1389`	`+ /* No known transition > t, so use last known segment's type */`
`1389`	`1390`	`i = sp->types[sp->timecnt - 1];`
`1390`	`1391`	`ttisp = &sp->ttis[i];`
`1391`	`1392`	`*before_gmtoff = ttisp->tt_gmtoff;`
`1392`	`1393`	`*before_isdst = ttisp->tt_isdst;`
`1393`	`1394`	`return 0;`
`1394`	`1395`	`}`
`1395`		`- if (t <= sp->ats[0])`
	`1396`	`+ if (t < sp->ats[0])`
`1396`	`1397`	`{`
`1397`	`1398`	`/* For "before", use lowest-numbered standard type */`
`1398`	`1399`	`i = 0;`
`@@ -1413,10 +1414,10 @@ pg_next_dst_boundary(const pg_time_t *timep,`
`1413`	`1414`	`*after_isdst = ttisp->tt_isdst;`
`1414`	`1415`	`return 1;`
`1415`	`1416`	`}`
`1416`		`- /* Else search to find the containing segment */`
	`1417`	`+ /* Else search to find the boundary following t */`
`1417`	`1418`	`{`
`1418`	`1419`	`int lo = 1;`
`1419`		`- int hi = sp->timecnt;`
	`1420`	`+ int hi = sp->timecnt - 1;`
`1420`	`1421`
`1421`	`1422`	`while (lo < hi)`
`1422`	`1423`	`{`