Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Dec 2011 18:40:47 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Dimitry Andric <dim@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r228719 - head/usr.sbin/timed/timed
Message-ID:  <20111220181539.E15364@besplex.bde.org>
In-Reply-To: <201112192029.pBJKTpDc073856@svn.freebsd.org>
References:  <201112192029.pBJKTpDc073856@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 19 Dec 2011, Dimitry Andric wrote:

> Log:
>  Some people pointed out long is 32-bit on some arches, while time_t is
>  64-bit, so better cast time_t to intmax_t, and use the appropriate
>  printf format strings.

Some pointed out that long is adequate.  It works on all arches until
2038.  It works on all arches with 64-bit longs for a measly 292
billion years.  timed deals with current times, so times after 2038
can't happen for it yet.  So long only fails for arches still using
32-bit longs in 2038.  It's not as if casting to intmax_t is correct
in all cases.  time_t might be unsigned, or floating point.

> Modified: head/usr.sbin/timed/timed/correct.c
> ==============================================================================
> --- head/usr.sbin/timed/timed/correct.c	Mon Dec 19 20:01:21 2011	(r228718)
> +++ head/usr.sbin/timed/timed/correct.c	Mon Dec 19 20:29:50 2011	(r228719)
> @@ -162,8 +162,8 @@ adjclock(corr)
> 		}
> 	} else {
> 		syslog(LOG_WARNING,
> -		       "clock correction %ld sec too large to adjust",
> -		       (long)adj.tv_sec);
> +		       "clock correction %jd sec too large to adjust",
> +		       (intmax_t)adj.tv_sec);

This is a for time delta, so it doesn't even run out with 32-bit
longs in 2038.  It runs out when the time difference between systems
exceeds 68+ years.  If you have such a difference, then you have more
problems than misprinting it by truncating it to long.

> 		(void) gettimeofday(&now, 0);
> 		timevaladd(&now, corr);
> 		if (settimeofday(&now, 0) < 0)

> Modified: head/usr.sbin/timed/timed/readmsg.c
> ==============================================================================
> --- head/usr.sbin/timed/timed/readmsg.c	Mon Dec 19 20:01:21 2011	(r228718)
> +++ head/usr.sbin/timed/timed/readmsg.c	Mon Dec 19 20:29:50 2011	(r228719)
> @@ -181,8 +181,8 @@ again:
> 			rwait.tv_usec = 1000000/CLK_TCK;
>
> 		if (trace) {
> -			fprintf(fd, "readmsg: wait %ld.%6ld at %s\n",
> -				(long)rwait.tv_sec, rwait.tv_usec, date());
> +			fprintf(fd, "readmsg: wait %jd.%6ld at %s\n",
> +				(intmax_t)rwait.tv_sec, rwait.tv_usec, date());

This is also for a time difference.  It will soon be passed to select()
as a timeout.  If we are asking for a select timeout of 68+ years, then
we have worse problems than misprinting it in debugging code.

> 			/* Notice a full disk, as we flush trace info.
> 			 * It is better to flush periodically than at
> 			 * every line because the tracing consists of bursts
>

Supporting time differences of 292 billion years is silly.  If you do that,
then you should worry about time_t being 128-bit long double, while
intmax_t is a mere 64 bits.  128-bit long double can only go up to
3*10**17 billion years before losing precision.  But when it does, you
should be more careful not to blindly truncate it to intmax_t.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111220181539.E15364>