Question about non-__BSD_VISIBLE guarded CLOCK_* constants

[Garrett Cooper]

On Mon, Nov 8, 2010 at 7:59 AM, Bruce Evans <brde at optusnet.com.au> wrote:
> On Sun, 7 Nov 2010, Garrett Cooper wrote:
>
>>   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    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. */
>
> 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,
>  except 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.  It is missing the comment, and seems to be
>  undocumented.
>
>>   Also, they're blocked off by #if !defined(CLOCK_REALTIME) &&
>> __POSIX_VISIBLE >= 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.  It defines all of
> these macros unconditionally, and also includes <time.h> in the !_KERNEL
> case.  Thus including the POSIX header <sys/time.h> gives massive namespace
> pollution.
>
> % #if !defined(CLOCK_REALTIME) && __POSIX_VISIBLE >= 200112
> % #define CLOCK_REALTIME        0
> % #ifdef __BSD_VISIBLE
> % #define CLOCK_VIRTUAL 1
> % #define CLOCK_PROF    2
> % #endif
> % #define CLOCK_MONOTONIC       4
> /* These macros are also in sys/time.h. */
> #define CLOCK_MONOTONIC 4
> #define CLOCK_UPTIME    5               /* FreeBSD-specific. */
> #define ...
> #endif /* !defined(CLOCK_REALTIME) && __POSIX_VISIBLE >= 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 shouldn't
>  be defined yet, so it has no effect in this ifdef
> - if <sys/time.h> is     included before here, then CLOCK_REALTIME
>  is defined now, so its effect in this ifdef is to prevent repeating ifdefs
>  that should be the same.  Repeated ifdefs are a very small part of header
>  bloat, so it is hardly worth avoiding them, and not avoiding them gives the
>  feature of checking that they really are repeated.  I think the repetitions
>  are all indentical, including their style bugs and whitespace that would
>  not be checked.
> - if <sys/time.h> is not included before here, but is included later, then
>  CLOCK_REALTIME etc. gets declared here, and since <sys/time.h> has no
>  similar ifdefs, it gets defined again later.  Thus the automatic checking
>  is only partially broken.
>
> As you noticed, the __POSIX_VISIBLE part of this has the wrong version
> number
> at best.  CLOCK_REALTIME dates from POSIX.4 in ~1994 (it is in my POSIX.4
> book published in 1995, and in the 1996 POSIX standard).  FreeBSD 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 visible
> by default by putting them under __BSD_VISIBLE.
>
> So the correct ifdefs here aresomething like __BSD_VISIBLE ||
> POSIX_VISIBLE >= 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 and
> numericl order), there are actually several sub-blocks:
> - CLOCK_MONOTONIC seems to be new in POSIX.1-2001.  Thus the current ifdef
>  is partially correct for it.  The rest of the file has careful ifdefs for
>  1993 POSIX, so perhaps it is worth being equally careful for
> CLOCK_REALTIME.
> - the BSD extensions CLOCK_VIRTUAL and CLOCK_PROF are already in a
>  __BSD_VISIBLE block.  Then there seems to be space for expansion (a
>  whole 1 id = 3).  Then there is the whole 1 new id = 4 for 2001 POSIX.
>  Then there is a FreeBSD-specific block (ids 5-14).  Too much bloat for me.

    Looks like I got the isolation working properly:

# gcc -D_BSD_SOURCE -o ~gcooper/test_clock_uptime ~gcooper/test_clock_uptime.c
# ~gcooper/test_clock_uptime
100031 661918304
# gcc -D_POSIX_C_SOURCE=200809 -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 in.)

    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