[PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE

Edward Liaw posted 68 patches 1 year, 6 months ago
[PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Edward Liaw 1 year, 6 months ago
From: Mark Brown <broonie@kernel.org>

Commit daef47b89efd0b7 ("selftests: Compile kselftest headers with
-D_GNU_SOURCE") adds a static_assert() which means that things which
would be warnings about undeclared functions get escalated into build
failures.  While we do actually want _GNU_SOURCE to be defined for users
of kselftest_harness we haven't actually done that yet and this is
causing widespread build breaks which were previously warnings about
uses of asprintf() without prototypes, including causing other test
programs in the same directory to fail to build.

Since the build failures that are introduced cause additional issues due
to make stopping builds early replace the static_assert() with a
missing without making the error more severe than it already was.  This
will be moot once the issue is fixed properly but reduces the disruption
while that happens.

Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
 tools/testing/selftests/kselftest_harness.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index b634969cbb6f..8e53686b71cb 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -51,7 +51,7 @@
 #define __KSELFTEST_HARNESS_H
 
 #ifndef _GNU_SOURCE
-#define _GNU_SOURCE
+#warning kselftest harness requires _GNU_SOURCE to be defined
 #endif
 #include <asm/types.h>
 #include <ctype.h>
-- 
2.45.1.288.g0e0cd299f1-goog
Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Mark Brown 1 year, 6 months ago
On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:

> to make stopping builds early replace the static_assert() with a
> missing without making the error more severe than it already was.  This
> will be moot once the issue is fixed properly but reduces the disruption
> while that happens.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
>  tools/testing/selftests/kselftest_harness.h | 2 +-

You've not provided a Signed-off-by for this so people can't do anything
with it, please see Documentation/process/submitting-patches.rst for
details on what this is and why it's important.
Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Edward Liaw 1 year, 6 months ago
On Wed, May 22, 2024 at 4:21 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:
>
> > to make stopping builds early replace the static_assert() with a
> > missing without making the error more severe than it already was.  This
> > will be moot once the issue is fixed properly but reduces the disruption
> > while that happens.
> >
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > ---
> >  tools/testing/selftests/kselftest_harness.h | 2 +-
>
> You've not provided a Signed-off-by for this so people can't do anything
> with it, please see Documentation/process/submitting-patches.rst for
> details on what this is and why it's important.

Sorry, my mistake, I forgot to add it after cherry-picking.  If added
in a reply like so would it suffice, or would I need to send another
patch?

Signed-off-by: Edward Liaw <edliaw@google.com>
Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by John Hubbard 1 year, 6 months ago
On 5/22/24 10:46 AM, Edward Liaw wrote:
> On Wed, May 22, 2024 at 4:21 AM Mark Brown <broonie@kernel.org> wrote:
>> On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:
...
>> You've not provided a Signed-off-by for this so people can't do anything
>> with it, please see Documentation/process/submitting-patches.rst for
>> details on what this is and why it's important.
> 
> Sorry, my mistake, I forgot to add it after cherry-picking.  If added

Adding this to your .gitconfig would cover you for cases like this, I think
it's pretty common to do this:

[format]
     signoff = true



thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Shuah Khan 1 year, 6 months ago
On 5/22/24 20:28, John Hubbard wrote:
> On 5/22/24 10:46 AM, Edward Liaw wrote:
>> On Wed, May 22, 2024 at 4:21 AM Mark Brown <broonie@kernel.org> wrote:
>>> On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:
> ...
>>> You've not provided a Signed-off-by for this so people can't do anything
>>> with it, please see Documentation/process/submitting-patches.rst for
>>> details on what this is and why it's important.
>>
>> Sorry, my mistake, I forgot to add it after cherry-picking.  If added
> 
> Adding this to your .gitconfig would cover you for cases like this, I think
> it's pretty common to do this:
> 
> [format]
>      signoff = true
> 
> 

Mark, Edward,

Is this patch still necessary of the series is dropped?

thanks,
-- Shuah

Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Edward Liaw 1 year, 6 months ago
On Thu, May 23, 2024 at 11:02 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 5/22/24 20:28, John Hubbard wrote:
> > On 5/22/24 10:46 AM, Edward Liaw wrote:
> >> On Wed, May 22, 2024 at 4:21 AM Mark Brown <broonie@kernel.org> wrote:
> >>> On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:
> > ...
> >>> You've not provided a Signed-off-by for this so people can't do anything
> >>> with it, please see Documentation/process/submitting-patches.rst for
> >>> details on what this is and why it's important.
> >>
> >> Sorry, my mistake, I forgot to add it after cherry-picking.  If added
> >
> > Adding this to your .gitconfig would cover you for cases like this, I think
> > it's pretty common to do this:
> >
> > [format]
> >      signoff = true
> >
> >

Thanks Mark, I'll add that.

>
> Mark, Edward,
>
> Is this patch still necessary of the series is dropped?
>
> thanks,
> -- Shuah
>

No, it is not necessary anymore.
Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Shuah Khan 1 year, 6 months ago
On 5/23/24 12:12, Edward Liaw wrote:
> On Thu, May 23, 2024 at 11:02 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 5/22/24 20:28, John Hubbard wrote:
>>> On 5/22/24 10:46 AM, Edward Liaw wrote:
>>>> On Wed, May 22, 2024 at 4:21 AM Mark Brown <broonie@kernel.org> wrote:
>>>>> On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:
>>> ...
>>>>> You've not provided a Signed-off-by for this so people can't do anything
>>>>> with it, please see Documentation/process/submitting-patches.rst for
>>>>> details on what this is and why it's important.
>>>>
>>>> Sorry, my mistake, I forgot to add it after cherry-picking.  If added
>>>
>>> Adding this to your .gitconfig would cover you for cases like this, I think
>>> it's pretty common to do this:
>>>
>>> [format]
>>>       signoff = true
>>>
>>>
> 
> Thanks Mark, I'll add that.
> 
>>
>> Mark, Edward,
>>
>> Is this patch still necessary of the series is dropped?
>>
>> thanks,
>> -- Shuah
>>
> 
> No, it is not necessary anymore.

Thank you Edward.

thanks,
-- Shuah
Re: [PATCH v5 02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE
Posted by Vitaly Chikunov 1 year, 6 months ago
On Wed, May 22, 2024 at 12:56:48AM +0000, Edward Liaw wrote:
> From: Mark Brown <broonie@kernel.org>
> 
> Commit daef47b89efd0b7 ("selftests: Compile kselftest headers with
> -D_GNU_SOURCE") adds a static_assert() which means that things which
> would be warnings about undeclared functions get escalated into build
> failures.  While we do actually want _GNU_SOURCE to be defined for users
> of kselftest_harness we haven't actually done that yet and this is
> causing widespread build breaks which were previously warnings about
> uses of asprintf() without prototypes, including causing other test
> programs in the same directory to fail to build.
> 
> Since the build failures that are introduced cause additional issues due
> to make stopping builds early replace the static_assert() with a
> missing without making the error more severe than it already was.  This
> will be moot once the issue is fixed properly but reduces the disruption
> while that happens.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
>  tools/testing/selftests/kselftest_harness.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index b634969cbb6f..8e53686b71cb 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -51,7 +51,7 @@
>  #define __KSELFTEST_HARNESS_H
>  
>  #ifndef _GNU_SOURCE
> -#define _GNU_SOURCE
> +#warning kselftest harness requires _GNU_SOURCE to be defined

Maybe this should be `#error` if it really requires?

Thanks,

>  #endif
>  #include <asm/types.h>
>  #include <ctype.h>
> -- 
> 2.45.1.288.g0e0cd299f1-goog
>