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

Add -Wundef to gcc's --strict-warnings options #2709

Conversation

bernd-edlinger
Copy link
Member

This PR adds -Wundef to --strict-warnings for gcc.
This warns on #if with undefined macros.

For 1.1.0-stable only.
I will do another PR for master once #2705 is merged.

Copy link
Contributor

@richsalz richsalz left a comment

Choose a reason for hiding this comment

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

Because I'm lazy, what was undef'd?

@bernd-edlinger
Copy link
Member Author

bernd-edlinger commented Feb 22, 2017

OPENSSL_API_COMPAT and __STDC_VERSION__ in pedantic mode
there was a discussion on the gcc mailing list, that
-Wundef should be silent on predefined macros,
but I don't know if anybody cared to implement that.

@richsalz
Copy link
Contributor

Please add this to clang strict warnings too.

@bernd-edlinger
Copy link
Member Author

Ok, does clang have -Wundef? And what is clang by the way :-)

@richsalz
Copy link
Contributor

#2705 is merged, 7c6335a. You're kidding about clang, I assume.

@dot-asm dot-asm added the approval: done This pull request has the required number of approvals label Feb 23, 2017
@dot-asm
Copy link
Contributor

dot-asm commented Feb 23, 2017

Side note. If #2712 was broken to two commits, then it would be possible to cherry-pick one to 1.1.0 so that one wouldn't have to have separate request. Cherry-picks should be preferred way as it gives same modification same "tracking number"...

@bernd-edlinger
Copy link
Member Author

@dot-asm I will keep that in mind; when I started I had the impression that 1.1.0 and master
are too different for a single commit. I will try to do smaller commits in the future.

Now I see two issues in the Travis CI build:
the gcc build fails here:

apps/rehash.c: In function 'ends_with_dirsep':
apps/rehash.c:277:8: error: "_WIN32" is not defined [-Werror=undef]
 # elif _WIN32

that elif _WIN32 was not there in my tree, but CI did a rebase apparently,
so I will do that as well.

I need to change that to elif defined(_WIN32)

In one clang build a different error happens, here:

crypto/o_str.c:230:8: error: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef]
#elif (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)
crypto/o_str.c:230:38: error: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef]
#elif (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)

I will add two single commits for each.

@levitte
Copy link
Member

levitte commented Feb 23, 2017

Now I see two issues in the Travis CI build:
the gcc build fails here:

apps/rehash.c: In function 'ends_with_dirsep':
apps/rehash.c:277:8: error: "_WIN32" is not defined [-Werror=undef]
 # elif _WIN32

Good catch. Fix in #2727

@levitte
Copy link
Member

levitte commented Feb 23, 2017

In one clang build a different error happens, here:

crypto/o_str.c:230:8: error: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef]
#elif (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)
crypto/o_str.c:230:38: error: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef]
#elif (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600)

I'm curious, on what configuration was that? grep perlarg configdata.pm will show what Configure was hit with.

@bernd-edlinger
Copy link
Member Author

I'm curious, on what configuration was that? grep perlarg configdata.pm will show what Configure was hit with.

I think it was with "no-stdio --strict-warnings" BUILDONLY="yes"

levitte pushed a commit that referenced this pull request Feb 23, 2017
Avoid a -Wundef warning in o_str.c
Avoid a -Wundef warning in testutil.h
Include internal/cryptlib.h before openssl/stack.h
to avoid use of undefined symbol OPENSSL_API_COMPAT.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #2709)
@richsalz
Copy link
Contributor

4c3376e in 1.1.0 thanks!

@richsalz richsalz closed this Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants