[PATCH] pidfd: fix selftest compilation by removing linux/wait.h

[PATCH] pidfd: fix selftest compilation by removing linux/wait.h

[Christian Kellner]

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>
-- 
2.21.0

Re: [PATCH] pidfd: fix selftest compilation by removing linux/wait.h

[Christian Brauner]

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

[PATCH v2] pidfd: avoid linux/wait.h and sys/wait.h name clashes

[Christian Kellner]

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",
-- 
2.21.0

Re: [PATCH v2] pidfd: avoid linux/wait.h and sys/wait.h name clashes

[Christian Brauner]

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>