| Message ID | 20240507063534.4191447-1-tao1.su@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | selftests: Add _GNU_SOURCE definition when including kselftest_harness.h | expand |
On Tue, 7 May 2024 14:35:34 +0800 Tao Su <tao1.su@linux.intel.com> wrote: > asprintf() is declared in stdio.h when defining _GNU_SOURCE, but stdio.h > is so common that many files don’t define _GNU_SOURCE before including > stdio.h, and defining _GNU_SOURCE after including stdio.h will no longer > take effect. > > Since kselftest_harness.h introduces asprintf(), it is necessary to add > _GNU_SOURCE definition in all selftests including kselftest_harness.h, > otherwise, there will be warnings or even errors during compilation. > There are already many selftests that define _GNU_SOURCE or put the > include of kselftest_harness.h at the very beginning of the .c file, just > add the _GNU_SOURCE definition in the tests that have compilation warnings. That asprintf() continues to cause problems. How about we just remove it? Do the malloc(snprintf(str, 0, ...)) separately?
On Tue, May 07, 2024 at 10:06:51AM -0700, Andrew Morton wrote: > On Tue, 7 May 2024 14:35:34 +0800 Tao Su <tao1.su@linux.intel.com> wrote: > > > asprintf() is declared in stdio.h when defining _GNU_SOURCE, but stdio.h > > is so common that many files don’t define _GNU_SOURCE before including > > stdio.h, and defining _GNU_SOURCE after including stdio.h will no longer > > take effect. > > > > Since kselftest_harness.h introduces asprintf(), it is necessary to add > > _GNU_SOURCE definition in all selftests including kselftest_harness.h, > > otherwise, there will be warnings or even errors during compilation. > > There are already many selftests that define _GNU_SOURCE or put the > > include of kselftest_harness.h at the very beginning of the .c file, just > > add the _GNU_SOURCE definition in the tests that have compilation warnings. > > That asprintf() continues to cause problems. How about we just remove > it? Do the malloc(snprintf(str, 0, ...)) separately? Removing asprintf() is indeed an good option, but using snprintf(str, 0, ...) to get string size may go against the original intention of commit 38c957f07038. Back to commit 38c957f07038, I don't see any advantage in using LINE_MAX. Can we use a fixed value instead of LINE_MAX? E.g., 1024, 2048. Then we just need to revert commit 809216233555.
On Wed, 8 May 2024 10:55:05 +0800 Tao Su wrote: > Back to commit 38c957f07038, I don't see any advantage in using LINE_MAX. > Can we use a fixed value instead of LINE_MAX? E.g., 1024, 2048. Then we > just need to revert commit 809216233555. SGTM, FWIW. The print is printing a test summary line, printing more than 1k seems rather unreasonable. Other facilities, like TH_LOG(), should be used for displaying longer info.
On Wed, May 8, 2024 at 7:00 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 8 May 2024 10:55:05 +0800 Tao Su wrote: > > Back to commit 38c957f07038, I don't see any advantage in using LINE_MAX. > > Can we use a fixed value instead of LINE_MAX? E.g., 1024, 2048. Then we > > just need to revert commit 809216233555. > > SGTM, FWIW. The print is printing a test summary line, printing more > than 1k seems rather unreasonable. Other facilities, like TH_LOG(), > should be used for displaying longer info. I also submitted some patches to fix the _GNU_SOURCE issues here: https://lore.kernel.org/linux-kselftest/20240507214254.2787305-1-edliaw@google.com/ I'm fine with this approach. It's aligned to what Sean suggested there, since it's causing a lot of troubles for the release cycle.
On Wed, May 08, 2024 at 07:00:03AM -0700, Jakub Kicinski wrote: > On Wed, 8 May 2024 10:55:05 +0800 Tao Su wrote: > > Back to commit 38c957f07038, I don't see any advantage in using LINE_MAX. > > Can we use a fixed value instead of LINE_MAX? E.g., 1024, 2048. Then we > > just need to revert commit 809216233555. > > SGTM, FWIW. The print is printing a test summary line, printing more > than 1k seems rather unreasonable. Other facilities, like TH_LOG(), > should be used for displaying longer info. Thanks, do you think 1k is enough for test_name? >
On Wed, May 08, 2024 at 10:57:47AM -0700, Edward Liaw wrote: > On Wed, May 8, 2024 at 7:00 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Wed, 8 May 2024 10:55:05 +0800 Tao Su wrote: > > > Back to commit 38c957f07038, I don't see any advantage in using LINE_MAX. > > > Can we use a fixed value instead of LINE_MAX? E.g., 1024, 2048. Then we > > > just need to revert commit 809216233555. > > > > SGTM, FWIW. The print is printing a test summary line, printing more > > than 1k seems rather unreasonable. Other facilities, like TH_LOG(), > > should be used for displaying longer info. > > I also submitted some patches to fix the _GNU_SOURCE issues here: > https://lore.kernel.org/linux-kselftest/20240507214254.2787305-1-edliaw@google.com/ > > I'm fine with this approach. It's aligned to what Sean suggested > there, since it's causing a lot of troubles for the release cycle. Thanks, I will submit patches with this approach soon.
On Thu, 9 May 2024 13:08:36 +0800 Tao Su wrote: > > SGTM, FWIW. The print is printing a test summary line, printing more > > than 1k seems rather unreasonable. Other facilities, like TH_LOG(), > > should be used for displaying longer info. > > Thanks, do you think 1k is enough for test_name? Definitely.
diff --git a/tools/testing/selftests/alsa/test-pcmtest-driver.c b/tools/testing/selftests/alsa/test-pcmtest-driver.c index ca81afa4ee90..5a01100d459d 100644 --- a/tools/testing/selftests/alsa/test-pcmtest-driver.c +++ b/tools/testing/selftests/alsa/test-pcmtest-driver.c @@ -5,6 +5,7 @@ * * Copyright 2023 Ivan Orlov <ivan.orlov0322@gmail.com> */ +#define _GNU_SOURCE #include <string.h> #include <alsa/asoundlib.h> #include "../kselftest_harness.h" diff --git a/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c b/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c index f3c2239228b1..40f3e81b1a6c 100644 --- a/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c +++ b/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c @@ -4,6 +4,7 @@ * * Tests for KVM paravirtual feature disablement */ +#define _GNU_SOURCE #include <asm/kvm_para.h> #include <linux/kvm_para.h> #include <linux/stringify.h> diff --git a/tools/testing/selftests/nci/nci_dev.c b/tools/testing/selftests/nci/nci_dev.c index 1562aa7d60b0..7cf18aced644 100644 --- a/tools/testing/selftests/nci/nci_dev.c +++ b/tools/testing/selftests/nci/nci_dev.c @@ -6,6 +6,7 @@ * Test code for nci */ +#define _GNU_SOURCE #include <stdlib.h> #include <errno.h> #include <string.h> diff --git a/tools/testing/selftests/net/bind_wildcard.c b/tools/testing/selftests/net/bind_wildcard.c index b7b54d646b93..f271e2ee6c7a 100644 --- a/tools/testing/selftests/net/bind_wildcard.c +++ b/tools/testing/selftests/net/bind_wildcard.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright Amazon.com Inc. or its affiliates. */ +#define _GNU_SOURCE #include <sys/socket.h> #include <netinet/in.h> diff --git a/tools/testing/selftests/net/ip_local_port_range.c b/tools/testing/selftests/net/ip_local_port_range.c index 193b82745fd8..fadefb0ab147 100644 --- a/tools/testing/selftests/net/ip_local_port_range.c +++ b/tools/testing/selftests/net/ip_local_port_range.c @@ -7,6 +7,7 @@ * Don't run these directly but with ip_local_port_range.sh script. */ +#define _GNU_SOURCE #include <fcntl.h> #include <netinet/ip.h> diff --git a/tools/testing/selftests/net/reuseaddr_ports_exhausted.c b/tools/testing/selftests/net/reuseaddr_ports_exhausted.c index 066efd30e294..4f6fb2fbb96d 100644 --- a/tools/testing/selftests/net/reuseaddr_ports_exhausted.c +++ b/tools/testing/selftests/net/reuseaddr_ports_exhausted.c @@ -17,6 +17,7 @@ * * Author: Kuniyuki Iwashima <kuniyu@amazon.co.jp> */ +#define _GNU_SOURCE #include <arpa/inet.h> #include <netinet/in.h> #include <sys/socket.h> diff --git a/tools/testing/selftests/prctl/set-anon-vma-name-test.c b/tools/testing/selftests/prctl/set-anon-vma-name-test.c index 4275cb256dce..e5ea821be241 100644 --- a/tools/testing/selftests/prctl/set-anon-vma-name-test.c +++ b/tools/testing/selftests/prctl/set-anon-vma-name-test.c @@ -3,6 +3,7 @@ * This test covers the anonymous VMA naming functionality through prctl calls */ +#define _GNU_SOURCE #include <errno.h> #include <sys/prctl.h> #include <stdio.h> diff --git a/tools/testing/selftests/prctl/set-process-name.c b/tools/testing/selftests/prctl/set-process-name.c index 562f707ba771..9cbfe9d38d72 100644 --- a/tools/testing/selftests/prctl/set-process-name.c +++ b/tools/testing/selftests/prctl/set-process-name.c @@ -3,6 +3,7 @@ * This test covers the PR_SET_NAME functionality of prctl calls */ +#define _GNU_SOURCE #include <errno.h> #include <sys/prctl.h> #include <string.h> diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c index 63ce02d1d5cc..2ace7a75c638 100644 --- a/tools/testing/selftests/rtc/rtctest.c +++ b/tools/testing/selftests/rtc/rtctest.c @@ -5,6 +5,7 @@ * Copyright (c) 2018 Alexandre Belloni <alexandre.belloni@bootlin.com> */ +#define _GNU_SOURCE #include <errno.h> #include <fcntl.h> #include <linux/rtc.h> diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9820b3809c69..bb6e795d06e2 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright(c) 2016-20 Intel Corporation. */ +#define _GNU_SOURCE #include <cpuid.h> #include <elf.h> #include <errno.h> diff --git a/tools/testing/selftests/tdx/tdx_guest_test.c b/tools/testing/selftests/tdx/tdx_guest_test.c index 81d8cb88ea1a..f966467d1ef1 100644 --- a/tools/testing/selftests/tdx/tdx_guest_test.c +++ b/tools/testing/selftests/tdx/tdx_guest_test.c @@ -7,6 +7,7 @@ * Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> */ +#define _GNU_SOURCE #include <sys/ioctl.h> #include <errno.h> diff --git a/tools/testing/selftests/user_events/dyn_test.c b/tools/testing/selftests/user_events/dyn_test.c index bdf9ab127488..9d090ba3bfc3 100644 --- a/tools/testing/selftests/user_events/dyn_test.c +++ b/tools/testing/selftests/user_events/dyn_test.c @@ -5,6 +5,7 @@ * Copyright (c) 2021 Beau Belgrave <beaub@linux.microsoft.com> */ +#define _GNU_SOURCE #include <errno.h> #include <linux/user_events.h> #include <stdio.h> diff --git a/tools/testing/selftests/user_events/ftrace_test.c b/tools/testing/selftests/user_events/ftrace_test.c index dcd7509fe2e0..25adef590a94 100644 --- a/tools/testing/selftests/user_events/ftrace_test.c +++ b/tools/testing/selftests/user_events/ftrace_test.c @@ -5,6 +5,7 @@ * Copyright (c) 2021 Beau Belgrave <beaub@linux.microsoft.com> */ +#define _GNU_SOURCE #include <errno.h> #include <linux/user_events.h> #include <stdio.h> diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c index 5288e768b207..176740a0fc02 100644 --- a/tools/testing/selftests/user_events/perf_test.c +++ b/tools/testing/selftests/user_events/perf_test.c @@ -5,6 +5,7 @@ * Copyright (c) 2021 Beau Belgrave <beaub@linux.microsoft.com> */ +#define _GNU_SOURCE #include <errno.h> #include <linux/user_events.h> #include <linux/perf_event.h>
asprintf() is declared in stdio.h when defining _GNU_SOURCE, but stdio.h is so common that many files don’t define _GNU_SOURCE before including stdio.h, and defining _GNU_SOURCE after including stdio.h will no longer take effect. Since kselftest_harness.h introduces asprintf(), it is necessary to add _GNU_SOURCE definition in all selftests including kselftest_harness.h, otherwise, there will be warnings or even errors during compilation. There are already many selftests that define _GNU_SOURCE or put the include of kselftest_harness.h at the very beginning of the .c file, just add the _GNU_SOURCE definition in the tests that have compilation warnings. Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX") Signed-off-by: Tao Su <tao1.su@linux.intel.com> --- tools/testing/selftests/alsa/test-pcmtest-driver.c | 1 + tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c | 1 + tools/testing/selftests/nci/nci_dev.c | 1 + tools/testing/selftests/net/bind_wildcard.c | 1 + tools/testing/selftests/net/ip_local_port_range.c | 1 + tools/testing/selftests/net/reuseaddr_ports_exhausted.c | 1 + tools/testing/selftests/prctl/set-anon-vma-name-test.c | 1 + tools/testing/selftests/prctl/set-process-name.c | 1 + tools/testing/selftests/rtc/rtctest.c | 1 + tools/testing/selftests/sgx/main.c | 1 + tools/testing/selftests/tdx/tdx_guest_test.c | 1 + tools/testing/selftests/user_events/dyn_test.c | 1 + tools/testing/selftests/user_events/ftrace_test.c | 1 + tools/testing/selftests/user_events/perf_test.c | 1 + 14 files changed, 14 insertions(+) base-commit: dccb07f2914cdab2ac3a5b6c98406f765acab803