* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
@ 2024-12-05 8:04 ` Andrew Jones
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2024-12-05 8:04 UTC (permalink / raw)
To: Charlie Jenkins
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
> When compiling the pointer masking tests with -Wall this warning
> is present:
>
> pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
> pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
> declared with attribute ‘warn_unused_result’ [-Wunused-result]
> 203 | pwrite(fd, &value, 1, 0); |
> ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
> ignoring return value of ‘pwrite’ declared with attribute
> ‘warn_unused_result’ [-Wunused-result]
> 208 | pwrite(fd, &value, 1, 0);
>
> I came across this on riscv64-linux-gnu-gcc (Ubuntu
> 11.4.0-1ubuntu1~22.04).
>
> Fix this by checking that the number of bytes written equal the expected
> number of bytes written.
>
> Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
> Changes in v2:
> - I had ret != 2 for testing, I changed it to be ret != 1.
> - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
> ---
> tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
> index dee41b7ee3e3..229d85ccff50 100644
> --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
> +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
> @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
> {
> char value;
> int fd;
> + int ret;
>
> ksft_print_msg("Testing tagged address ABI sysctl\n");
>
> @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
> }
>
> value = '1';
> - pwrite(fd, &value, 1, 0);
> + ret = pwrite(fd, &value, 1, 0);
> + if (ret != 1) {
> + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> + return;
> + }
> +
> ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> "sysctl disabled\n");
>
> value = '0';
> - pwrite(fd, &value, 1, 0);
> - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
> - "sysctl enabled\n");
> + ret = pwrite(fd, &value, 1, 0);
> + if (ret != 1) {
> + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> + return;
> + }
Could make a wrapper function for pwrite() to avoid duplicating the ret
value check.
> +
> + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> + "sysctl disabled\n");
Why is this changed from expecting 0 for the return and being the
"sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
>
> set_tagged_addr_ctrl(0, false);
>
>
> ---
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
> --
> - Charlie
>
Not part of this patch, but now that I looked at
test_tagged_addr_abi_sysctl() I see that
ksft_test_result_skip() is duplicated.
Thanks,
drew
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
2024-12-05 8:04 ` Andrew Jones
@ 2024-12-05 11:15 ` Alexandre Ghiti
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandre Ghiti @ 2024-12-05 11:15 UTC (permalink / raw)
To: Andrew Jones, Charlie Jenkins
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
Hi Drew,
On 05/12/2024 09:04, Andrew Jones wrote:
> On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
>> When compiling the pointer masking tests with -Wall this warning
>> is present:
>>
>> pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
>> pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
>> declared with attribute ‘warn_unused_result’ [-Wunused-result]
>> 203 | pwrite(fd, &value, 1, 0); |
>> ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
>> ignoring return value of ‘pwrite’ declared with attribute
>> ‘warn_unused_result’ [-Wunused-result]
>> 208 | pwrite(fd, &value, 1, 0);
>>
>> I came across this on riscv64-linux-gnu-gcc (Ubuntu
>> 11.4.0-1ubuntu1~22.04).
>>
>> Fix this by checking that the number of bytes written equal the expected
>> number of bytes written.
>>
>> Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
>> ---
>> Changes in v2:
>> - I had ret != 2 for testing, I changed it to be ret != 1.
>> - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
>> ---
>> tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
>> index dee41b7ee3e3..229d85ccff50 100644
>> --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
>> +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
>> @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
>> {
>> char value;
>> int fd;
>> + int ret;
>>
>> ksft_print_msg("Testing tagged address ABI sysctl\n");
>>
>> @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
>> }
>>
>> value = '1';
>> - pwrite(fd, &value, 1, 0);
>> + ret = pwrite(fd, &value, 1, 0);
>> + if (ret != 1) {
>> + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
>> + return;
>> + }
>> +
>> ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
>> "sysctl disabled\n");
>>
>> value = '0';
>> - pwrite(fd, &value, 1, 0);
>> - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
>> - "sysctl enabled\n");
>> + ret = pwrite(fd, &value, 1, 0);
>> + if (ret != 1) {
>> + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
>> + return;
>> + }
> Could make a wrapper function for pwrite() to avoid duplicating the ret
> value check.
>
>> +
>> + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
>> + "sysctl disabled\n");
> Why is this changed from expecting 0 for the return and being the
> "sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
>
>>
>> set_tagged_addr_ctrl(0, false);
>>
>>
>> ---
>> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
>> change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
>> --
>> - Charlie
>>
> Not part of this patch, but now that I looked at
> test_tagged_addr_abi_sysctl() I see that
> ksft_test_result_skip() is duplicated.
>
> Thanks,
> drew
Your mails often end up in my junk folder, am I the only one? Any idea
what could be wrong?
Thanks,
Alex
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
@ 2024-12-05 11:15 ` Alexandre Ghiti
0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Ghiti @ 2024-12-05 11:15 UTC (permalink / raw)
To: Andrew Jones, Charlie Jenkins
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
Hi Drew,
On 05/12/2024 09:04, Andrew Jones wrote:
> On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
>> When compiling the pointer masking tests with -Wall this warning
>> is present:
>>
>> pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
>> pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
>> declared with attribute ‘warn_unused_result’ [-Wunused-result]
>> 203 | pwrite(fd, &value, 1, 0); |
>> ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
>> ignoring return value of ‘pwrite’ declared with attribute
>> ‘warn_unused_result’ [-Wunused-result]
>> 208 | pwrite(fd, &value, 1, 0);
>>
>> I came across this on riscv64-linux-gnu-gcc (Ubuntu
>> 11.4.0-1ubuntu1~22.04).
>>
>> Fix this by checking that the number of bytes written equal the expected
>> number of bytes written.
>>
>> Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
>> ---
>> Changes in v2:
>> - I had ret != 2 for testing, I changed it to be ret != 1.
>> - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
>> ---
>> tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
>> index dee41b7ee3e3..229d85ccff50 100644
>> --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
>> +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
>> @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
>> {
>> char value;
>> int fd;
>> + int ret;
>>
>> ksft_print_msg("Testing tagged address ABI sysctl\n");
>>
>> @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
>> }
>>
>> value = '1';
>> - pwrite(fd, &value, 1, 0);
>> + ret = pwrite(fd, &value, 1, 0);
>> + if (ret != 1) {
>> + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
>> + return;
>> + }
>> +
>> ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
>> "sysctl disabled\n");
>>
>> value = '0';
>> - pwrite(fd, &value, 1, 0);
>> - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
>> - "sysctl enabled\n");
>> + ret = pwrite(fd, &value, 1, 0);
>> + if (ret != 1) {
>> + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
>> + return;
>> + }
> Could make a wrapper function for pwrite() to avoid duplicating the ret
> value check.
>
>> +
>> + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
>> + "sysctl disabled\n");
> Why is this changed from expecting 0 for the return and being the
> "sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
>
>>
>> set_tagged_addr_ctrl(0, false);
>>
>>
>> ---
>> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
>> change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
>> --
>> - Charlie
>>
> Not part of this patch, but now that I looked at
> test_tagged_addr_abi_sysctl() I see that
> ksft_test_result_skip() is duplicated.
>
> Thanks,
> drew
Your mails often end up in my junk folder, am I the only one? Any idea
what could be wrong?
Thanks,
Alex
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
2024-12-05 11:15 ` Alexandre Ghiti
@ 2024-12-05 12:25 ` Andrew Jones
-1 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2024-12-05 12:25 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Charlie Jenkins, Shuah Khan, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Samuel Holland, linux-kselftest, linux-riscv,
linux-kernel, Palmer Dabbelt
On Thu, Dec 05, 2024 at 12:15:16PM +0100, Alexandre Ghiti wrote:
...
> Your mails often end up in my junk folder, am I the only one? Any idea what
> could be wrong?
>
I'm constantly pulling legitimate mails out of Spam, usually ones from
@google.com, which I heard was because spam filters, including gmail's
spam filters, assume those are made up addresses... Frequently I pull out
Samuel's messages, at least when he posts to the opensbi list. Ventana
uses gmail, so I've tried adding anybody who goes to spam, and shouldn't,
to my gmail contacts, but that doesn't help...
Anyway, I don't know why my messages go to your Spam folder either. Do
you see anything weird in their formatting or headers? Maybe try adding
me to some contact list which your spam filters hopefully use when
deciding what's spam.
Thanks,
drew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
@ 2024-12-05 12:25 ` Andrew Jones
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2024-12-05 12:25 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Charlie Jenkins, Shuah Khan, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Samuel Holland, linux-kselftest, linux-riscv,
linux-kernel, Palmer Dabbelt
On Thu, Dec 05, 2024 at 12:15:16PM +0100, Alexandre Ghiti wrote:
...
> Your mails often end up in my junk folder, am I the only one? Any idea what
> could be wrong?
>
I'm constantly pulling legitimate mails out of Spam, usually ones from
@google.com, which I heard was because spam filters, including gmail's
spam filters, assume those are made up addresses... Frequently I pull out
Samuel's messages, at least when he posts to the opensbi list. Ventana
uses gmail, so I've tried adding anybody who goes to spam, and shouldn't,
to my gmail contacts, but that doesn't help...
Anyway, I don't know why my messages go to your Spam folder either. Do
you see anything weird in their formatting or headers? Maybe try adding
me to some contact list which your spam filters hopefully use when
deciding what's spam.
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
2024-12-05 8:04 ` Andrew Jones
@ 2024-12-05 21:30 ` Charlie Jenkins
-1 siblings, 0 replies; 16+ messages in thread
From: Charlie Jenkins @ 2024-12-05 21:30 UTC (permalink / raw)
To: Andrew Jones
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
On Thu, Dec 05, 2024 at 09:04:12AM +0100, Andrew Jones wrote:
> On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
> > When compiling the pointer masking tests with -Wall this warning
> > is present:
> >
> > pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
> > pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
> > declared with attribute ‘warn_unused_result’ [-Wunused-result]
> > 203 | pwrite(fd, &value, 1, 0); |
> > ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
> > ignoring return value of ‘pwrite’ declared with attribute
> > ‘warn_unused_result’ [-Wunused-result]
> > 208 | pwrite(fd, &value, 1, 0);
> >
> > I came across this on riscv64-linux-gnu-gcc (Ubuntu
> > 11.4.0-1ubuntu1~22.04).
> >
> > Fix this by checking that the number of bytes written equal the expected
> > number of bytes written.
> >
> > Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> > Changes in v2:
> > - I had ret != 2 for testing, I changed it to be ret != 1.
> > - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
> > ---
> > tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
> > 1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > index dee41b7ee3e3..229d85ccff50 100644
> > --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
> > +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
> > {
> > char value;
> > int fd;
> > + int ret;
> >
> > ksft_print_msg("Testing tagged address ABI sysctl\n");
> >
> > @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
> > }
> >
> > value = '1';
> > - pwrite(fd, &value, 1, 0);
> > + ret = pwrite(fd, &value, 1, 0);
> > + if (ret != 1) {
> > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > + return;
> > + }
> > +
> > ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > "sysctl disabled\n");
> >
> > value = '0';
> > - pwrite(fd, &value, 1, 0);
> > - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
> > - "sysctl enabled\n");
> > + ret = pwrite(fd, &value, 1, 0);
> > + if (ret != 1) {
> > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > + return;
> > + }
>
> Could make a wrapper function for pwrite() to avoid duplicating the ret
> value check.
I'll change it to a goto statement to avoid duplicating the
ksft_test_result_fail call.
>
> > +
> > + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > + "sysctl disabled\n");
>
> Why is this changed from expecting 0 for the return and being the
> "sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
Silly copy mistake, thank you!
>
> >
> > set_tagged_addr_ctrl(0, false);
> >
> >
> > ---
> > base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> > change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
> > --
> > - Charlie
> >
>
> Not part of this patch, but now that I looked at
> test_tagged_addr_abi_sysctl() I see that
> ksft_test_result_skip() is duplicated.
Oh huh I hadn't noticed that. I'll send a patch for that I guess, easy
fix.
- Charlie
>
> Thanks,
> drew
>
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
@ 2024-12-05 21:30 ` Charlie Jenkins
0 siblings, 0 replies; 16+ messages in thread
From: Charlie Jenkins @ 2024-12-05 21:30 UTC (permalink / raw)
To: Andrew Jones
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
On Thu, Dec 05, 2024 at 09:04:12AM +0100, Andrew Jones wrote:
> On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
> > When compiling the pointer masking tests with -Wall this warning
> > is present:
> >
> > pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
> > pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
> > declared with attribute ‘warn_unused_result’ [-Wunused-result]
> > 203 | pwrite(fd, &value, 1, 0); |
> > ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
> > ignoring return value of ‘pwrite’ declared with attribute
> > ‘warn_unused_result’ [-Wunused-result]
> > 208 | pwrite(fd, &value, 1, 0);
> >
> > I came across this on riscv64-linux-gnu-gcc (Ubuntu
> > 11.4.0-1ubuntu1~22.04).
> >
> > Fix this by checking that the number of bytes written equal the expected
> > number of bytes written.
> >
> > Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> > Changes in v2:
> > - I had ret != 2 for testing, I changed it to be ret != 1.
> > - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
> > ---
> > tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
> > 1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > index dee41b7ee3e3..229d85ccff50 100644
> > --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
> > +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
> > {
> > char value;
> > int fd;
> > + int ret;
> >
> > ksft_print_msg("Testing tagged address ABI sysctl\n");
> >
> > @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
> > }
> >
> > value = '1';
> > - pwrite(fd, &value, 1, 0);
> > + ret = pwrite(fd, &value, 1, 0);
> > + if (ret != 1) {
> > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > + return;
> > + }
> > +
> > ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > "sysctl disabled\n");
> >
> > value = '0';
> > - pwrite(fd, &value, 1, 0);
> > - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
> > - "sysctl enabled\n");
> > + ret = pwrite(fd, &value, 1, 0);
> > + if (ret != 1) {
> > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > + return;
> > + }
>
> Could make a wrapper function for pwrite() to avoid duplicating the ret
> value check.
I'll change it to a goto statement to avoid duplicating the
ksft_test_result_fail call.
>
> > +
> > + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > + "sysctl disabled\n");
>
> Why is this changed from expecting 0 for the return and being the
> "sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
Silly copy mistake, thank you!
>
> >
> > set_tagged_addr_ctrl(0, false);
> >
> >
> > ---
> > base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> > change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
> > --
> > - Charlie
> >
>
> Not part of this patch, but now that I looked at
> test_tagged_addr_abi_sysctl() I see that
> ksft_test_result_skip() is duplicated.
Oh huh I hadn't noticed that. I'll send a patch for that I guess, easy
fix.
- Charlie
>
> Thanks,
> drew
>
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
2024-12-05 21:30 ` Charlie Jenkins
@ 2024-12-05 21:40 ` Charlie Jenkins
-1 siblings, 0 replies; 16+ messages in thread
From: Charlie Jenkins @ 2024-12-05 21:40 UTC (permalink / raw)
To: Andrew Jones
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
On Thu, Dec 05, 2024 at 01:30:59PM -0800, Charlie Jenkins wrote:
> On Thu, Dec 05, 2024 at 09:04:12AM +0100, Andrew Jones wrote:
> > On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
> > > When compiling the pointer masking tests with -Wall this warning
> > > is present:
> > >
> > > pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
> > > pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
> > > declared with attribute ‘warn_unused_result’ [-Wunused-result]
> > > 203 | pwrite(fd, &value, 1, 0); |
> > > ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
> > > ignoring return value of ‘pwrite’ declared with attribute
> > > ‘warn_unused_result’ [-Wunused-result]
> > > 208 | pwrite(fd, &value, 1, 0);
> > >
> > > I came across this on riscv64-linux-gnu-gcc (Ubuntu
> > > 11.4.0-1ubuntu1~22.04).
# Testing tagged address ABI sysctl
ok 57 # SKIP failed to open sysctl file
ok 58 # SKIP failed to open sysctl file> > >
> > > Fix this by checking that the number of bytes written equal the expected
> > > number of bytes written.
> > >
> > > Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
> > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > > ---
> > > Changes in v2:
> > > - I had ret != 2 for testing, I changed it to be ret != 1.
> > > - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
> > > ---
> > > tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
> > > 1 file changed, 15 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > > index dee41b7ee3e3..229d85ccff50 100644
> > > --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
> > > +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > > @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
> > > {
> > > char value;
> > > int fd;
> > > + int ret;
> > >
> > > ksft_print_msg("Testing tagged address ABI sysctl\n");
> > >
> > > @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
> > > }
> > >
> > > value = '1';
> > > - pwrite(fd, &value, 1, 0);
> > > + ret = pwrite(fd, &value, 1, 0);
> > > + if (ret != 1) {
> > > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > > + return;
> > > + }
> > > +
> > > ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > > "sysctl disabled\n");
> > >
> > > value = '0';
> > > - pwrite(fd, &value, 1, 0);
> > > - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
> > > - "sysctl enabled\n");
> > > + ret = pwrite(fd, &value, 1, 0);
> > > + if (ret != 1) {
> > > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > > + return;
> > > + }
> >
> > Could make a wrapper function for pwrite() to avoid duplicating the ret
> > value check.
>
> I'll change it to a goto statement to avoid duplicating the
> ksft_test_result_fail call.
>
> >
> > > +
> > > + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > > + "sysctl disabled\n");
> >
> > Why is this changed from expecting 0 for the return and being the
> > "sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
>
> Silly copy mistake, thank you!
>
> >
> > >
> > > set_tagged_addr_ctrl(0, false);
> > >
> > >
> > > ---
> > > base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> > > change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
> > > --
> > > - Charlie
> > >
> >
> > Not part of this patch, but now that I looked at
> > test_tagged_addr_abi_sysctl() I see that
> > ksft_test_result_skip() is duplicated.
>
> Oh huh I hadn't noticed that. I'll send a patch for that I guess, easy
> fix.
Oh wait, there are two skips because there are two ksft_test_result() in
this function. I guess I should make it so that if the first pwrite()
fails (for the sysctl disabled test) it should skip the "sysctl enabled"
test.
- Charlie
>
> - Charlie
>
> >
> > Thanks,
> > drew
> >
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test
@ 2024-12-05 21:40 ` Charlie Jenkins
0 siblings, 0 replies; 16+ messages in thread
From: Charlie Jenkins @ 2024-12-05 21:40 UTC (permalink / raw)
To: Andrew Jones
Cc: Shuah Khan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Samuel Holland, linux-kselftest, linux-riscv, linux-kernel,
Palmer Dabbelt
On Thu, Dec 05, 2024 at 01:30:59PM -0800, Charlie Jenkins wrote:
> On Thu, Dec 05, 2024 at 09:04:12AM +0100, Andrew Jones wrote:
> > On Wed, Dec 04, 2024 at 06:57:10PM -0800, Charlie Jenkins wrote:
> > > When compiling the pointer masking tests with -Wall this warning
> > > is present:
> > >
> > > pointer_masking.c: In function ‘test_tagged_addr_abi_sysctl’:
> > > pointer_masking.c:203:9: warning: ignoring return value of ‘pwrite’
> > > declared with attribute ‘warn_unused_result’ [-Wunused-result]
> > > 203 | pwrite(fd, &value, 1, 0); |
> > > ^~~~~~~~~~~~~~~~~~~~~~~~ pointer_masking.c:208:9: warning:
> > > ignoring return value of ‘pwrite’ declared with attribute
> > > ‘warn_unused_result’ [-Wunused-result]
> > > 208 | pwrite(fd, &value, 1, 0);
> > >
> > > I came across this on riscv64-linux-gnu-gcc (Ubuntu
> > > 11.4.0-1ubuntu1~22.04).
# Testing tagged address ABI sysctl
ok 57 # SKIP failed to open sysctl file
ok 58 # SKIP failed to open sysctl file> > >
> > > Fix this by checking that the number of bytes written equal the expected
> > > number of bytes written.
> > >
> > > Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
> > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > > ---
> > > Changes in v2:
> > > - I had ret != 2 for testing, I changed it to be ret != 1.
> > > - Link to v1: https://lore.kernel.org/r/20241204-fix_warnings_pointer_masking_tests-v1-1-ea1e9665ce7a@rivosinc.com
> > > ---
> > > tools/testing/selftests/riscv/abi/pointer_masking.c | 19 +++++++++++++++----
> > > 1 file changed, 15 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > > index dee41b7ee3e3..229d85ccff50 100644
> > > --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
> > > +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
> > > @@ -189,6 +189,7 @@ static void test_tagged_addr_abi_sysctl(void)
> > > {
> > > char value;
> > > int fd;
> > > + int ret;
> > >
> > > ksft_print_msg("Testing tagged address ABI sysctl\n");
> > >
> > > @@ -200,14 +201,24 @@ static void test_tagged_addr_abi_sysctl(void)
> > > }
> > >
> > > value = '1';
> > > - pwrite(fd, &value, 1, 0);
> > > + ret = pwrite(fd, &value, 1, 0);
> > > + if (ret != 1) {
> > > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > > + return;
> > > + }
> > > +
> > > ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > > "sysctl disabled\n");
> > >
> > > value = '0';
> > > - pwrite(fd, &value, 1, 0);
> > > - ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == 0,
> > > - "sysctl enabled\n");
> > > + ret = pwrite(fd, &value, 1, 0);
> > > + if (ret != 1) {
> > > + ksft_test_result_fail("Write to /proc/sys/abi/tagged_addr_disabled failed.\n");
> > > + return;
> > > + }
> >
> > Could make a wrapper function for pwrite() to avoid duplicating the ret
> > value check.
>
> I'll change it to a goto statement to avoid duplicating the
> ksft_test_result_fail call.
>
> >
> > > +
> > > + ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > > + "sysctl disabled\n");
> >
> > Why is this changed from expecting 0 for the return and being the
> > "sysctrl enabled" test? We still write '0' to tagged_addr_disabled here.
>
> Silly copy mistake, thank you!
>
> >
> > >
> > > set_tagged_addr_ctrl(0, false);
> > >
> > >
> > > ---
> > > base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> > > change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
> > > --
> > > - Charlie
> > >
> >
> > Not part of this patch, but now that I looked at
> > test_tagged_addr_abi_sysctl() I see that
> > ksft_test_result_skip() is duplicated.
>
> Oh huh I hadn't noticed that. I'll send a patch for that I guess, easy
> fix.
Oh wait, there are two skips because there are two ksft_test_result() in
this function. I guess I should make it so that if the first pwrite()
fails (for the sysctl disabled test) it should skip the "sysctl enabled"
test.
- Charlie
>
> - Charlie
>
> >
> > Thanks,
> > drew
> >
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread