-
Notifications
You must be signed in to change notification settings - Fork 167
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 C89 compliance #3787
Fix C89 compliance #3787
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -ansi
flag is not the same as being pure C89
without POSIX. POSIX functions are still available and there is no way to disable them on a standard GCC install on Linux as far as I know. That's why the _POSIX_VERSION
macro is enabled. Almost nothing in the Linux system headers changes depending on the __STRICT_ANSI__
macro being present. Some macros change from being x ? y : z
to if (x) y; else z;
, disables inlining of some math functions. But nothing really major.
How did you find this in the first place?
I found this when compiling C-sources with
|
We have to be clear regarding ISO (and ANSI) compliance. ANSI and ISO standards without a year normally refer to the latest released one (as far as I understand the previous standards are even superseded by the new ones), and as far as I understand the planned ISO standard nicknamed C23 (or C2x) is planning to add localtime_r. I don't know if it works with Thus we should call this C89 compliance, not ISO Standard compliance; and ensure that it works with C23. |
If you I guess the correct way on Linux/glibc would be to The following seems to work in both Linux and Mac (unix is not defined if STRICT_ANSI is given; could also be #if (defined(__unix__) && (!defined(__linux__) || defined(unix))) || defined(__APPLE_CC__)
#include <unistd.h>
#endif
#if !defined(_POSIX_) && defined(_POSIX_VERSION)
#define _POSIX_ 1
#endif That seems slightly better since we won't load |
Or perhaps we could look for unix and not GLIBC with STRICT_ANSI. Might capture more OSes... |
What about defining |
That's also an option and would hopefully give us the correct value for |
OK, it was easier than expected. Defining |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still does not compile with C89 because zlib requires snprintf (C99 or #define _POSIX_C_SOURCE 200112L
in gzwrite.c
and gzlib.c
)
I disabled snprintf in case of C89. No need to worry, since gzwrite and gzlib are not used at all. |
Then maybe we should remove gzlib and gzwrite sources? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I found a separate issue with C99-testing, but that deserves another PR.
Take care to not break modelica#3787 again.
Take care to not break modelica#3787 again.
Take care to not break modelica#3787 again.
Take care to not break modelica#3787 again
Take care to not break modelica#3787 again.
Take care to not break modelica#3787 again
Take care to not break #3787 again.
POSIX is not availabe if C-Sources are compiled with
-ansi
(or-std=c89
).