1. Patchew
  2. linux
  3. View series

[PATCH RESEND] selftests: Suppress unused variable warning

Chen Linxuan posted 1 patch 4 months ago 
.../selftests/filesystems/mount-notify/mount-notify_test.c    | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH RESEND] selftests: Suppress unused variable warning
Posted by Chen Linxuan 4 months ago 
When running `make kselftest`, the following compilation warning was encountered:

mount-notify_test.c: In function ‘fanotify_rmdir’:
mount-notify_test.c:490:17: warning: ignoring return value of ‘chdir’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  490 |                 chdir("/");
      |                 ^~~~~~~~~~

This patch addresses the warning by
explicitly suppressing the unused result of the `chdir` function.

Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
---
 .../selftests/filesystems/mount-notify/mount-notify_test.c    | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
index 63ce708d93ed0..34afe27b7978f 100644
--- a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
+++ b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
@@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
 	ASSERT_GE(ret, 0);
 
 	if (ret == 0) {
-		chdir("/");
+		// Suppress -Wunused-result
+		// Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34
+		(void) !chdir("/");
 		unshare(CLONE_NEWNS);
 		mount("", "/", NULL, MS_REC|MS_PRIVATE, NULL);
 		umount2("/a", MNT_DETACH);
-- 
2.43.0
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by Shuah Khan 3 months, 3 weeks ago 
On 6/9/25 20:07, Chen Linxuan wrote:
> When running `make kselftest`, the following compilation warning was encountered:
> 
> mount-notify_test.c: In function ‘fanotify_rmdir’:
> mount-notify_test.c:490:17: warning: ignoring return value of ‘chdir’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>    490 |                 chdir("/");
>        |                 ^~~~~~~~~~
> 
> This patch addresses the warning by
> explicitly suppressing the unused result of the `chdir` function.
> 
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> ---
>   .../selftests/filesystems/mount-notify/mount-notify_test.c    | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> index 63ce708d93ed0..34afe27b7978f 100644
> --- a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> +++ b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
>   	ASSERT_GE(ret, 0);
>   
>   	if (ret == 0) {
> -		chdir("/");
> +		// Suppress -Wunused-result
> +		// Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34
> +		(void) !chdir("/");

Why not fix the problem the right way by checking the return value.
Suppressing the error isn't useful.


>   		unshare(CLONE_NEWNS);
>   		mount("", "/", NULL, MS_REC|MS_PRIVATE, NULL);
>   		umount2("/a", MNT_DETACH);

thanks,
-- Shuah
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by Chen Linxuan 3 months, 3 weeks ago 
On Thu, Jun 19, 2025 at 5:23 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/9/25 20:07, Chen Linxuan wrote:
> > When running `make kselftest`, the following compilation warning was encountered:
> >
> > mount-notify_test.c: In function ‘fanotify_rmdir’:
> > mount-notify_test.c:490:17: warning: ignoring return value of ‘chdir’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> >    490 |                 chdir("/");
> >        |                 ^~~~~~~~~~
> >
> > This patch addresses the warning by
> > explicitly suppressing the unused result of the `chdir` function.
> >
> > Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> > ---
> >   .../selftests/filesystems/mount-notify/mount-notify_test.c    | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> > index 63ce708d93ed0..34afe27b7978f 100644
> > --- a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> > +++ b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> > @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
> >       ASSERT_GE(ret, 0);
> >
> >       if (ret == 0) {
> > -             chdir("/");
> > +             // Suppress -Wunused-result
> > +             // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34
> > +             (void) !chdir("/");
>
> Why not fix the problem the right way by checking the return value.
> Suppressing the error isn't useful.

The code is already handling cleanup in error cases,
and I don't think checking the result of chdir would be useful here.

>
>
> >               unshare(CLONE_NEWNS);
> >               mount("", "/", NULL, MS_REC|MS_PRIVATE, NULL);
> >               umount2("/a", MNT_DETACH);
>
> thanks,
> -- Shuah
>
>
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by Shuah Khan 3 months, 3 weeks ago 
On 6/19/25 01:46, Chen Linxuan wrote:
> On Thu, Jun 19, 2025 at 5:23 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 6/9/25 20:07, Chen Linxuan wrote:
>>> When running `make kselftest`, the following compilation warning was encountered:
>>>
>>> mount-notify_test.c: In function ‘fanotify_rmdir’:
>>> mount-notify_test.c:490:17: warning: ignoring return value of ‘chdir’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>>>     490 |                 chdir("/");
>>>         |                 ^~~~~~~~~~
>>>
>>> This patch addresses the warning by
>>> explicitly suppressing the unused result of the `chdir` function.
>>>
>>> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
>>> ---
>>>    .../selftests/filesystems/mount-notify/mount-notify_test.c    | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
>>> index 63ce708d93ed0..34afe27b7978f 100644
>>> --- a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
>>> +++ b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
>>> @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
>>>        ASSERT_GE(ret, 0);
>>>
>>>        if (ret == 0) {
>>> -             chdir("/");
>>> +             // Suppress -Wunused-result
>>> +             // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34
>>> +             (void) !chdir("/");
>>>> Why not fix the problem the right way by checking the return value.
>> Suppressing the error isn't useful.
> 
> The code is already handling cleanup in error cases,
> and I don't think checking the result of chdir would be useful here.

We check for chdir() in several tools in the kernel. Add a check for
it instead of suppressing the [-Wunused-result] - suppressing doesn't
do any good.

thanks,
-- Shuah
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by John Hubbard 3 months, 3 weeks ago 
On 6/19/25 10:00 AM, Shuah Khan wrote:
> On 6/19/25 01:46, Chen Linxuan wrote:
>> On Thu, Jun 19, 2025 at 5:23 AM Shuah Khan <skhan@linuxfoundation.org> 
>> wrote:
>>> On 6/9/25 20:07, Chen Linxuan wrote:
...
>>>> diff --git a/tools/testing/selftests/filesystems/mount-notify/mount- 
>>>> notify_test.c b/tools/testing/selftests/filesystems/mount-notify/ 
>>>> mount-notify_test.c
>>>> index 63ce708d93ed0..34afe27b7978f 100644
>>>> --- a/tools/testing/selftests/filesystems/mount-notify/mount- 
>>>> notify_test.c
>>>> +++ b/tools/testing/selftests/filesystems/mount-notify/mount- 
>>>> notify_test.c
>>>> @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
>>>>        ASSERT_GE(ret, 0);
>>>>
>>>>        if (ret == 0) {
>>>> -             chdir("/");
>>>> +             // Suppress -Wunused-result
>>>> +             // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi? 
>>>> id=66425#c34
>>>> +             (void) !chdir("/");

This is quite ugly. :)

>>>>> Why not fix the problem the right way by checking the return value.
>>> Suppressing the error isn't useful.
>>
>> The code is already handling cleanup in error cases,
>> and I don't think checking the result of chdir would be useful here.
> 

Why not just fail with the appropriate test result, if chdir() fails
here, instead of making a bit of a mess with odd void casts to a
negated return value, and a reference to a compiler bug report?

Really, Shuah is putting you on the right path here.

> We check for chdir() in several tools in the kernel. Add a check for
> it instead of suppressing the [-Wunused-result] - suppressing doesn't
> do any good.
> 
> thanks,
> -- Shuah
> 
> 

thanks,
-- 
John Hubbard
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by Shuah Khan 3 months, 3 weeks ago 
On 6/19/25 13:00, John Hubbard wrote:
> On 6/19/25 10:00 AM, Shuah Khan wrote:
>> On 6/19/25 01:46, Chen Linxuan wrote:
>>> On Thu, Jun 19, 2025 at 5:23 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>>> On 6/9/25 20:07, Chen Linxuan wrote:
> ...
>>>>> diff --git a/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c b/tools/testing/selftests/filesystems/mount-notify/ mount-notify_test.c
>>>>> index 63ce708d93ed0..34afe27b7978f 100644
>>>>> --- a/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c
>>>>> +++ b/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c
>>>>> @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
>>>>>        ASSERT_GE(ret, 0);
>>>>>
>>>>>        if (ret == 0) {
>>>>> -             chdir("/");
>>>>> +             // Suppress -Wunused-result
>>>>> +             // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi? id=66425#c34
>>>>> +             (void) !chdir("/");
> 
> This is quite ugly. :)
> 
>>>>>> Why not fix the problem the right way by checking the return value.
>>>> Suppressing the error isn't useful.
>>>
>>> The code is already handling cleanup in error cases,
>>> and I don't think checking the result of chdir would be useful here.
>>
> 
> Why not just fail with the appropriate test result, if chdir() fails
> here, instead of making a bit of a mess with odd void casts to a
> negated return value, and a reference to a compiler bug report?
> 
> Really, Shuah is putting you on the right path here.

Ha. I didn't ask to suppress the error with the cast. I asked
to check the return and fail.

> 
>> We check for chdir() in several tools in the kernel. Add a check for
>> it instead of suppressing the [-Wunused-result] - suppressing doesn't
>> do any good.

This is what I said.

thanks,
-- Shuah
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by Chen Linxuan 3 months, 3 weeks ago 
On Fri, Jun 20, 2025 at 5:41 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/19/25 13:00, John Hubbard wrote:
> > On 6/19/25 10:00 AM, Shuah Khan wrote:
> >> On 6/19/25 01:46, Chen Linxuan wrote:
> >>> On Thu, Jun 19, 2025 at 5:23 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
> >>>> On 6/9/25 20:07, Chen Linxuan wrote:
> > ...
> >>>>> diff --git a/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c b/tools/testing/selftests/filesystems/mount-notify/ mount-notify_test.c
> >>>>> index 63ce708d93ed0..34afe27b7978f 100644
> >>>>> --- a/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c
> >>>>> +++ b/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c
> >>>>> @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
> >>>>>        ASSERT_GE(ret, 0);
> >>>>>
> >>>>>        if (ret == 0) {
> >>>>> -             chdir("/");
> >>>>> +             // Suppress -Wunused-result
> >>>>> +             // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi? id=66425#c34
> >>>>> +             (void) !chdir("/");
> >
> > This is quite ugly. :)

I agree with you. :)

> >
> >>>>>> Why not fix the problem the right way by checking the return value.
> >>>> Suppressing the error isn't useful.
> >>>
> >>> The code is already handling cleanup in error cases,
> >>> and I don't think checking the result of chdir would be useful here.

I think I was mistaken earlier. Here we are in the child process after
a fork, not handling an error case.
I think simply calling exit(-1) here when chdir failed should be
enough to make the test fail in the parent process.
Maybe we should do the same for other similar calls as well. I will
send a v2 soon.

> >>
> >
> > Why not just fail with the appropriate test result, if chdir() fails
> > here, instead of making a bit of a mess with odd void casts to a
> > negated return value, and a reference to a compiler bug report?
> >
> > Really, Shuah is putting you on the right path here.
>
> Ha. I didn't ask to suppress the error with the cast. I asked
> to check the return and fail.
>
> >
> >> We check for chdir() in several tools in the kernel. Add a check for
> >> it instead of suppressing the [-Wunused-result] - suppressing doesn't
> >> do any good.
>
> This is what I said.
>
> thanks,
> -- Shuah
>
>
Re: [PATCH RESEND] selftests: Suppress unused variable warning
Posted by Shuah Khan 3 months, 3 weeks ago 
On 6/19/25 15:40, Shuah Khan wrote:
> On 6/19/25 13:00, John Hubbard wrote:
>> On 6/19/25 10:00 AM, Shuah Khan wrote:
>>> On 6/19/25 01:46, Chen Linxuan wrote:
>>>> On Thu, Jun 19, 2025 at 5:23 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>>>> On 6/9/25 20:07, Chen Linxuan wrote:
>> ...
>>>>>> diff --git a/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c b/tools/testing/selftests/filesystems/mount-notify/ mount-notify_test.c
>>>>>> index 63ce708d93ed0..34afe27b7978f 100644
>>>>>> --- a/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c
>>>>>> +++ b/tools/testing/selftests/filesystems/mount-notify/mount- notify_test.c
>>>>>> @@ -465,7 +465,9 @@ TEST_F(fanotify, rmdir)
>>>>>>        ASSERT_GE(ret, 0);
>>>>>>
>>>>>>        if (ret == 0) {
>>>>>> -             chdir("/");
>>>>>> +             // Suppress -Wunused-result
>>>>>> +             // Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi? id=66425#c34
>>>>>> +             (void) !chdir("/");
>>
>> This is quite ugly. :)
>>
>>>>>>> Why not fix the problem the right way by checking the return value.
>>>>> Suppressing the error isn't useful.
>>>>
>>>> The code is already handling cleanup in error cases,
>>>> and I don't think checking the result of chdir would be useful here.
>>>
>>
>> Why not just fail with the appropriate test result, if chdir() fails
>> here, instead of making a bit of a mess with odd void casts to a
>> negated return value, and a reference to a compiler bug report?
>>
>> Really, Shuah is putting you on the right path here.
> 
> Ha. I didn't ask to suppress the error with the cast. I asked
> to check the return and fail.
> 
>>

Sorry John. I didn't read it correctly the first time.
I probably go get more coffee. :)

thanks,
-- Shuah

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.