Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix glibc specific conditional for Mac OS/X #5269

Closed
wants to merge 1 commit into from

Conversation

paulidale
Copy link
Contributor

MacOS seems to define GLIBC but not __GLIBC_PREREQ.

The issue was reported against this commit. It follows on from the changes in #5231

MacOS seems to define __GLIBC__ but not __GLIBC_PREREQ.
@paulidale
Copy link
Contributor Author

An alternative solution would be to conditionally define __GLIBC_PREREQ:

#ifndef __GLIBC_PREREQ
#define __GLIBC_PREREQ(a, b) (0)
#endif

somewhere.

@paulidale
Copy link
Contributor Author

@lodenos this might help.

@paulidale
Copy link
Contributor Author

Thanks @richsalz
Merged

levitte pushed a commit that referenced this pull request Feb 7, 2018
MacOS seems to define __GLIBC__ but not __GLIBC_PREREQ.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #5269)
@@ -231,7 +231,8 @@ static uint64_t get_timer_bits(void)
# if defined(_POSIX_C_SOURCE) \
&& defined(_POSIX_TIMERS) \
&& _POSIX_C_SOURCE >= 199309L \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's like producing least readable code was the purpose. I mean _POSIX_C_SOURCE is about same thing, so why is there another thing in between? Basically both _POSIX_C_SOURCE should be in parentheses to denote it's about same thing. As for _POSIX_TIMERS and MacOS X context. It's defined as -1, while Linux(*) man page says it's positive.

(*) It's not clear if we can hold Linux man pages as official standard, one should check POSIX specs.

@dot-asm
Copy link
Contributor

dot-asm commented Feb 7, 2018

BTW, referred comment asks "work?", this conversation says "might help". You know what? It doesn't work. In fact it doesn't work on any system [that attempts to compile this] but Linux.

@paulidale
Copy link
Contributor Author

Which systems doesn't this work for?
Our CI builds for Windows & Solaris built okay. AIXand HP/UX didn't for a different reason.

That conditional is getting complex. Grouping the _POSIX_C_SOURCE checks makes sense and was missed on the previous commit. Would the alternative of defining __GLIBC_PREREQ if it isn't already defined help? I could also add a comment about what it is trying to achieve.

@dot-asm
Copy link
Contributor

dot-asm commented Feb 7, 2018

Which systems doesn't this work for?

Once again, any that tries to compile it and is not Linux, such as MacOS X, *BSD, OSF, IRIX... VMS ought to fail too...

@paulidale
Copy link
Contributor Author

In what way isn't this conditional working? I'm confused.

@dot-asm
Copy link
Contributor

dot-asm commented Feb 7, 2018

Just remove __sun condition and see for yourself on Solaris.

@paulidale
Copy link
Contributor Author

Thanks, got it now. PR coming.

@mouse07410
Copy link
Contributor

Which systems doesn't this work for?

it does not work for MacOS El Capitan, Sierra, and High Sierra. See #5290.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants