[PATCH v2] riscv: selftests: Fix warnings pointer masking test

[PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Charlie Jenkins]

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;
+	}
+
+	ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
+			 "sysctl disabled\n");
 
 	set_tagged_addr_ctrl(0, false);
 

---
base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429
-- 
- Charlie

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Andrew Jones]

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

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Alexandre Ghiti]

Hi Charlie,

On 05/12/2024 03:57, 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;
> +	}
> +
> +	ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> +			 "sysctl disabled\n");

Why did you change the test from 0 to -EINVAL here?

Thanks,

Alex


>   
>   	set_tagged_addr_ctrl(0, false);
>   
>
> ---
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Alexandre Ghiti]

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

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Andrew Jones]

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

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Charlie Jenkins]

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

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Charlie Jenkins]

On Thu, Dec 05, 2024 at 09:11:46AM +0100, Alexandre Ghiti wrote:
> Hi Charlie,
> 
> On 05/12/2024 03:57, 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;
> > +	}
> > +
> > +	ksft_test_result(set_tagged_addr_ctrl(min_pmlen, true) == -EINVAL,
> > +			 "sysctl disabled\n");
> 
> Why did you change the test from 0 to -EINVAL here?

Thank you for pointing that out, copy-paste issue, I will revert that change!

- Charlie

> 
> Thanks,
> 
> Alex
> 
> 
> >   	set_tagged_addr_ctrl(0, false);
> > 
> > ---
> > base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> > change-id: 20241204-fix_warnings_pointer_masking_tests-3860e4f35429

Re: [PATCH v2] riscv: selftests: Fix warnings pointer masking test

[Charlie Jenkins]

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