Discussion:
Unification of common date/time macros
Kamil Rytarowski
2014-09-15 00:53:08 UTC
Permalink
Hello,
 
I need to use common macros (defines) for a year of Epoch (1970), seconds per day, seconds per year etc.

For user-space there is already /usr/include/tzfile.h, that contains i.a.:
#define SECSPERMIN 60
#define MINSPERHOUR 60
#define HOURSPERDAY 24
#define DAYSPERWEEK 7
#define DAYSPERNYEAR 365
#define DAYSPERLYEAR 366
#define SECSPERHOUR (SECSPERMIN * MINSPERHOUR)
#define SECSPERDAY ((long) SECSPERHOUR * HOURSPERDAY)
#define MONSPERYEAR 12
[...]
#define TM_YEAR_BASE 1900

#define EPOCH_YEAR 1970
#define EPOCH_WDAY TM_THURSDAY
[...]
#define isleap(y) ((((y) % 4) == 0 && ((y) % 100) != 0) || ((y) % 400) == 0)

For kernel-space these defines are spread across different files, drivers, architectures.
Examples:
- arch/hp300/stand/common/clock.c:
#define FEBRUARY 2
#define STARTOFTIME 1970
#define SECDAY (60L * 60L * 24L)
#define SECYR (SECDAY * 365)
[...]
#define leapyear(year) ((year) % 4 == 0)
[...]
#define days_in_year(a) (leapyear(a) ? 366 : 365)
#define days_in_month(a) (month_days[(a) - 1])
[...]

- arch/mvmeppc/stand/libsa/clock.c:
#define SECDAY (24 * 60 * 60)
#define SECYR (SECDAY * 365)
#define LEAPYEAR(y) (((y) & 3) == 0)
#define YEAR0 68
[...]

- arch/amiga/dev/rtc.h
#define STARTOFTIME 1970

- arch/atari/dev/clockreg.h
#define is_leap(x) (!(x % 4) && ((x % 100) || !(x % 1000)))
[...]
#define SECS_DAY 86400L
#define SECS_HOUR 3600L
[...]
#define BSDSTARTOFTIME 1970

- arch/luna68k/dev/timekeeper.c
#define YEAR0 1970 /* year offset */

When these numbers seem to be used to be hard-coded into algorithms, like:
- arch/vax/vax/clock.c
int
numtoyear(int num)
{
int y = 1970, j;
while(num >= (j = SECPERYEAR(y))) {
y++;
num -= j;
}
return y;
}

Well, why am I raising this topic? My code will need a set of similar macros mentioned above and I don't want to copy-paste the same piece of code, but reuse something central and well-tested. Why well tested? It's worth to see that these definitions may differ, like missing (U)L-ong modifiers for large numbers (it may overflow 16-bit integers, a standard minimum for int).

For Tru64 there was a central file /usr/sys/include/sys/machine/clock.h with relevant defines.

I'm a volunteer for a patch, but please discuss it. Where it ought to be placed? File per architecture or uniform for all of them?
Kamil Rytarowski
2014-09-15 23:37:11 UTC
Permalink
Hello,

I did some further investigation:
- FreeBSD provides NetBSD's src/syc/dev/clock_subr.h as /usr/include/sys/clock.h
- OpenBSD merged src/sys/dev/clock_subr.h with src/sys/sys/time.h [2]
- Linux kernel nothing (?)
- Tru64 as mentioned before, clock.h inside several paths:
include/alpha/clock.h
include/machine/clock.h
include/sys/machine/clock.h
sys/include/arch/alpha/clock.h
sys/include/machine/clock.h
sys/include/sys/machine/clock.h

My proposition is to go for a new file src/sys/sys/clock.h. Normalize naming with /usr/include/tzfile.h, then uniformly export the file for reuse across the kernel.

