-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libc] Fix macro definition hermeticity #114467
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
Conversation
Previously shm_test was including <asm-generic/fcntl.h> for the macro FD_CLOEXEC. This patch adds that macro to the list we have defined, and redirects the test to use the correct proxy header.
@llvm/pr-subscribers-libc Author: Michael Jones (michaelrj-google) ChangesPreviously shm_test was including <asm-generic/fcntl.h> for the macro Full diff: https://github.com/llvm/llvm-project/pull/114467.diff 3 Files Affected:
diff --git a/libc/include/llvm-libc-macros/linux/fcntl-macros.h b/libc/include/llvm-libc-macros/linux/fcntl-macros.h
index 8ee95863728e15..aec8a0d2da0b52 100644
--- a/libc/include/llvm-libc-macros/linux/fcntl-macros.h
+++ b/libc/include/llvm-libc-macros/linux/fcntl-macros.h
@@ -88,6 +88,9 @@
// Close on succesful
#define F_CLOEXEC 1
+// Close on execute for fcntl.
+#define FD_CLOEXEC 1
+
#define F_RDLCK 0
#define F_WRLCK 1
#define F_UNLCK 2
diff --git a/libc/test/src/sys/mman/linux/CMakeLists.txt b/libc/test/src/sys/mman/linux/CMakeLists.txt
index a432d88ffb90c9..69263986cc574c 100644
--- a/libc/test/src/sys/mman/linux/CMakeLists.txt
+++ b/libc/test/src/sys/mman/linux/CMakeLists.txt
@@ -163,5 +163,6 @@ add_libc_unittest(
libc.src.unistd.ftruncate
libc.src.unistd.close
libc.src.__support.OSUtil.osutil
+ libc.hdr.fcntl_macros
libc.test.UnitTest.ErrnoSetterMatcher
)
diff --git a/libc/test/src/sys/mman/linux/shm_test.cpp b/libc/test/src/sys/mman/linux/shm_test.cpp
index 4b8971f670581c..97de705c4c2b06 100644
--- a/libc/test/src/sys/mman/linux/shm_test.cpp
+++ b/libc/test/src/sys/mman/linux/shm_test.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/fcntl_macros.h"
#include "src/__support/OSUtil/syscall.h"
#include "src/fcntl/fcntl.h"
#include "src/sys/mman/mmap.h"
@@ -16,7 +17,6 @@
#include "src/unistd/ftruncate.h"
#include "test/UnitTest/ErrnoSetterMatcher.h"
#include "test/UnitTest/LibcTest.h"
-#include <asm-generic/fcntl.h>
#include <sys/syscall.h>
using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
|
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 is fine to unbreak the build for now.
Longer term, I think the linux specific libc/include/llvm-libc-macros/linux/fcntl-macros.h should be replaced with #include <asm-generic/fcntl.h>
. We should not be redefining these ourselves IMO.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/7857 Here is the relevant piece of the build log for the reference
|
Previously shm_test was including <asm-generic/fcntl.h> for the macro FD_CLOEXEC. This patch adds that macro to the list we have defined, and redirects the test to use the correct proxy header.
Previously shm_test was including <asm-generic/fcntl.h> for the macro FD_CLOEXEC. This patch adds that macro to the list we have defined, and redirects the test to use the correct proxy header.
Previously shm_test was including <asm-generic/fcntl.h> for the macro
FD_CLOEXEC. This patch adds that macro to the list we have defined, and
redirects the test to use the correct proxy header.