Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
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.
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
On Wed, May 08, 2024 at 10:57:47AM -0700, Edward Liaw wrote: > On Wed, May 8, 2024 at 7:00 AM 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. > > I also submitted some patches to fix the _GNU_SOURCE issues here: > https://lore.kernel.org/linux-kselftest/20240507214254.2787305-1-edl...@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.
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
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? >
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
On Wed, May 8, 2024 at 7:00 AM 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. I also submitted some patches to fix the _GNU_SOURCE issues here: https://lore.kernel.org/linux-kselftest/20240507214254.2787305-1-edl...@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.
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
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.
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
On Tue, May 07, 2024 at 10:06:51AM -0700, Andrew Morton wrote: > On Tue, 7 May 2024 14:35:34 +0800 Tao Su 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.
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
On Tue, 7 May 2024 14:35:34 +0800 Tao Su 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?
[PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.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
---
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(+)
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
*/
+#define _GNU_SOURCE
#include
#include
#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
#include
#include
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
#include
#include
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
#include
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
#include
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
*/
+#define _GNU_SOURCE
#include
#include
#include
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
#include
#include
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
#include
#include
diff --git a/tools/testing/selftests/rtc/rtctest.c
b/tools/testing/selftests/rtc/rtctest.c
index