#define SECSPERMIN 60L
#define MINSPERHOUR 60L
#define HOURSPERDAY 24L
#define DAYSPERWEEK 7L
#define DAYSPERNYEAR 365L
#define DAYSPERLYEAR 366L
#define SECSPERHOUR (SECSPERMIN * MINSPERHOUR)
#define SECSPERDAY (SECSPERHOUR * HOURSPERDAY)
#define MONSPERYEAR 12L
#define EPOCH_YEAR 1970L

+ macros/defines of leap-year macro, weak-of-day etc.

Maybe avoid name-clashes with tzfile.h and go for SECSMIN etc.?

What do you think? Is it worth adding?

Thanks in advance,

[1] http://fxr.watson.org/fxr/source/sys/clock.h
[2] http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/sys/time.h.diff?r1=1.22&r2=1.23&f=h
David Holland
2014-09-22 05:30:54 UTC
Permalink
On Tue, Sep 16, 2014 at 01:37:11AM +0200, Kamil Rytarowski wrote:
> My proposition is to go for a new file src/sys/sys/clock.h. Normalize naming with /usr/include/tzfile.h, then uniformly export the file for reuse across the kernel.
>
> #define SECSPERMIN 60L
> #define MINSPERHOUR 60L
> #define HOURSPERDAY 24L
> #define DAYSPERWEEK 7L
> #define DAYSPERNYEAR 365L
> #define DAYSPERLYEAR 366L
> #define SECSPERHOUR (SECSPERMIN * MINSPERHOUR)
> #define SECSPERDAY (SECSPERHOUR * HOURSPERDAY)
> #define MONSPERYEAR 12L
> #define EPOCH_YEAR 1970L
>
> + macros/defines of leap-year macro, weak-of-day etc.
>
> Maybe avoid name-clashes with tzfile.h and go for SECSMIN etc.?
>
> What do you think? Is it worth adding?

Probably, but I'd call it sys/calendar.h. It's not really clock stuff,
in the sense that clock stuff is mostly about ticking seconds.

--
David A. Holland
***@netbsd.org
Christos Zoulas
2014-09-22 14:23:26 UTC
Permalink
In article <***@netbsd.org>,
David Holland <dholland-***@netbsd.org> wrote:
>On Tue, Sep 16, 2014 at 01:37:11AM +0200, Kamil Rytarowski wrote:
> > My proposition is to go for a new file src/sys/sys/clock.h. Normalize
>naming with /usr/include/tzfile.h, then uniformly export the file for
>reuse across the kernel.
> >
> > #define SECSPERMIN 60L
> > #define MINSPERHOUR 60L
> > #define HOURSPERDAY 24L
> > #define DAYSPERWEEK 7L
> > #define DAYSPERNYEAR 365L
> > #define DAYSPERLYEAR 366L
> > #define SECSPERHOUR (SECSPERMIN * MINSPERHOUR)
> > #define SECSPERDAY (SECSPERHOUR * HOURSPERDAY)
> > #define MONSPERYEAR 12L
> > #define EPOCH_YEAR 1970L
> >
> > + macros/defines of leap-year macro, weak-of-day etc.
> >
> > Maybe avoid name-clashes with tzfile.h and go for SECSMIN etc.?
> >
> > What do you think? Is it worth adding?
>
>Probably, but I'd call it sys/calendar.h. It's not really clock stuff,
>in the sense that clock stuff is mostly about ticking seconds.

I would separate words with _ consistently. I don't know if we should be
forcing those contacts to long anyway...

christos
Robert Elz
2014-09-22 09:50:04 UTC
Permalink
Date: Tue, 16 Sep 2014 01:37:11 +0200
From: "Kamil Rytarowski" <***@gmx.com>
Message-ID: <trinity-b05998cc-53a3-45bb-b083-35267de76e00-***@3capp-mailcom-bs14>


| My proposition is [...]
|
| #define SECSPERMIN 60L
| #define MINSPERHOUR 60L
| #define HOURSPERDAY 24L
| #define DAYSPERWEEK 7L
| #define DAYSPERNYEAR 365L
| #define DAYSPERLYEAR 366L
| #define SECSPERHOUR (SECSPERMIN * MINSPERHOUR)
| #define SECSPERDAY (SECSPERHOUR * HOURSPERDAY)
| #define MONSPERYEAR 12L
| #define EPOCH_YEAR 1970L

