Skip to content

[libc] fix -Wmacro-redefined #75261

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

Merged
merged 2 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions libc/include/llvm-libc-macros/linux/fcntl-macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libc/include/llvm-libc-macros/linux/sys-stat-macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#define __LLVM_LIBC_MACROS_LINUX_SYS_STAT_MACROS_H

// Definitions from linux/stat.h
Copy link
Member Author

Choose a reason for hiding this comment

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

kind of curious why we don't just simply

#include <linux/stat.h>

for this whole file?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@SchrodingerZhu SchrodingerZhu Dec 13, 2023

Choose a reason for hiding this comment

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

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.

#define S_IFMT 00170000
#define S_IFMT 0170000
#define S_IFSOCK 0140000
#define S_IFLNK 0120000
#define S_IFREG 0100000
Expand Down
1 change: 1 addition & 0 deletions libc/src/__support/File/linux/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <fcntl.h> // For mode_t and other flags to the open syscall
#include <stdio.h>
#include <sys/stat.h> // For S_IS*, S_IF*, and S_IR* flags.
#include <sys/syscall.h> // For syscall numbers

namespace LIBC_NAMESPACE {
Expand Down