From: Christian Kellner christian@kellner.me
The pidfd_{open,poll}_test.c files both include `linux/wait.h` and later `sys/wait.h`. The former has `#define P_ALL 0`, but in the latter P_ALL is part of idtype_t enum, where it gets substituted due to the aforementioned define to be `0`, which then results in `typedef enum {0, ...`, which then results into a compiler error: "error: expected identifier before numeric constant". Since we need `sys/wait.h` for waitpid, drop `linux/wait.h`.
Signed-off-by: Christian Kellner christian@kellner.me --- tools/testing/selftests/pidfd/pidfd_open_test.c | 1 - tools/testing/selftests/pidfd/pidfd_poll_test.c | 1 - 2 files changed, 2 deletions(-)
diff --git a/tools/testing/selftests/pidfd/pidfd_open_test.c b/tools/testing/selftests/pidfd/pidfd_open_test.c index b9fe75fc3e51..8a59438ccc78 100644 --- a/tools/testing/selftests/pidfd/pidfd_open_test.c +++ b/tools/testing/selftests/pidfd/pidfd_open_test.c @@ -6,7 +6,6 @@ #include <inttypes.h> #include <limits.h> #include <linux/types.h> -#include <linux/wait.h> #include <sched.h> #include <signal.h> #include <stdbool.h> diff --git a/tools/testing/selftests/pidfd/pidfd_poll_test.c b/tools/testing/selftests/pidfd/pidfd_poll_test.c index 4b115444dfe9..610811275357 100644 --- a/tools/testing/selftests/pidfd/pidfd_poll_test.c +++ b/tools/testing/selftests/pidfd/pidfd_poll_test.c @@ -3,7 +3,6 @@ #define _GNU_SOURCE #include <errno.h> #include <linux/types.h> -#include <linux/wait.h> #include <poll.h> #include <signal.h> #include <stdbool.h>
On Fri, Oct 11, 2019 at 06:38:11PM +0200, Christian Kellner wrote:
From: Christian Kellner christian@kellner.me
The pidfd_{open,poll}_test.c files both include `linux/wait.h` and later `sys/wait.h`. The former has `#define P_ALL 0`, but in the latter P_ALL is part of idtype_t enum, where it gets substituted due to the aforementioned define to be `0`, which then results in `typedef enum {0, ...`, which then results into a compiler error: "error: expected identifier before numeric constant". Since we need `sys/wait.h` for waitpid, drop `linux/wait.h`.
Signed-off-by: Christian Kellner christian@kellner.me
Sorry, I missed this patch. This is problematic and your patch would only temporarily fix it. If glibc adds a P_PIDFD to the enum we'll run into the same issue. So please: - remove the linux/wait.h header (as you've already done here) - add a custom define for P_PIDFD under a different name, e.g.: #ifndef __P_PIDFD #define __P_PIDFD 3 #endif and add a comment above it explaining the reason for this mess.
Thanks and (_ugh_) Christian
From: Christian Kellner christian@kellner.me
Both linux/wait.h and sys/wait.h contain values for the first argument of 'waitid' (P_ALL, P_PID, ...). While the former uses defines the latter uses an enum. When linux/wait.h is included before sys/wait.h this will lead to an error, because P_ALL, P_PID, ... will already have been substituted to 0, 1, ... respectively and this the resulting code will be 'typedef enum {0, 1, ...'. Remove 'linux/wait.h' and rename P_PIDFD to avoid a future clash, in case P_PIDFD gets added to the idtype_t enum in sys/wait.h.
Signed-off-by: Christian Kellner christian@kellner.me --- tools/testing/selftests/pidfd/pidfd.h | 4 ++-- tools/testing/selftests/pidfd/pidfd_open_test.c | 1 - tools/testing/selftests/pidfd/pidfd_poll_test.c | 1 - tools/testing/selftests/pidfd/pidfd_wait.c | 14 +++++++------- 4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index c6bc68329f4b..9d5e76bd5466 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -16,8 +16,8 @@
#include "../kselftest.h"
-#ifndef P_PIDFD -#define P_PIDFD 3 +#ifndef __P_PIDFD +#define __P_PIDFD 3 #endif
#ifndef CLONE_PIDFD diff --git a/tools/testing/selftests/pidfd/pidfd_open_test.c b/tools/testing/selftests/pidfd/pidfd_open_test.c index b9fe75fc3e51..8a59438ccc78 100644 --- a/tools/testing/selftests/pidfd/pidfd_open_test.c +++ b/tools/testing/selftests/pidfd/pidfd_open_test.c @@ -6,7 +6,6 @@ #include <inttypes.h> #include <limits.h> #include <linux/types.h> -#include <linux/wait.h> #include <sched.h> #include <signal.h> #include <stdbool.h> diff --git a/tools/testing/selftests/pidfd/pidfd_poll_test.c b/tools/testing/selftests/pidfd/pidfd_poll_test.c index 4b115444dfe9..610811275357 100644 --- a/tools/testing/selftests/pidfd/pidfd_poll_test.c +++ b/tools/testing/selftests/pidfd/pidfd_poll_test.c @@ -3,7 +3,6 @@ #define _GNU_SOURCE #include <errno.h> #include <linux/types.h> -#include <linux/wait.h> #include <poll.h> #include <signal.h> #include <stdbool.h> diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c index 7079f8eef792..7e9170b84b81 100644 --- a/tools/testing/selftests/pidfd/pidfd_wait.c +++ b/tools/testing/selftests/pidfd/pidfd_wait.c @@ -54,7 +54,7 @@ static int test_pidfd_wait_simple(void) ksft_exit_fail_msg("%s test: failed to open /proc/self %s\n", test_name, strerror(errno));
- pid = sys_waitid(P_PIDFD, pidfd, &info, WEXITED, NULL); + pid = sys_waitid(__P_PIDFD, pidfd, &info, WEXITED, NULL); if (pid == 0) ksft_exit_fail_msg( "%s test: succeeded to wait on invalid pidfd %s\n", @@ -67,7 +67,7 @@ static int test_pidfd_wait_simple(void) ksft_exit_fail_msg("%s test: failed to open /dev/null %s\n", test_name, strerror(errno));
- pid = sys_waitid(P_PIDFD, pidfd, &info, WEXITED, NULL); + pid = sys_waitid(__P_PIDFD, pidfd, &info, WEXITED, NULL); if (pid == 0) ksft_exit_fail_msg( "%s test: succeeded to wait on invalid pidfd %s\n", @@ -83,7 +83,7 @@ static int test_pidfd_wait_simple(void) if (pid == 0) exit(EXIT_SUCCESS);
- pid = sys_waitid(P_PIDFD, pidfd, &info, WEXITED, NULL); + pid = sys_waitid(__P_PIDFD, pidfd, &info, WEXITED, NULL); if (pid < 0) ksft_exit_fail_msg( "%s test: failed to wait on process with pid %d and pidfd %d: %s\n", @@ -145,7 +145,7 @@ static int test_pidfd_wait_states(void) exit(EXIT_SUCCESS); }
- ret = sys_waitid(P_PIDFD, pidfd, &info, WSTOPPED, NULL); + ret = sys_waitid(__P_PIDFD, pidfd, &info, WSTOPPED, NULL); if (ret < 0) ksft_exit_fail_msg( "%s test: failed to wait on WSTOPPED process with pid %d and pidfd %d: %s\n", @@ -175,7 +175,7 @@ static int test_pidfd_wait_states(void) "%s test: failed to send signal to process with pid %d and pidfd %d: %s\n", test_name, parent_tid, pidfd, strerror(errno));
- ret = sys_waitid(P_PIDFD, pidfd, &info, WCONTINUED, NULL); + ret = sys_waitid(__P_PIDFD, pidfd, &info, WCONTINUED, NULL); if (ret < 0) ksft_exit_fail_msg( "%s test: failed to wait WCONTINUED on process with pid %d and pidfd %d: %s\n", @@ -199,7 +199,7 @@ static int test_pidfd_wait_states(void) test_name, info.si_pid, parent_tid, pidfd, strerror(errno));
- ret = sys_waitid(P_PIDFD, pidfd, &info, WUNTRACED, NULL); + ret = sys_waitid(__P_PIDFD, pidfd, &info, WUNTRACED, NULL); if (ret < 0) ksft_exit_fail_msg( "%s test: failed to wait on WUNTRACED process with pid %d and pidfd %d: %s\n", @@ -229,7 +229,7 @@ static int test_pidfd_wait_states(void) "%s test: failed to send SIGKILL to process with pid %d and pidfd %d: %s\n", test_name, parent_tid, pidfd, strerror(errno));
- ret = sys_waitid(P_PIDFD, pidfd, &info, WEXITED, NULL); + ret = sys_waitid(__P_PIDFD, pidfd, &info, WEXITED, NULL); if (ret < 0) ksft_exit_fail_msg( "%s test: failed to wait on WEXITED process with pid %d and pidfd %d: %s\n",
On Tue Oct 15, 2019 at 6:55 PM Christian Kellner wrote:
From: Christian Kellner christian@kellner.me
Both linux/wait.h and sys/wait.h contain values for the first argument of 'waitid' (P_ALL, P_PID, ...). While the former uses defines the latter uses an enum. When linux/wait.h is included before sys/wait.h this will lead to an error, because P_ALL, P_PID, ... will already have been substituted to 0, 1, ... respectively and this the resulting code will be 'typedef enum {0, 1, ...'. Remove 'linux/wait.h' and rename P_PIDFD to avoid a future clash, in case P_PIDFD gets added to the idtype_t enum in sys/wait.h.
Signed-off-by: Christian Kellner christian@kellner.me
Reviewed-by: Christian Brauner christian.brauner@ubuntu.com
linux-kselftest-mirror@lists.linaro.org
-
Christian Brauner -
Christian Kellner