[tz] localtime.c issue

Paul Eggert eggert at cs.ucla.edu
Mon Jul 8 23:03:56 UTC 2013


On 07/08/2013 03:00 PM, Andy Heninger wrote:
> Whatever the code ends up being, it would be helpful if it produced no
> compiler warnings with the warning level cranked up to the maximum, on
> either gcc or clang.

That goal is too ambitious, as there are too many diagnostics
if one enables them all, and too many of them are false alarms.

The clang warning is a false alarm, as the code is portable and
correct as-is, but I suppose it's worth working around the problem
if it's easy -- more because the code is confusing to human
readers than because clang is barfing on it.

Arthur's first workaround makes the code slower and (as
Alan mentioned) might introduce rounding errors in theory.
As Alan also mentioned, Paul's idea of multiplying by 2 might
overflow.  Arthur's second workaround of adding a cast might
work, but I prefer to avoid casts when possible, as they're
too powerful -- it's too easy to convert a pointer to an
integer by mistake, for example.

So, how about this idea instead?  It adds a
function to do the conversion, with no casts.  This lets us
add commentary explaining the situation, and it pacifies
clang for me.

>From ba39973190ea28aa07b962a59e8bf73d4f54ef7a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Mon, 8 Jul 2013 15:51:18 -0700
Subject: [PATCH] Rework to avoid bogus warning from clang
 -Wliteral-conversion.

Reported by Andy Heninger in
<http://mm.icann.org/pipermail/tz/2013-July/019446.html>.
* localtime.c (double_to_time): New function.
(timesub): Use it.
---
 localtime.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/localtime.c b/localtime.c
index 7db8ceb..d83b8c1 100644
--- a/localtime.c
+++ b/localtime.c
@@ -1434,6 +1434,18 @@ offtime(const time_t *const timep, const int_fast32_t offset)
 #endif /* defined STD_INSPIRED */
 
 /*
+** Convert T to time_t, truncating toward zero if time_t is integral.
+** On most platforms, double_to_time(0.5) returns 0; the exceptions are
+** the rare platforms where time_t is floating.
+*/
+
+static time_t
+double_to_time(double t)
+{
+	return t;
+}
+
+/*
 ** Return the number of leap years through the end of the given year
 ** where, to make the math easy, the answer for year zero is defined as zero.
 */
@@ -1514,7 +1526,7 @@ timesub(const time_t *const timep, const int_fast32_t offset,
 	}
 	{
 		register int_fast32_t	seconds;
-		register time_t		half_second = 0.5;
+		register time_t		half_second = double_to_time(0.5);
 
 		seconds = tdays * SECSPERDAY + half_second;
 		tdays = seconds / SECSPERDAY;
-- 
1.8.1.2




More information about the tz mailing list