[ntp:questions] Re: Jonathan Buzzard's radioclkd and FreeBSD

Dave Thompson david.thompson1 at worldnet.att.net
Wed Nov 19 04:13:56 UTC 2003

On Mon, 10 Nov 2003 13:23:54 +0100, Terje Mathisen
<terje.mathisen at hda.hydro.com> wrote:

> /* These operations are valid for any date after 1600-03-01,
>     according to the modern calendar
> */

But only up to mid-1779 if unsigned int is only 16 bits, as is
permitted. Simplest to just use unsigned long, which is at least 32
bits, but may be more (e.g. I32L64); or in C99 use [u]int_least32_t
from <stdint.h> but that isn't very widespread yet although <stdint.h>
can be easily patched onto a C90 implementation; or make it adjust
automatically (using preprocessor, autoconf, or the like), but bearing
in mind that if this function is exported/published, the declared
return type seen by callers must agree with the definition (body).

> #define MASKSHIFT (8 * sizeof(int) - 1) /* 31 on a 32-bit machine */
> /* Table to simplify conversion between mm-dd and day_in_year */
> static daysToMonth[13] = {-1,30,60,91,121,152,183,213,244,274,305,336,366};
Omitting the data type (int) was always bad style and disallowed in
C99. The [12] entry isn't needed (and inconsistent anyway).
Personally I prefer to just do (m*306U+5/*or 4*/)/10, with zero-origin
d, although it is (almost certainly) a little slower.

> unsigned ymd2days(unsigned y, unsigned m, unsigned d)
> {
> 	unsigned days, y100;
> 	int mask = (int) (m -= 3);
> 	mask >>= MASKSHIFT;		/* mask is -1 if jan or feb */
> 	y += mask;
> 	m += (mask & 12);		/* Make believe the year starts in March */

This trick isn't completely safe; it is implementation defined in C
what happens for right shift of a negative (signed) integer (need not
propagate the sign, especially on a non-two's-complement machine,
which is allowed though there are very few current examples) and
completely undefined if there are any padding bits in the
representation of int and hence MASKSHIFT is > actual width; this
latter could be fixed by using >>4 which is enough (given 2sC sign
propagation) for any value in the range used, -2 to +9.

I think it's better to write what you mean; make m signed and
  if( (m -= 3) < 0 ){ m += 12; --y; }
and if on a given machine conditional or predicated or even masked
operations are better than the obvious branch, a good compiler should
choose them for you. And even if it doesn't, this code isn't very
likely to be a performance bottleneck.

> 	y -= STARTYEAR;
> 	y100 = y / 100;			/* Centuries for leap year calc */
> 	days = y * 365 + (y >> 2) - y100 + (y100 >> 2);

Given y and y100 are unsigned (and not narrower than int), the clearer
/ 4 is equivalent to >> 2, and again a decent compiler will do that.
The y*365 term needs to be done in the wider type (by casting y and/or
suffixing 365) on a machine where u/int is 16 bits. (Actually if the
machine has 16x16->32 that's enough, but there's no way to directly
represent that in C source.) The remaining terms can be done
(together) in 16-bit, if that matters.

> 	days += (unsigned) daysToMonth[m] + d;
The cast does nothing because d is already unsigned (and the same rank
as daysToMonth[]), but if days needs to be wider than uint the result
is wrong; instead you need to cast daysToMonth to the full width in
one step, or IMO clearer either cast d to signed int (or wider) and do
the right-hand addition signed, or use (d-1) and adjust daysToMonth to
be zero-origin.

> 	return days;
> }

- David.Thompson1 at worldnet.att.net

More information about the questions mailing list