Date: Mon, 14 Apr 2014 00:07:30 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: Bruce Evans <brde@optusnet.com.au> Cc: standards@freebsd.org, freebsd-gnats-submit@freebsd.org, Christian Neukirchen <chneukirchen@gmail.com> Subject: Re: standards/188173: O_NOFOLLOW visibility not POSIX 2008 conforming Message-ID: <20140413220730.GA61434@stack.nl> In-Reply-To: <20140406165123.J4942@besplex.bde.org> References: <201404011531.s31FVVNR008903@cgiserv.freebsd.org> <20140405201607.GL21331@kib.kiev.ua> <20140405204748.GA20798@stack.nl> <20140406165123.J4942@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140413220730.GA61434>