Re: [PATCH v3 54/68] selftests/rseq: Drop define _GNU_SOURCE
On Thu, May 9, 2024 at 1:16 PM Mathieu Desnoyers
wrote:
>
> On 2024-05-09 15:58, Edward Liaw wrote:
> > _GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
> > redefinition warnings.
> >
> > Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX")
>
> The patch per se looks fine, except for the "Fixes" tag.
>
> Commit 809216233555 introduces use of asprintf in kselftest_harness.h
> which is used by (all ?) selftests, including the rseq ones. However,
> the rseq selftests each have the #define _GNU_SOURCE, which would have
> been OK without those further changes.
>
> So this patch is more about consolidating where the _GNU_SOURCE is
> defined, which is OK with me, but not so much about "fixing" an
> issue with commit 809216233555.
>
> A "Fix" is something to be backported to stable kernels, and I
> don't think this patch reaches that threshold.
>
> If anything, this patch removes a warning that gets added by
> https://lore.kernel.org/lkml/20240509200022.253089-1-edl...@google.com/T/#mf8438d03de6e2b613da4f86d4f60c5fe1c5f8483
> within the same series.
>
> Arguably, each #define _GNU_SOURCE could have been first protected
> by a #ifndef guard to eliminate this transient warning, and there
> would be nothing to "fix" in this consolidation series.
That makes sense. I can remove the fixes tags. 809216233555 will
likely be reverted first anyway, and you're right that the focus of
this patch series is on consolidating _GNU_SOURCE.
>
> Thoughts ?
>
> Thanks,
>
> Mathieu
>
> > Reviewed-by: John Hubbard
> > Reviewed-by: Muhammad Usama Anjum
> > Signed-off-by: Edward Liaw
> > ---
> > tools/testing/selftests/rseq/basic_percpu_ops_test.c | 1 -
> > tools/testing/selftests/rseq/basic_test.c| 2 --
> > tools/testing/selftests/rseq/param_test.c| 1 -
> > tools/testing/selftests/rseq/rseq.c | 2 --
> > 4 files changed, 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> > b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> > index 2348d2c20d0a..5961c24ee1ae 100644
> > --- a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> > +++ b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> > @@ -1,5 +1,4 @@
> > // SPDX-License-Identifier: LGPL-2.1
> > -#define _GNU_SOURCE
> > #include
> > #include
> > #include
> > diff --git a/tools/testing/selftests/rseq/basic_test.c
> > b/tools/testing/selftests/rseq/basic_test.c
> > index 295eea16466f..1fed749b4bd7 100644
> > --- a/tools/testing/selftests/rseq/basic_test.c
> > +++ b/tools/testing/selftests/rseq/basic_test.c
> > @@ -2,8 +2,6 @@
> > /*
> >* Basic test coverage for critical regions and rseq_current_cpu().
> >*/
> > -
> > -#define _GNU_SOURCE
> > #include
> > #include
> > #include
> > diff --git a/tools/testing/selftests/rseq/param_test.c
> > b/tools/testing/selftests/rseq/param_test.c
> > index 2f37961240ca..48a55d94eb72 100644
> > --- a/tools/testing/selftests/rseq/param_test.c
> > +++ b/tools/testing/selftests/rseq/param_test.c
> > @@ -1,5 +1,4 @@
> > // SPDX-License-Identifier: LGPL-2.1
> > -#define _GNU_SOURCE
> > #include
> > #include
> > #include
> > diff --git a/tools/testing/selftests/rseq/rseq.c
> > b/tools/testing/selftests/rseq/rseq.c
> > index 96e812bdf8a4..88602889414c 100644
> > --- a/tools/testing/selftests/rseq/rseq.c
> > +++ b/tools/testing/selftests/rseq/rseq.c
> > @@ -14,8 +14,6 @@
> >* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >* Lesser General Public License for more details.
> >*/
> > -
> > -#define _GNU_SOURCE
> > #include
> > #include
> > #include
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
>
Re: [PATCH v3 54/68] selftests/rseq: Drop define _GNU_SOURCE
On 2024-05-09 15:58, Edward Liaw wrote:
_GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
redefinition warnings.
Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX")
The patch per se looks fine, except for the "Fixes" tag.
Commit 809216233555 introduces use of asprintf in kselftest_harness.h
which is used by (all ?) selftests, including the rseq ones. However,
the rseq selftests each have the #define _GNU_SOURCE, which would have
been OK without those further changes.
So this patch is more about consolidating where the _GNU_SOURCE is
defined, which is OK with me, but not so much about "fixing" an
issue with commit 809216233555.
A "Fix" is something to be backported to stable kernels, and I
don't think this patch reaches that threshold.
If anything, this patch removes a warning that gets added by
https://lore.kernel.org/lkml/20240509200022.253089-1-edl...@google.com/T/#mf8438d03de6e2b613da4f86d4f60c5fe1c5f8483
within the same series.
Arguably, each #define _GNU_SOURCE could have been first protected
by a #ifndef guard to eliminate this transient warning, and there
would be nothing to "fix" in this consolidation series.
Thoughts ?
Thanks,
Mathieu
Reviewed-by: John Hubbard
Reviewed-by: Muhammad Usama Anjum
Signed-off-by: Edward Liaw
---
tools/testing/selftests/rseq/basic_percpu_ops_test.c | 1 -
tools/testing/selftests/rseq/basic_test.c| 2 --
tools/testing/selftests/rseq/param_test.c| 1 -
tools/testing/selftests/rseq/rseq.c | 2 --
4 files changed, 6 deletions(-)
diff --git a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
index 2348d2c20d0a..5961c24ee1ae 100644
--- a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
+++ b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: LGPL-2.1
-#define _GNU_SOURCE
#include
#include
#include
diff --git a/tools/testing/selftests/rseq/basic_test.c
b/tools/testing/selftests/rseq/basic_test.c
index 295eea16466f..1fed749b4bd7 100644
--- a/tools/testing/selftests/rseq/basic_test.c
+++ b/tools/testing/selftests/rseq/basic_test.c
@@ -2,8 +2,6 @@
/*
* Basic test coverage for critical regions and rseq_current_cpu().
*/
-
-#define _GNU_SOURCE
#include
#include
#include
diff --git a/tools/testing/selftests/rseq/param_test.c
b/tools/testing/selftests/rseq/param_test.c
index 2f37961240ca..48a55d94eb72 100644
--- a/tools/testing/selftests/rseq/param_test.c
+++ b/tools/testing/selftests/rseq/param_test.c
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: LGPL-2.1
-#define _GNU_SOURCE
#include
#include
#include
diff --git a/tools/testing/selftests/rseq/rseq.c
b/tools/testing/selftests/rseq/rseq.c
index 96e812bdf8a4..88602889414c 100644
--- a/tools/testing/selftests/rseq/rseq.c
+++ b/tools/testing/selftests/rseq/rseq.c
@@ -14,8 +14,6 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*/
-
-#define _GNU_SOURCE
#include
#include
#include
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
[PATCH v3 54/68] selftests/rseq: Drop define _GNU_SOURCE
_GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
redefinition warnings.
Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX")
Reviewed-by: John Hubbard
Reviewed-by: Muhammad Usama Anjum
Signed-off-by: Edward Liaw
---
tools/testing/selftests/rseq/basic_percpu_ops_test.c | 1 -
tools/testing/selftests/rseq/basic_test.c| 2 --
tools/testing/selftests/rseq/param_test.c| 1 -
tools/testing/selftests/rseq/rseq.c | 2 --
4 files changed, 6 deletions(-)
diff --git a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
index 2348d2c20d0a..5961c24ee1ae 100644
--- a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
+++ b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: LGPL-2.1
-#define _GNU_SOURCE
#include
#include
#include
diff --git a/tools/testing/selftests/rseq/basic_test.c
b/tools/testing/selftests/rseq/basic_test.c
index 295eea16466f..1fed749b4bd7 100644
--- a/tools/testing/selftests/rseq/basic_test.c
+++ b/tools/testing/selftests/rseq/basic_test.c
@@ -2,8 +2,6 @@
/*
* Basic test coverage for critical regions and rseq_current_cpu().
*/
-
-#define _GNU_SOURCE
#include
#include
#include
diff --git a/tools/testing/selftests/rseq/param_test.c
b/tools/testing/selftests/rseq/param_test.c
index 2f37961240ca..48a55d94eb72 100644
--- a/tools/testing/selftests/rseq/param_test.c
+++ b/tools/testing/selftests/rseq/param_test.c
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: LGPL-2.1
-#define _GNU_SOURCE
#include
#include
#include
diff --git a/tools/testing/selftests/rseq/rseq.c
b/tools/testing/selftests/rseq/rseq.c
index 96e812bdf8a4..88602889414c 100644
--- a/tools/testing/selftests/rseq/rseq.c
+++ b/tools/testing/selftests/rseq/rseq.c
@@ -14,8 +14,6 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*/
-
-#define _GNU_SOURCE
#include
#include
#include
--
2.45.0.118.g7fe29c98d7-goog
