Skip to content
This repository has been archived by the owner on Mar 9, 2019. It is now read-only.

Make _XOPEN_SOURCE definition agree with _POSIX_C_SOURCE for POSIX 2008 env #47

Closed

Conversation

kev009
Copy link
Contributor

@kev009 kev009 commented Dec 23, 2013

If we want a POSIX 2008 environment as requested by _POSIX_C_SOURCE here, _XOPEN_SOURCE should be 700 so they are in agreement.

See:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap02.html#tag_02_01_04

Fixes build on FreeBSD 10 which redefines _POSIX_C_SOURCE depending on _XOPEN_SOURCE def.

@wez
Copy link
Contributor

wez commented Dec 23, 2013

Solaris (or more specifically, Illumos, but basically the same) yields this:

In file included from /usr/include/sys/types.h:33:0,
                 from include/phenom/defs.h:55,
                 from thirdparty/tap/tap.c:28:
/opt/gcc-4.6.3/lib/gcc/i386-pc-solaris2.11/4.6.3/include-fixed/sys/feature_tests.h:360:2: error: #error "Compiler or options invalid for pre-UNIX 03 X/Open applications        and pre-2001 POSIX applications"

This environment only goes as far as _XOPEN_VERSION=600; if the value is higher, it doesn't consider that XPG6 is supported and raises that error.

Looks like Solaris only goes as far as _POSIX_C_SOURCE=200112L

@wez
Copy link
Contributor

wez commented Dec 23, 2013

This patch on top of this pull request makes things happy on my Illumos VM, and builds and runs on Darwin and Linux; can you verify on FreeBSD and then amend your pull request? Thanks!

diff --git a/include/phenom/defs.h b/include/phenom/defs.h
index f0bddb3..006f215 100644
--- a/include/phenom/defs.h
+++ b/include/phenom/defs.h
@@ -33,9 +33,14 @@
 # define _REENTRANT
 #endif
 #define __EXTENSIONS__ 1
-#define _XOPEN_SOURCE 700
 #define _BSD_SOURCE
+#ifdef __sun__
+#define _XOPEN_SOURCE 600
+#define _POSIX_C_SOURCE 200112L
+#else
+#define _XOPEN_SOURCE 700
 #define _POSIX_C_SOURCE 200809
+#endif
 #define _GNU_SOURCE
 #define _DARWIN_C_SOURCE

…08 env

Treat Solaris as POSIX 2001 as their check is not additive
@kev009
Copy link
Contributor Author

kev009 commented Dec 23, 2013

LGTM

@kev009
Copy link
Contributor Author

kev009 commented Dec 23, 2013

We could push some of the other ifdefs at the top of this file out to autoconf with AC_USE_SYSTEM_EXTENSIONS. Probably do it as defs.h as an AC_CONFIG_HEADER if we want to install public env requirements. What do you think?

wez pushed a commit that referenced this pull request Dec 23, 2013
…08 env

Treat Solaris as POSIX 2001 as their check is not additive

Addresses #47
@wez
Copy link
Contributor

wez commented Dec 23, 2013

I haven't run into AC_USE_SYSTEM_EXTENSIONS before. I think integrating it could be slightly tricky because the defines need to be set before making any other includes. We have include/phenom/feature_test.h for some features we detect at configure time; if we can persuade the relevant extensions to show up there (we defer all the others to autoheader and I'm not sure how/if we can put them in one and not the other), we could then include that at the top of include/phenom/defs.h unconditionally.

I think we roll that into a separate pull request as a nice to have, or maybe just leave things be, as what we have now that we've applied your patch, works on the major platforms that most people care about.

@kev009
Copy link
Contributor Author

kev009 commented Dec 23, 2013

Agreed. Closing, I will investigate the other change with low priority.

@kev009 kev009 closed this Dec 23, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants