[PATCH 1/2] selftests: fix header dependency for pid_namespace selftests

[PATCH 1/2] selftests: fix header dependency for pid_namespace selftests

[Axel Rasmussen]

The way the test target was defined before, when building with clang we
get a command line like this:

clang -Wall -Werror -g -I../../../../usr/include/ \
	regression_enomem.c ../pidfd/pidfd.h  -o regression_enomem

This yields an error, because clang thinks we want to produce both a *.o
file, as well as a precompiled header:

clang: error: cannot specify -o when generating multiple output files

gcc, for whatever reason, doesn't exhibit the same behavior which I
suspect is why the problem wasn't noticed before.

This can be fixed simply by using the LOCAL_HDRS infrastructure the
selftests lib.mk provides. It does the right think and marks the target
as depending on the header (so if the header changes, we rebuild), but
it filters the header out of the compiler command line, so we don't get
the error described above.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 tools/testing/selftests/pid_namespace/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/pid_namespace/Makefile b/tools/testing/selftests/pid_namespace/Makefile
index dcaefa224ca0..edafaca1aeb3 100644
--- a/tools/testing/selftests/pid_namespace/Makefile
+++ b/tools/testing/selftests/pid_namespace/Makefile
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 CFLAGS += -g -I../../../../usr/include/
 
-TEST_GEN_PROGS := regression_enomem
+TEST_GEN_PROGS = regression_enomem
 
-include ../lib.mk
+LOCAL_HDRS += $(selfdir)/pidfd/pidfd.h
 
-$(OUTPUT)/regression_enomem: regression_enomem.c ../pidfd/pidfd.h
+include ../lib.mk
-- 
2.35.1.1021.g381101b075-goog

[PATCH 2/2] selftests: fix an unused variable warning in pidfd selftest

[Axel Rasmussen]

I fixed a few warnings like this in commit e2aa5e650b07
("selftests: fixup build warnings in pidfd / clone3 tests"), but I
missed this one by mistake. Since this variable is unused, remove it.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 tools/testing/selftests/pidfd/pidfd_wait.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c
index 17999e082aa7..070c1c876df1 100644
--- a/tools/testing/selftests/pidfd/pidfd_wait.c
+++ b/tools/testing/selftests/pidfd/pidfd_wait.c
@@ -95,7 +95,6 @@ TEST(wait_states)
 		.flags = CLONE_PIDFD | CLONE_PARENT_SETTID,
 		.exit_signal = SIGCHLD,
 	};
-	int ret;
 	pid_t pid;
 	siginfo_t info = {
 		.si_signo = 0,
-- 
2.35.1.1021.g381101b075-goog

Re: [PATCH 1/2] selftests: fix header dependency for pid_namespace selftests

[Christian Brauner]

On Thu, Mar 24, 2022 at 03:39:28PM -0700, Axel Rasmussen wrote:
> The way the test target was defined before, when building with clang we
> get a command line like this:
> 
> clang -Wall -Werror -g -I../../../../usr/include/ \
> 	regression_enomem.c ../pidfd/pidfd.h  -o regression_enomem
> 
> This yields an error, because clang thinks we want to produce both a *.o
> file, as well as a precompiled header:
> 
> clang: error: cannot specify -o when generating multiple output files
> 
> gcc, for whatever reason, doesn't exhibit the same behavior which I
> suspect is why the problem wasn't noticed before.
> 
> This can be fixed simply by using the LOCAL_HDRS infrastructure the
> selftests lib.mk provides. It does the right think and marks the target
> as depending on the header (so if the header changes, we rebuild), but
> it filters the header out of the compiler command line, so we don't get
> the error described above.
> 
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> ---

Looks good,
Reviewed-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH 2/2] selftests: fix an unused variable warning in pidfd selftest

[Christian Brauner]

On Thu, Mar 24, 2022 at 03:39:29PM -0700, Axel Rasmussen wrote:
> I fixed a few warnings like this in commit e2aa5e650b07
> ("selftests: fixup build warnings in pidfd / clone3 tests"), but I
> missed this one by mistake. Since this variable is unused, remove it.
> 
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> ---

Looks good,
Reviewed-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH 1/2] selftests: fix header dependency for pid_namespace selftests

[Shuah Khan]

On 3/24/22 4:39 PM, Axel Rasmussen wrote:
> The way the test target was defined before, when building with clang we
> get a command line like this:
> 
> clang -Wall -Werror -g -I../../../../usr/include/ \
> 	regression_enomem.c ../pidfd/pidfd.h  -o regression_enomem
> 
> This yields an error, because clang thinks we want to produce both a *.o
> file, as well as a precompiled header:
> 
> clang: error: cannot specify -o when generating multiple output files
> 
> gcc, for whatever reason, doesn't exhibit the same behavior which I
> suspect is why the problem wasn't noticed before.
> 

Thank you fixing this.

> This can be fixed simply by using the LOCAL_HDRS infrastructure the
> selftests lib.mk provides. It does the right think and marks the target
> as depending on the header (so if the header changes, we rebuild), but
> it filters the header out of the compiler command line, so we don't get
> the error described above.
> 
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> ---
>   tools/testing/selftests/pid_namespace/Makefile | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/pid_namespace/Makefile b/tools/testing/selftests/pid_namespace/Makefile
> index dcaefa224ca0..edafaca1aeb3 100644
> --- a/tools/testing/selftests/pid_namespace/Makefile
> +++ b/tools/testing/selftests/pid_namespace/Makefile
> @@ -1,8 +1,8 @@
>   # SPDX-License-Identifier: GPL-2.0
>   CFLAGS += -g -I../../../../usr/include/
>   
> -TEST_GEN_PROGS := regression_enomem
> +TEST_GEN_PROGS = regression_enomem
>   
> -include ../lib.mk
> +LOCAL_HDRS += $(selfdir)/pidfd/pidfd.h
>   
> -$(OUTPUT)/regression_enomem: regression_enomem.c ../pidfd/pidfd.h
> +include ../lib.mk
> 

Will apply this for Linux 5.18-rc2

thanks,
-- Shuah

Re: [PATCH 2/2] selftests: fix an unused variable warning in pidfd selftest

[Shuah Khan]

On 3/25/22 9:21 AM, Christian Brauner wrote:
> On Thu, Mar 24, 2022 at 03:39:29PM -0700, Axel Rasmussen wrote:
>> I fixed a few warnings like this in commit e2aa5e650b07
>> ("selftests: fixup build warnings in pidfd / clone3 tests"), but I
>> missed this one by mistake. Since this variable is unused, remove it.
>>
>> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
>> ---
> 
> Looks good,
> Reviewed-by: Christian Brauner <brauner@kernel.org>
> 

Thank you both. Will apply it for Linux 5.18-rc2.

thanks,
-- Shuah