Why are they all to be long ? The only one that has even the slightest
potential for that need (and which is currently defined as long for the
userspace definitions) is SECSPERDAY, and that's only to cope with the
possibility that int is 16 bits (which I don't think NetBSD supports at all,
since there is no pdp11 port - but is kept that way for API consistency.)

Changing any of the others to long would change the semantics of existing
code that uses them (as would changing away from long in the kernels of
those architectures where the symbols are currently defined that way - though
that's less of a problem, internal kernel APIs can be changed when needed,
fixing the breakage is a reasonable (and bounded) amount of work.)

I have no problem with unifying the things in one common header file that
can be used by everyone, but I cannot think of a single rational reason why
DAYSPERWEEK (ie: 7) needs to be long to be useful (or the others that are
currently defined as int.)

If you really believe that some code would benefit from having them all be
long, then do it (in the new file) like ...

#ifndef _CAL_CONST_TYPE
#define _CAL_CONST_TYPE int
#endif

typedef _CAL_CONST_TYPE __cal_c_t;

#define DAYSPERWEEK ((__cal_c_t) 7)
(etc)

Then any code that needs the constants to be long for whatever reason
can just

#define _CAL_CONST_TYPE long

before including this new file or anything that includes it, so to be
safe it would be done before any includes, or on the cc command line.

Such code would really need to understand the implications however
(and if someone needs them to be long long, or even float or double,
they can have what they want too.)

[And apologies if I haven't used leading _'s the way they should be,
adjust that however is required to keep it all standards compliant.]

kre
Alan Barrett
2014-09-22 15:43:21 UTC
Permalink
On Mon, 22 Sep 2014, Robert Elz wrote:
> | My proposition is [...]
> |
> | #define SECSPERMIN 60L
> | #define MINSPERHOUR 60L
> | #define HOURSPERDAY 24L
> | #define DAYSPERWEEK 7L
> | #define DAYSPERNYEAR 365L
> | #define DAYSPERLYEAR 366L
> | #define SECSPERHOUR (SECSPERMIN * MINSPERHOUR)
> | #define SECSPERDAY (SECSPERHOUR * HOURSPERDAY)
> | #define MONSPERYEAR 12L
> | #define EPOCH_YEAR 1970L
>
> Why are they all to be long ? The only one that has even the
> slightest potential for that need (and which is currently
> defined as long for the userspace definitions) is SECSPERDAY,
> and that's only to cope with the possibility that int is 16 bits
> (which I don't think NetBSD supports at all, since there is no
> pdp11 port - but is kept that way for API consistency.)

kre's analysis is correct. I'd just define them all as plain
numbers, without any "L", "U", or "UL" suffix. I'd probably
also use 3600 and 86400 for SECSPERHOUR and SECSPERDAY, to avoid
surprises in the arithmetic.

For an example of an unwanted surprise, consider (SECSPERHOUR
* HOURSPERDAY) or ((60 * 60) * 24) on a machine with 16-bit
ints: the desired result of 86400 is too large to represent in 16
bits, which causes undefined behaviour. NetBSD doesn't support
any machines with 16-bit int, but this is the sort of code where
it's easy to accommodate such machines, so we might as well do it.

--apb (Alan Barrett)
Kamil Rytarowski
2014-09-22 16:54:19 UTC
Permalink
Hello,

Good point with reducing the (U)L modifiers, also reducing possible side-effects of 16-bit int, so going for 86400 explicitly is a good idea.

I've already proposed patches with this bug-report:
http://mail-index.netbsd.org/netbsd-bugs/2014/09/16/msg038315.html

My idea was to extract verbatim defines from clock_subr.h, put them to a file inside 'sys/' and keep 100% compatibility with files that depend on the original defines inside clock_subr.h (so add #include in it to the new .h file).

With regards,
Loading...