Date: Sun, 14 Nov 2010 21:06:17 -0800 From: Garrett Cooper <gcooper@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: standards@freebsd.org Subject: Re: Question about non-__BSD_VISIBLE guarded CLOCK_* constants Message-ID: <AANLkTin9Cr2pyJKRKcRoVXk_SAGvDmpF5qUwfkxMkK3s@mail.gmail.com> In-Reply-To: <20101109015704.L1243@besplex.bde.org> References: <AANLkTim-CuFGqeC%2BObg0yphm2oaj23pzdVcnQhVpoZwz@mail.gmail.com> <20101109015704.L1243@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 8, 2010 at 7:59 AM, Bruce Evans <brde@optusnet.com.au> wrote:
> On Sun, 7 Nov 2010, Garrett Cooper wrote:
>
>> =A0 None of the following constants in time.h are guarded by
>> __BSD_VISIBLE, __FreeBSD__, etc, even though other sections of the
>> file are blocked off that way (and the comments suggest that they're
>> FreeBSD-specific). I was wondering why that's the case...
>
> It is because the other sections were written by standards-aware persons.
>
>> #define CLOCK_UPTIME =A0 =A05 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* FreeBSD-spe=
cific. */
>> #define CLOCK_UPTIME_PRECISE =A0 =A07 =A0 =A0 =A0 /* FreeBSD-specific. *=
/
>> #define CLOCK_UPTIME_FAST =A0 =A0 =A0 8 =A0 =A0 =A0 /* FreeBSD-specific.=
*/
>> #define CLOCK_REALTIME_PRECISE =A09 =A0 =A0 =A0 /* FreeBSD-specific. */
>> #define CLOCK_REALTIME_FAST =A0 =A0 10 =A0 =A0 =A0/* FreeBSD-specific. *=
/
>> #define CLOCK_MONOTONIC_PRECISE 11 =A0 =A0 =A0/* FreeBSD-specific. */
>> #define CLOCK_MONOTONIC_FAST =A0 =A012 =A0 =A0 =A0/* FreeBSD-specific. *=
/
>> #define CLOCK_SECOND =A0 =A013 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* FreeBSD-spe=
cific. */
>
> Defining these without using __BSD_VISBLE is just a style bug, since the
> CLOCK_ prefix is reserved in <time.h>, but FreeBSD normally ifdefs
> extensions that use reserved prefixes anyway (e.g., FreeBSD-specific
> signal numbers at least used to be ifdefed to a fault in <signal.h>.
>
> The definitions themselves have lots of style bugs:
> - space instead of tab after #define (all tabs were corrupted in the mail=
,
> =A0except these are corrupt in the file)
> - macro values don't line up, although they are indented with tabs
> - comment duplicated ad nauseum
> - there is now an id 14. =A0It is missing the comment, and seems to be
> =A0undocumented.
>
>> =A0 Also, they're blocked off by #if !defined(CLOCK_REALTIME) &&
>> __POSIX_VISIBLE >=3D 200112 , which doesn't seem to make sense, given
>> that it's an "advanced realtime" feature, according to POSIX 2008.
>
> Re-quoting everything to get more context.
>
> % /* These macros are also in sys/time.h. */
>
> <sys/time.h> is considerably more broken than here. =A0It defines all of
> these macros unconditionally, and also includes <time.h> in the !_KERNEL
> case. =A0Thus including the POSIX header <sys/time.h> gives massive names=
pace
> pollution.
>
> % #if !defined(CLOCK_REALTIME) && __POSIX_VISIBLE >=3D 200112
> % #define CLOCK_REALTIME =A0 =A0 =A0 =A00
> % #ifdef __BSD_VISIBLE
> % #define CLOCK_VIRTUAL 1
> % #define CLOCK_PROF =A0 =A02
> % #endif
> % #define CLOCK_MONOTONIC =A0 =A0 =A0 4
> /* These macros are also in sys/time.h. */
> #define CLOCK_MONOTONIC 4
> #define CLOCK_UPTIME =A0 =A05 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* FreeBSD-spec=
ific. */
> #define ...
> #endif /* !defined(CLOCK_REALTIME) && __POSIX_VISIBLE >=3D 200112 */
>
> The CLOCK_REALTIME part of this does nothing except partially break
> automatic checking that the defines here are the same as the ones in
> <sys/time.h>:
> - if <sys/time.h> is not included before here, then CLOCK_REALTIME should=
n't
> =A0be defined yet, so it has no effect in this ifdef
> - if <sys/time.h> is =A0 =A0 included before here, then CLOCK_REALTIME
> =A0is defined now, so its effect in this ifdef is to prevent repeating if=
defs
> =A0that should be the same. =A0Repeated ifdefs are a very small part of h=
eader
> =A0bloat, so it is hardly worth avoiding them, and not avoiding them give=
s the
> =A0feature of checking that they really are repeated. =A0I think the repe=
titions
> =A0are all indentical, including their style bugs and whitespace that wou=
ld
> =A0not be checked.
> - if <sys/time.h> is not included before here, but is included later, the=
n
> =A0CLOCK_REALTIME etc. gets declared here, and since <sys/time.h> has no
> =A0similar ifdefs, it gets defined again later. =A0Thus the automatic che=
cking
> =A0is only partially broken.
>
> As you noticed, the __POSIX_VISIBLE part of this has the wrong version
> number
> at best. =A0CLOCK_REALTIME dates from POSIX.4 in ~1994 (it is in my POSIX=
.4
> book published in 1995, and in the 1996 POSIX standard). =A0FreeBSD doesn=
't
> try to track old versions of POSIX very carefully, except original POSIX,
> but it normally makes features that appeared in in-between POSIXes visibl=
e
> by default by putting them under __BSD_VISIBLE.
>
> So the correct ifdefs here aresomething like __BSD_VISIBLE ||
> POSIX_VISIBLE >=3D 199309 (check the latter) for the whole block, and
> __BSD_VISIBLE for the sub-block consisting of FreeBSD extensions.
> Since the organization is poor (historical, giving random alphabetical an=
d
> numericl order), there are actually several sub-blocks:
> - CLOCK_MONOTONIC seems to be new in POSIX.1-2001. =A0Thus the current if=
def
> =A0is partially correct for it. =A0The rest of the file has careful ifdef=
s for
> =A01993 POSIX, so perhaps it is worth being equally careful for
> CLOCK_REALTIME.
> - the BSD extensions CLOCK_VIRTUAL and CLOCK_PROF are already in a
> =A0__BSD_VISIBLE block. =A0Then there seems to be space for expansion (a
> =A0whole 1 id =3D 3). =A0Then there is the whole 1 new id =3D 4 for 2001 =
POSIX.
> =A0Then there is a FreeBSD-specific block (ids 5-14). =A0Too much bloat f=
or me.
Looks like I got the isolation working properly:
# gcc -D_BSD_SOURCE -o ~gcooper/test_clock_uptime ~gcooper/test_clock_uptim=
e.c
# ~gcooper/test_clock_uptime
100031 661918304
# gcc -D_POSIX_C_SOURCE=3D200809 -D_POSIX_SOURCE -o
~gcooper/test_clock_uptime ~gcooper/test_clock_uptime.c
/home/gcooper/test_clock_uptime.c: In function 'main':
/home/gcooper/test_clock_uptime.c:10: error: 'CLOCK_UPTIME' undeclared
(first use in this function)
/home/gcooper/test_clock_uptime.c:10: error: (Each undeclared
identifier is reported only once
/home/gcooper/test_clock_uptime.c:10: error: for each function it appears i=
n.)
Removing time.h looks like a mini-project in and of itself, as
I've had to go fix a handful of files in libc that were looking for
clock or timer definitions that were defined in time.h according to
POSIX, found a few bugs elsewhere when running buildworld, etc. I'll
be submitting patches for all of these items eventually, but it looks
like the breakage is potentially quite large.
Thanks,
-Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTin9Cr2pyJKRKcRoVXk_SAGvDmpF5qUwfkxMkK3s>