Date: Sun, 20 May 2012 01:39:39 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: Konstantin Belousov <kostikbel@gmail.com>, Robert Millan <rmh@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: headers that use "struct bintime" Message-ID: <20120520004236.D1313@besplex.bde.org> In-Reply-To: <14164.1337435556@critter.freebsd.dk> References: <14164.1337435556@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 19 May 2012, Poul-Henning Kamp wrote:
> In message <20120519134005.GJ2358@deviant.kiev.zoral.com.ua>, Konstantin Belous
> ov writes:
>
>>> Or maybe "struct bintime" be defined unconditionally?
>
> I think this is the best/right way to go.
Not permitted.
Exposing struct bintime also exposes its internal pollution (see below).
sys/time.h is still massively polluted in other ways:
% #ifndef _SYS_TIME_H_
% #define _SYS_TIME_H_
%
% #include <sys/_timeval.h>
% #include <sys/types.h>
Not permitted in POSIX.1, at least in the 2001 version (except all _t
names from here are permitted). Only (all of the) pollution from
including <sys/select.h> is permitted.
% #include <sys/timespec.h>
This only gives the undocumented pollution TIMESPEC_TO_TIMEVAL() and
TIMEVAL_TO_TIMESPEC() (only under __BSD_VISIBLE).
%
% struct timezone {
% int tz_minuteswest; /* minutes west of Greenwich */
% int tz_dsttime; /* type of dst correction */
% };
% #define DST_NONE 0 /* not on dst */
% #define DST_USA 1 /* USA style dst */
% #define DST_AUST 2 /* Australian style dst */
% #define DST_WET 3 /* Western European dst */
% #define DST_MET 4 /* Middle European dst */
% #define DST_EET 5 /* Eastern European dst */
% #define DST_CAN 6 /* Canada */
This compatibility cruft isn't even under __BSD_VISIBLE.
%
% #if __BSD_VISIBLE
% struct bintime {
% time_t sec;
% uint64_t frac;
% };
__BSD_VISIBLE exposes bintime and thus necessarily the namespace pollution
in its API: field names 'sec' and 'frac'.
%
% static __inline void
% bintime_addx(struct bintime *bt, uint64_t x)
% {
% uint64_t u;
__BSD_VISIBLE exposes bintime and thus unnecessarily the namespace
pollution in its implementation: parameter names 'bt' and 'x', and
local variable name 'u'. 'x' and 'u' are vey popular application
names. Though applications shouldn't use them in the global namespace
in a way they would conflict with these, they might.
%
% u = bt->frac;
% bt->frac += x;
% if (u > bt->frac)
% bt->sec++;
% }
%
% static __inline void
% bintime_add(struct bintime *bt, const struct bintime *bt2)
% {
% uint64_t u;
%
% u = bt->frac;
% bt->frac += bt2->frac;
% if (u > bt->frac)
% bt->sec++;
% bt->sec += bt2->sec;
% }
%
% static __inline void
% bintime_sub(struct bintime *bt, const struct bintime *bt2)
% {
% uint64_t u;
%
% u = bt->frac;
% bt->frac -= bt2->frac;
% if (u < bt->frac)
% bt->sec--;
% bt->sec -= bt2->sec;
% }
%
% static __inline void
% bintime_mul(struct bintime *bt, u_int x)
% {
% uint64_t p1, p2;
%
% p1 = (bt->frac & 0xffffffffull) * x;
% p2 = (bt->frac >> 32) * x + (p1 >> 32);
% bt->sec *= x;
% bt->sec += (p2 >> 32);
% bt->frac = (p2 << 32) | (p1 & 0xffffffffull);
% }
Just a few more polluting parameter and variable names.
%
% #define bintime_clear(a) ((a)->sec = (a)->frac = 0)
% #define bintime_isset(a) ((a)->sec || (a)->frac)
% #define bintime_cmp(a, b, cmp) \
% (((a)->sec == (b)->sec) ? \
% ((a)->frac cmp (b)->frac) : \
% ((a)->sec cmp (b)->sec))
This can be made to blow up more confusingly than the inlines by user
definitions of frac and sec.
%
% /*-
% * Background information:
% *
% * When converting between timestamps on parallel timescales of differing
% * resolutions it is historical and scientific practice to round down rather
% * than doing 4/5 rounding.
% *
% * The date changes at midnight, not at noon.
% *
% * Even at 15:59:59.999999999 it's not four'o'clock.
% *
% * time_second ticks after N.999999999 not after N.4999999999
% */
No comment.
%
% static __inline void
% bintime2timespec(const struct bintime *bt, struct timespec *ts)
% {
%
% ts->tv_sec = bt->sec;
% ts->tv_nsec = ((uint64_t)1000000000 * (uint32_t)(bt->frac >> 32)) >> 32;
% }
%
% static __inline void
% timespec2bintime(const struct timespec *ts, struct bintime *bt)
% {
%
% bt->sec = ts->tv_sec;
% /* 18446744073 = int(2^64 / 1000000000) */
% bt->frac = ts->tv_nsec * (uint64_t)18446744073LL;
% }
%
% static __inline void
% bintime2timeval(const struct bintime *bt, struct timeval *tv)
% {
%
% tv->tv_sec = bt->sec;
% tv->tv_usec = ((uint64_t)1000000 * (uint32_t)(bt->frac >> 32)) >> 32;
% }
%
% static __inline void
% timeval2bintime(const struct timeval *tv, struct bintime *bt)
% {
%
% bt->sec = tv->tv_sec;
% /* 18446744073709 = int(2^64 / 1000000) */
% bt->frac = tv->tv_usec * (uint64_t)18446744073709LL;
% }
Yet more parameter names in the application namespace.
% #endif /* __BSD_VISIBLE */
%
% #ifdef _KERNEL
% ... [few problems since this is private]
% #endif /* _KERNEL */
%
% #ifndef _KERNEL /* NetBSD/OpenBSD compatible interfaces */
This was intentionally left out of the kernel.
%
% #define timerclear(tvp) ((tvp)->tv_sec = (tvp)->tv_usec = 0)
% #define timerisset(tvp) ((tvp)->tv_sec || (tvp)->tv_usec)
% #define timercmp(tvp, uvp, cmp) \
% (((tvp)->tv_sec == (uvp)->tv_sec) ? \
% ((tvp)->tv_usec cmp (uvp)->tv_usec) : \
% ((tvp)->tv_sec cmp (uvp)->tv_sec))
% #define timeradd(tvp, uvp, vvp) \
% do { \
% (vvp)->tv_sec = (tvp)->tv_sec + (uvp)->tv_sec; \
% (vvp)->tv_usec = (tvp)->tv_usec + (uvp)->tv_usec; \
% if ((vvp)->tv_usec >= 1000000) { \
% (vvp)->tv_sec++; \
% (vvp)->tv_usec -= 1000000; \
% } \
% } while (0)
% #define timersub(tvp, uvp, vvp) \
% do { \
% (vvp)->tv_sec = (tvp)->tv_sec - (uvp)->tv_sec; \
% (vvp)->tv_usec = (tvp)->tv_usec - (uvp)->tv_usec; \
% if ((vvp)->tv_usec < 0) { \
% (vvp)->tv_sec--; \
% (vvp)->tv_usec += 1000000; \
% } \
% } while (0)
% #endif
Now it's massive sideways compatibility cruft.
%
% /*
% * Names of the interval timers, and structure
% * defining a timer setting.
% */
% #define ITIMER_REAL 0
% #define ITIMER_VIRTUAL 1
% #define ITIMER_PROF 2
%
% struct itimerval {
% struct timeval it_interval; /* timer interval */
% struct timeval it_value; /* current value */
% };
Now standard in POSIX, but missing ifdefs for old-POSIX.
%
% /*
% * Getkerninfo clock information structure
% */
% struct clockinfo {
% int hz; /* clock frequency */
% int tick; /* micro-seconds per hz tick */
% int spare;
% int stathz; /* statistics clock frequency */
% int profhz; /* profiling clock frequency */
% };
More FreeBSD features not under __BSD_VISIBLE.
%
% /* These macros are also in time.h. */
% #ifndef CLOCK_REALTIME
% #define CLOCK_REALTIME 0
% #define CLOCK_VIRTUAL 1
% #define CLOCK_PROF 2
% #define CLOCK_MONOTONIC 4
% #define CLOCK_UPTIME 5 /* FreeBSD-specific. */
% #define CLOCK_UPTIME_PRECISE 7 /* FreeBSD-specific. */
% #define CLOCK_UPTIME_FAST 8 /* FreeBSD-specific. */
% #define CLOCK_REALTIME_PRECISE 9 /* FreeBSD-specific. */
% #define CLOCK_REALTIME_FAST 10 /* FreeBSD-specific. */
% #define CLOCK_MONOTONIC_PRECISE 11 /* FreeBSD-specific. */
% #define CLOCK_MONOTONIC_FAST 12 /* FreeBSD-specific. */
% #define CLOCK_SECOND 13 /* FreeBSD-specific. */
% #define CLOCK_THREAD_CPUTIME_ID 14
% #endif
%
% #ifndef TIMER_ABSTIME
% #define TIMER_RELTIME 0x0 /* relative timer */
% #define TIMER_ABSTIME 0x1 /* absolute timer */
% #endif
Not permitted here in POSIX I think. Only permitted in <time.h> in
POSIX. Part of the bad layering of time.h and sys/time.h in FreeBSD.
Of course the kernel needs these too, but it can't get them from
sys/time.h according to POSIX, so it should put them in a smaller
common header. FreeBSD uses 2-3 layers of timespec.h and timeval.h
headers to do much less.
The FreeBSD extensions could be ifdefed like the signal macros are, but
this is not required.
%
% #ifdef _KERNEL
% ...
% #else /* !_KERNEL */
% #include <time.h>
Gross pollution. The worst part of the bad layering.
%
% #include <sys/cdefs.h>
Really silly to include this again, after including it already about
20 times counting nesting.
% #include <sys/select.h>
Most of this is required by POSIX and all of it is allowed (assuming that
select.h doesn't have any pollution, and it has no obvious pollution,
unlike the above).
%
% __BEGIN_DECLS
% int setitimer(int, const struct itimerval *, struct itimerval *);
% int utimes(const char *, const struct timeval *);
%
% #if __BSD_VISIBLE
% int adjtime(const struct timeval *, struct timeval *);
% int futimes(int, const struct timeval *);
% int futimesat(int, const char *, const struct timeval [2]);
% int lutimes(const char *, const struct timeval *);
% int settimeofday(const struct timeval *, const struct timezone *);
% #endif
%
% #if __XSI_VISIBLE
% int getitimer(int, struct itimerval *);
% int gettimeofday(struct timeval *, struct timezone *);
% #endif
At least setitimer() and gettimeofday() in the above aren't actually
XSI compatible, since the XSI version has some restrict qualifiers in it.
Exposing bintime and other pollution bogotifiles the careful ifdefs in some
parts of the file.
%
% __END_DECLS
%
% #endif /* !_KERNEL */
%
% #endif /* !_SYS_TIME_H_ */
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120520004236.D1313>