Enable _GNU_SOURCE for Android #850
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When targeting the Android NDK _GNU_SOURCE is not enabled by default:
```
/*
* With bionic, you always get all C and POSIX API.
*
* If you want BSD and/or GNU extensions, _BSD_SOURCE and/or _GNU_SOURCE are
* expected to be defined by callers before *any* standard header file is
* included.
*
* In our header files we test against __USE_BSD and __USE_GNU.
*/
#if defined(_GNU_SOURCE)
# define __USE_BSD 1
# define __USE_GNU 1
#endif
```
Because of this `pipe2` is not available:
```
#if defined(__USE_GNU)
int pipe2(int __fds[2], int __flags) __INTRODUCED_IN(9);
#endif
```
The function used to check if it does exist:
```
CHECK_FUNCTION_EXISTS_EX(pipe2 EVENT__HAVE_PIPE2)
```
Just check that the _linking_ succeeds, which it does, it's just not
visible in the import, leading to a warning (or error):
```
evutil.c:2637:6: error: implicit declaration of function 'pipe2' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
if (pipe2(fd, O_NONBLOCK|O_CLOEXEC) == 0)
^
```
When targeting the NDK it should be safe to always opt into this. Clang
would pass the right flag for us automatically _if_ the source was C++
instead of C.
|
I don't think the red CI is related to this change, but let me know if it is! |
keith
added a commit
to keith/envoy
that referenced
this pull request
Jul 8, 2019
The Android NDK doesn't assume `_GNU_SOURCE`, clang only assumes it for C++ builds, not C builds. libevent uses `pipe2` which in the NDK is only exposed in the header when these variables are set. In bazel 0.27.0 these implicit use became an error. More details: - envoyproxy/envoy-mobile#116 - libevent/libevent#850 Signed-off-by: Keith Smiley <[email protected]>
|
Interesting, did you look into autotools build, IOW |
|
I'm not super familiar with autotools, but it appears to since event-config.h ends up having: |
lizan
pushed a commit
to envoyproxy/envoy
that referenced
this pull request
Jul 9, 2019
The Android NDK doesn't assume `_GNU_SOURCE`, clang only assumes it for C++ builds, not C builds. libevent uses `pipe2` which in the NDK is only exposed in the header when these variables are set. In bazel 0.27.0 these implicit use became an error. More details: - envoyproxy/envoy-mobile#116 - libevent/libevent#850 Signed-off-by: Keith Smiley <[email protected]>
|
Applied, thanks! |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When targeting the Android NDK _GNU_SOURCE is not enabled by default:
Because of this
pipe2is not available:The function used to check if it does exist:
Just check that the linking succeeds, which it does, it's just not
visible in the import, leading to a warning (or error):
When targeting the NDK it should be safe to always opt into this. Clang
would pass the right flag for us automatically if the source was C++
instead of C.