standards/188173: O_NOFOLLOW visibility not POSIX 2008 conforming

Sun Apr 13 22:10:03 UTC 2014

The following reply was made to PR standards/188173; it has been noted by GNATS.

From: Jilles Tjoelker <jilles at stack.nl>
To: Bruce Evans <brde at optusnet.com.au>
Cc: standards at freebsd.org, freebsd-gnats-submit at freebsd.org,
	Christian Neukirchen <chneukirchen at gmail.com>
Subject: Re: standards/188173: O_NOFOLLOW visibility not POSIX 2008 conforming
Date: Mon, 14 Apr 2014 00:07:30 +0200

 On Sun, Apr 06, 2014 at 05:30:11PM +1000, Bruce Evans wrote:
 > On Sat, 5 Apr 2014, Jilles Tjoelker wrote:

 > > On Sat, Apr 05, 2014 at 11:16:07PM +0300, Konstantin Belousov wrote:

 > >> diff --git a/sys/sys/fcntl.h b/sys/sys/fcntl.h
 > >> index 3461f8b..2691449 100644
 > >> --- a/sys/sys/fcntl.h
 > >> +++ b/sys/sys/fcntl.h
 > >> @@ -96,7 +96,7 @@ typedef	__pid_t		pid_t;
 > >>  #define	O_FSYNC		0x0080		/* synchronous writes */
 > >>  #endif
 > >>  #define	O_SYNC		0x0080		/* POSIX synonym for O_FSYNC */
 > >> -#if __BSD_VISIBLE
 > >> +#if __POSIX_VISIBLE >= 200809
 > >>  #define	O_NOFOLLOW	0x0100		/* don't follow symlinks */
 > >>  #endif
 > >>  #define	O_CREAT		0x0200		/* create if nonexistent */

 > > This looks good, but I went ahead and made the other new POSIX.1-2008
 > > things visible as well and removed redundant __BSD_VISIBLE condition
 > > parts:

 > That __BSD_VISIBLE is redundant is a bit confusing.  Perhaps add or
 > expand a comment about this.

 The comment about that is in <sys/cdefs.h>:

 ] #else                           /* Default environment: show everything.
 ] */
 ] #define __POSIX_VISIBLE         200809
 ] #define __XSI_VISIBLE           700
 ] #define __BSD_VISIBLE           1
 ] #define __ISO_C_VISIBLE         2011
 ] #endif

 By doing it this way, most #ifs are simplified.

 > > Index: sys/sys/fcntl.h
 > > ===================================================================
 > > --- sys/sys/fcntl.h	(revision 263842)
 > > +++ sys/sys/fcntl.h	(working copy)
 > > ...
 > > @@ -211,7 +211,7 @@ typedef	__pid_t		pid_t;
 > > #define	F_SETFD		2		/* set file descriptor flags */
 > > #define	F_GETFL		3		/* get file status flags */
 > > #define	F_SETFL		4		/* set file status flags */
 > > -#if __BSD_VISIBLE || __XSI_VISIBLE || __POSIX_VISIBLE >= 200112
 > > +#if __XSI_VISIBLE || __POSIX_VISIBLE >= 200112
 > > #define	F_GETOWN	5		/* get SIGIO/SIGURG proc/pgrp */
 > > #define	F_SETOWN	6		/* set SIGIO/SIGURG proc/pgrp */
 > > #endif

 > __XSI_VISIBLE seems to be the only condition that is sometimes needed in
 > ifdefs together with __POSIX_VISIBLE.

 Yes.

 > Many or most of the XSI ifdefs are out of date, with lots of XSI stuff
 > having been moved into POSIX but the ifdefs not being updated.  Fixing
 > this would probably give many more relatively complicated ifdefs like
 > the above.

 > The BSD vs POSIX redundancy is also in:

 > % capability.h:#if __BSD_VISIBLE || __XSI_VISIBLE || __POSIX_VISIBLE >= 200112
 > % capability.h:#if __BSD_VISIBLE || __XSI_VISIBLE || __POSIX_VISIBLE >= 200112

 > Old XSI ifdefs in new FreeBSD code seem to be nonsense.

 It does not make sense to use visibility macros if the application has
 to #include something not defined by that standard anyway.

 > % signal.h:#if __BSD_VISIBLE || __POSIX_VISIBLE > 0 && __POSIX_VISIBLE <= 200112

 > Perhaps complicated enough to be correct.

 Yes.

 > % stat.h:#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200809

 > The above is from grepping <sys> for VISIBLE and selecting lines with || and
 > removing lines without BSD_VISIBLE.  Many more lines have XSI || POSIX, but
 > not in enough files to give much chance that these are complete.

 > Sampling of errors and complications outside of <sys>:

 > ./dirent.h:#if __BSD_VISIBLE || __XSI_VISIBLE

 > Seems to be redundant.  I think BSD implies the latest version of XSI.
 > So this should use just XSI.

 Yes.

 > ./langinfo.h:#if __BSD_VISIBLE || __XSI_VISIBLE <= 500

 > Need both here since 500 is not the latest XSI.  Assuming 500 is correct.

 Hmm, that also includes __XSI_VISIBLE == 0, which is incorrect. It
 should probably be:
 #if __BSD_VISIBLE || (__XSI_VISIBLE && __XSI_VISIBLE <= 500)

 > ./netdb.h:#if __BSD_VISIBLE || (__POSIX_VISIBLE && __POSIX_VISIBLE <= 200112)

 > BSD together with POSIX is necessary when the POSIX test is reversed.

 Yes.

 > ./setjmp.h:#if __BSD_VISIBLE || __XSI_VISIBLE >= 600

 > Another complicated test.  Symbols are rarely removed, so such tests are rare.

 The __BSD_VISIBLE || part can go away here.

 > % ./stdio.h:#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200809
 > % ./stdio.h:#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200112 || __XSI_VISIBLE

 These could be simplified.

 > % ./stdio.h:#if __BSD_VISIBLE || __POSIX_VISIBLE <= 199506

 > Note: reversed test.

 This should be:
 #if __BSD_VISIBLE || (__POSIX_VISIBLE && __POSIX_VISIBLE <= 199506)
 since C11 (for example) does not define L_cuserid and friends.

 > % ./stdio.h:#if __BSD_VISIBLE || __XSI_VISIBLE > 0 && __XSI_VISIBLE < 600

 > Note: reversed test.  The previous reversed test may be broken, since
 > it is missing a check for '> 0'.  Elsewhere, the test for '> 0' is
 > obfuscated by writing it as a boolean check for != 0 with implicit 0.

 > % ./stdio.h:#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200809
 > % ./stdio.h:#endif /* __BSD_VISIBLE || __POSIX_VISIBLE >= 200809 */
 > % ./string.h:#if __POSIX_VISIBLE >= 200809 || __BSD_VISIBLE
 > % ./string.h:#if __POSIX_VISIBLE >= 200112 || __XSI_VISIBLE
 > % ./string.h:#if __POSIX_VISIBLE >= 200809 || __BSD_VISIBLE
 > % ./string.h:#if __POSIX_VISIBLE >= 200809 || __BSD_VISIBLE
 > % ./string.h:#if __POSIX_VISIBLE >= 199506 || __XSI_VISIBLE >= 500
 > % ./string.h:#if __POSIX_VISIBLE >= 200809 || defined(_XLOCALE_H_)

 > Redundancies are most dense in these 2 popular headers.  The tests
 > (mainly the redundant parts) are also obfuscated by writing them in
 > random orders (mostly BSD first in stdio.h and BSD last in string.h).
 > All of the randomly ordered redundancies in stdio.h and string.h are
 > new with the 2008 or 2012 versions of POSIX.  One in fcntl.h was old
 > with the 2001 version of POSIX.

 Yes, messy... Some stuff can be simplified here.

 -- 
 Jilles Tjoelker