[libc] fix -Wmacro-redefined #75261
[libc] fix -Wmacro-redefined #75261
Conversation
When building with compiler-rt enabled, warnings such as the following are
observed:
llvm-project/llvm/build/projects/compiler-rt/../libc/include/llvm-libc-macros/linux/sys-stat-macros.h:46:9:
warning: 'S_IXOTH' macro redefined [-Wmacro-redefined]
#define S_IXOTH 00001
^
llvm-project/llvm/build/projects/compiler-rt/../libc/include/llvm-libc-macros/linux/fcntl-macros.h:61:9:
note: previous definition is here
#define S_IXOTH 01
^
It looks like we have these multiply defined. Deduplicate these flags; users
should expect to find them in sys/stat.h. S_FIFO was wrong anyways (should
have been S_IFIFO).
|
@llvm/pr-subscribers-libc Author: Nick Desaulniers (nickdesaulniers) ChangesWhen building with compiler-rt enabled, warnings such as the following are It looks like we have these multiply defined. Deduplicate these flags; users Full diff: https://github.com/llvm/llvm-project/pull/75261.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 cdd1cf22d7b69c..495c5ec780edb0 100644
--- a/libc/include/llvm-libc-macros/linux/fcntl-macros.h
+++ b/libc/include/llvm-libc-macros/linux/fcntl-macros.h
@@ -46,31 +46,6 @@
#define O_RDWR 00000002
#define O_WRONLY 00000001
-// File mode flags
-#define S_IRWXU 0700
-#define S_IRUSR 0400
-#define S_IWUSR 0200
-#define S_IXUSR 0100
-#define S_IRWXG 070
-#define S_IRGRP 040
-#define S_IWGRP 020
-#define S_IXGRP 010
-#define S_IRWXO 07
-#define S_IROTH 04
-#define S_IWOTH 02
-#define S_IXOTH 01
-#define S_ISUID 04000
-#define S_ISGID 02000
-
-// File type flags
-#define S_IFMT 0170000
-#define S_IFDIR 0040000
-#define S_IFCHR 0020000
-#define S_IFBLK 0060000
-#define S_IFREG 0100000
-#define S_FIFO 0010000
-#define S_IFLNK 0120000
-
// Special directory FD to indicate that the path argument to
// openat is relative to the current directory.
#define AT_FDCWD -100
diff --git a/libc/include/llvm-libc-macros/linux/sys-stat-macros.h b/libc/include/llvm-libc-macros/linux/sys-stat-macros.h
index 3be743328a26bb..7886d356f220bd 100644
--- a/libc/include/llvm-libc-macros/linux/sys-stat-macros.h
+++ b/libc/include/llvm-libc-macros/linux/sys-stat-macros.h
@@ -10,7 +10,7 @@
#define __LLVM_LIBC_MACROS_LINUX_SYS_STAT_MACROS_H
// Definitions from linux/stat.h
-#define S_IFMT 00170000
+#define S_IFMT 0170000
#define S_IFSOCK 0140000
#define S_IFLNK 0120000
#define S_IFREG 0100000
diff --git a/libc/src/__support/File/linux/file.cpp b/libc/src/__support/File/linux/file.cpp
index 2d4cea5b53c581..1b2d0e590133b6 100644
--- a/libc/src/__support/File/linux/file.cpp
+++ b/libc/src/__support/File/linux/file.cpp
@@ -18,6 +18,7 @@
#include <fcntl.h> // For mode_t and other flags to the open syscall
#include <stdio.h>
#include <sys/syscall.h> // For syscall numbers
+#include <sys/stat.h> // For S_IS*, S_IF*, and S_IR* flags.
namespace LIBC_NAMESPACE {
|
|
| @@ -10,7 +10,7 @@ | |||
| #define __LLVM_LIBC_MACROS_LINUX_SYS_STAT_MACROS_H | |||
|
|
|||
| // Definitions from linux/stat.h | |||
There was a problem hiding this comment.
kind of curious why we don't just simply
#include <linux/stat.h>
for this whole file?
There was a problem hiding this comment.
I think originally we defined all of these constants inside some spec file, so the original generated header files didn't have #include statements. Then when they were refactored into the current .h file structure, they were just ported 1-to-1. With the current include structure, we definitelt can do more than before.
There was a problem hiding this comment.
I wanted to mention this last time I came across sys/auxv.h and sys/prctl.h. So, we include linux/prctl.h to get relevant macros but defined the macros that duplicate those in linux/auxvec.h. The behavior looks unfortunately inconsistent to me. I suppose there was a transit of implementation flavor taking place so eventually I did not bring it up last time.
I am actually thinking that linux/xxxx.h is a better way if the UAPI has the header and we are sure that the behavior is stable.
Just one little problem: on linux, I guess it is possible for user to install libc-dev without linux-headers. However, I think it is fine as the kernel header package is also common enough.
|
ah, I broke presubmits: #75361 |
When building with compiler-rt enabled, warnings such as the following are
observed:
It looks like we have these multiply defined. Deduplicate these flags; users
should expect to find them in sys/stat.h. S_FIFO was wrong anyways (should
have been S_IFIFO).