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

ior does not build without CFLAGS=-std=gnu99 #93

Closed
glennklockwood opened this issue Oct 2, 2018 · 8 comments · Fixed by #101
Closed

ior does not build without CFLAGS=-std=gnu99 #93

glennklockwood opened this issue Oct 2, 2018 · 8 comments · Fixed by #101
Assignees
Labels

Comments

@glennklockwood
Copy link
Contributor

A number of recent changes have essentially required that

  1. the code be compiled with C99 support, as there are now pieces that violate c89 (e.g., for (int i = 0; ...)
  2. there appear to be problems with certain gnuisms at link time (not sure if this is a #ifdef _GNU_SOURCE type of issue or what)

In particular, I am having lots of problems compiling ior's master using the Cray MPI environment (CLE6) and the Intel compiler, but I don't have the time to debug right now. In the long term, we should decide if we are OK with requiring C99 (and if so, add that to autoconf).

My personal preference is to use strict c89 and add that to the regression tests' compiler flags so we catch non-compliant C.

@glennklockwood glennklockwood added this to the 3.2.0 milestone Oct 2, 2018
@glennklockwood glennklockwood self-assigned this Oct 2, 2018
@hjelmn
Copy link
Collaborator

hjelmn commented Oct 2, 2018

AC_PROG_CC_C99 in the configure will do it. I think it is safe for IOR to require C99 or even just jump to C11. FWIW, Open MPI requires C99 now and will require C11 in the future.

@JulianKunkel
Copy link
Collaborator

JulianKunkel commented Oct 2, 2018 via email

@hjelmn
Copy link
Collaborator

hjelmn commented Oct 2, 2018

Indeed. Even Microsoft supports C99 now for the most part. They were the lone holdout for a long time. As of 2018 icc, gcc, pgcc, xlc, and sun cc all support all of C99 and most/all of C11.

@glennklockwood
Copy link
Contributor Author

It turns out we already require C99 so the first half of this issue is not actually an issue. However the fact remains that -std=c99 reveals a bunch of not-strictly-compliant behavior such as

  CC       libaiori_a-ior.o
../../src/ior.c(182): warning #266: function "strdup" declared implicitly
          p->api = strdup(default_aiori);
                   ^

This is an issue of what magic switches autoconf should be looking for and shouldn't be hard to fix.

More critically though, the following error is fatal:

../../src/parse_options.c(543): error: identifier "optarg" is undefined
            initialTestParams.memoryPerNode = NodeMemoryStringToBytes(optarg);
                                                                      ^

compilation aborted for ../../src/parse_options.c (code 2)

I'll work on getting this all compiling using standard C.

@JulianKunkel
Copy link
Collaborator

JulianKunkel commented Oct 2, 2018 via email

@glennklockwood
Copy link
Contributor Author

glennklockwood commented Oct 4, 2018

Making IOR actually comply with POSIX C now will require code changes that I don't want to hammer out before the 3.2 release, so this will have to wait until after 3.2.

In particular, we use usleep(3) which was removed from POSIX.1-2008 in favor of nanosleep(3), and there is some nuance to putenv/setenv that I would like to clear up as time permits.

@glennklockwood glennklockwood removed this from the 3.2.0 milestone Oct 4, 2018
@JulianKunkel
Copy link
Collaborator

I made the changes from usleep to nanosleep at least.

@glennklockwood
Copy link
Contributor Author

Thanks! I will try to get the rest of the patches tidied up and make a PR soon.

@glennklockwood glennklockwood reopened this Oct 8, 2018
glennklockwood added a commit that referenced this issue Dec 4, 2018
Moved _XOPEN_SOURCE to config.h; resolves #93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants