[PATCH 0/2] riscv,kvm: remove another strerrorname_np()

[PATCH 0/2] riscv,kvm: remove another strerrorname_np()

[Daniel Henrique Barboza]

Hi,

strerrorname_np() was removed last December due to problems with musl
libc builds. And then I ended up adding another instance during the 9.0
cycle (commit ).

So here I am removing the newly added instance and, this time, patching
checkpatch.pl to forbid this function from showing up again.

Michael, patch 1 is qemu-stable material.

Daniel Henrique Barboza (2):
  target/riscv/kvm: remove sneaky strerrorname_np() instance
  checkpatch.pl: forbid strerrorname_np()

 scripts/checkpatch.pl      | 4 ++++
 target/riscv/kvm/kvm-cpu.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.44.0

[PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance

[Daniel Henrique Barboza]

Commit d424db2354 excluded some strerrorname_np() instances because they
break musl libc builds. Another instance happened to slip by via commit
d4ff3da8f4.

Remove it before it causes trouble again.

Fixes: d4ff3da8f4 (target/riscv/kvm: initialize 'vlenb' via get-reg-list)
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/kvm/kvm-cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 6a6c6cae80..ee69ea9785 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1054,8 +1054,8 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
 
         ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
         if (ret != 0) {
-            error_report("Unable to read vlenb register, error code: %s",
-                         strerrorname_np(errno));
+            error_report("Unable to read vlenb register, error code: %d",
+                         errno);
             exit(EXIT_FAILURE);
         }
 
-- 
2.44.0

Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance

[Thomas Huth]

On 24/04/2024 11.46, Daniel Henrique Barboza wrote:
> Commit d424db2354 excluded some strerrorname_np() instances because they
> break musl libc builds. Another instance happened to slip by via commit
> d4ff3da8f4.
> 
> Remove it before it causes trouble again.
> 
> Fixes: d4ff3da8f4 (target/riscv/kvm: initialize 'vlenb' via get-reg-list)
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/kvm/kvm-cpu.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6a6c6cae80..ee69ea9785 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1054,8 +1054,8 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
>   
>           ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
>           if (ret != 0) {
> -            error_report("Unable to read vlenb register, error code: %s",
> -                         strerrorname_np(errno));
> +            error_report("Unable to read vlenb register, error code: %d",
> +                         errno);
>               exit(EXIT_FAILURE);
>           }
>   

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance

[Philippe Mathieu-Daudé]

On 24/4/24 11:46, Daniel Henrique Barboza wrote:
> Commit d424db2354 excluded some strerrorname_np() instances because they
> break musl libc builds. Another instance happened to slip by via commit
> d4ff3da8f4.
> 
> Remove it before it causes trouble again.
> 
> Fixes: d4ff3da8f4 (target/riscv/kvm: initialize 'vlenb' via get-reg-list)
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/kvm/kvm-cpu.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6a6c6cae80..ee69ea9785 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1054,8 +1054,8 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
>   
>           ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
>           if (ret != 0) {
> -            error_report("Unable to read vlenb register, error code: %s",
> -                         strerrorname_np(errno));
> +            error_report("Unable to read vlenb register, error code: %d",
> +                         errno);

Why not use "%s" strerror(errno)?

>               exit(EXIT_FAILURE);
>           }
>   

Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance

[Daniel Henrique Barboza]

On 4/24/24 06:55, Philippe Mathieu-Daudé wrote:
> On 24/4/24 11:46, Daniel Henrique Barboza wrote:
>> Commit d424db2354 excluded some strerrorname_np() instances because they
>> break musl libc builds. Another instance happened to slip by via commit
>> d4ff3da8f4.
>>
>> Remove it before it causes trouble again.
>>
>> Fixes: d4ff3da8f4 (target/riscv/kvm: initialize 'vlenb' via get-reg-list)
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/kvm/kvm-cpu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>> index 6a6c6cae80..ee69ea9785 100644
>> --- a/target/riscv/kvm/kvm-cpu.c
>> +++ b/target/riscv/kvm/kvm-cpu.c
>> @@ -1054,8 +1054,8 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
>>           ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
>>           if (ret != 0) {
>> -            error_report("Unable to read vlenb register, error code: %s",
>> -                         strerrorname_np(errno));
>> +            error_report("Unable to read vlenb register, error code: %d",
>> +                         errno);
> 
> Why not use "%s" strerror(errno)?

It's not exactly the same. For errno = 2 strerrorname_np() gives "ENOENT", and
sterror() gives "No such file or directory". In this particular context I think
just printing a "error code -2" is clearer because we're not mentioning files and
dirs in a KVM reg context.

But in the end I don't mind changing to strerror() if you feel strong about it. It's
fine either way.


Thanks,


Daniel

> 
>>               exit(EXIT_FAILURE);
>>           }
> 

Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance

[Philippe Mathieu-Daudé]

On 24/4/24 19:18, Daniel Henrique Barboza wrote:
> On 4/24/24 06:55, Philippe Mathieu-Daudé wrote:
>> On 24/4/24 11:46, Daniel Henrique Barboza wrote:
>>> Commit d424db2354 excluded some strerrorname_np() instances because they
>>> break musl libc builds. Another instance happened to slip by via commit
>>> d4ff3da8f4.
>>>
>>> Remove it before it causes trouble again.
>>>
>>> Fixes: d4ff3da8f4 (target/riscv/kvm: initialize 'vlenb' via 
>>> get-reg-list)
>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>> ---
>>>   target/riscv/kvm/kvm-cpu.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>>> index 6a6c6cae80..ee69ea9785 100644
>>> --- a/target/riscv/kvm/kvm-cpu.c
>>> +++ b/target/riscv/kvm/kvm-cpu.c
>>> @@ -1054,8 +1054,8 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, 
>>> KVMScratchCPU *kvmcpu,
>>>           ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
>>>           if (ret != 0) {
>>> -            error_report("Unable to read vlenb register, error code: 
>>> %s",
>>> -                         strerrorname_np(errno));
>>> +            error_report("Unable to read vlenb register, error code: 
>>> %d",
>>> +                         errno);
>>
>> Why not use "%s" strerror(errno)?
> 
> It's not exactly the same. For errno = 2 strerrorname_np() gives 
> "ENOENT", and
> sterror() gives "No such file or directory". In this particular context 
> I think
> just printing a "error code -2" is clearer because we're not mentioning 
> files and
> dirs in a KVM reg context.
> 
> But in the end I don't mind changing to strerror() if you feel strong 
> about it. It's
> fine either way.

I'm fine with "error code -2" ;)

Regards,

Phil.

[PATCH 2/2] checkpatch.pl: forbid strerrorname_np()

[Daniel Henrique Barboza]

Commit d424db2354 removed an instance of strerrorname_np() because it
was breaking building with musl libc. A recent RISC-V patch ended up
re-introducing it again by accident.

Put this function in the baddies list in checkpatch.pl to avoid this
situation again. This is what it will look like next time:

 $ ./scripts/checkpatch.pl 0001-temp-test.patch
 ERROR: use strerror() instead of strerrorname_np()
 #22: FILE: target/riscv/kvm/kvm-cpu.c:1058:
 +                         strerrorname_np(errno));

 total: 1 errors, 0 warnings, 10 lines checked

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 scripts/checkpatch.pl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7026895074..be0982246d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3056,6 +3056,10 @@ sub process {
 		}
 	}
 
+	if ($line =~ /\bstrerrorname_np\(/) {
+		ERROR("use strerror() instead of strerrorname_np()\n" . $herecurr);
+	}
+
 # check for non-portable libc calls that have portable alternatives in QEMU
 		if ($line =~ /\bffs\(/) {
 			ERROR("use ctz32() instead of ffs()\n" . $herecurr);
-- 
2.44.0

Re: [PATCH 2/2] checkpatch.pl: forbid strerrorname_np()

[Thomas Huth]

On 24/04/2024 11.47, Daniel Henrique Barboza wrote:
> Commit d424db2354 removed an instance of strerrorname_np() because it
> was breaking building with musl libc. A recent RISC-V patch ended up
> re-introducing it again by accident.
> 
> Put this function in the baddies list in checkpatch.pl to avoid this
> situation again. This is what it will look like next time:
> 
>   $ ./scripts/checkpatch.pl 0001-temp-test.patch
>   ERROR: use strerror() instead of strerrorname_np()
>   #22: FILE: target/riscv/kvm/kvm-cpu.c:1058:
>   +                         strerrorname_np(errno));
> 
>   total: 1 errors, 0 warnings, 10 lines checked
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   scripts/checkpatch.pl | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7026895074..be0982246d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3056,6 +3056,10 @@ sub process {
>   		}
>   	}
>   
> +	if ($line =~ /\bstrerrorname_np\(/) {
> +		ERROR("use strerror() instead of strerrorname_np()\n" . $herecurr);
> +	}
> +
>   # check for non-portable libc calls that have portable alternatives in QEMU
>   		if ($line =~ /\bffs\(/) {
>   			ERROR("use ctz32() instead of ffs()\n" . $herecurr);

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 2/2] checkpatch.pl: forbid strerrorname_np()

[Philippe Mathieu-Daudé]

On 24/4/24 11:47, Daniel Henrique Barboza wrote:
> Commit d424db2354 removed an instance of strerrorname_np() because it
> was breaking building with musl libc. A recent RISC-V patch ended up
> re-introducing it again by accident.
> 
> Put this function in the baddies list in checkpatch.pl to avoid this
> situation again. This is what it will look like next time:
> 
>   $ ./scripts/checkpatch.pl 0001-temp-test.patch
>   ERROR: use strerror() instead of strerrorname_np()
>   #22: FILE: target/riscv/kvm/kvm-cpu.c:1058:
>   +                         strerrorname_np(errno));
> 
>   total: 1 errors, 0 warnings, 10 lines checked
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   scripts/checkpatch.pl | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7026895074..be0982246d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3056,6 +3056,10 @@ sub process {
>   		}
>   	}
>   
> +	if ($line =~ /\bstrerrorname_np\(/) {
> +		ERROR("use strerror() instead of strerrorname_np()\n" . $herecurr);
> +	}
> +
>   # check for non-portable libc calls that have portable alternatives in QEMU

This should be added here in the "non-portable libc calls" section.

>   		if ($line =~ /\bffs\(/) {
>   			ERROR("use ctz32() instead of ffs()\n" . $herecurr);

Re: [PATCH 0/2] riscv,kvm: remove another strerrorname_np()

[Philippe Mathieu-Daudé]

On 24/4/24 11:46, Daniel Henrique Barboza wrote:

> Daniel Henrique Barboza (2):
>    target/riscv/kvm: remove sneaky strerrorname_np() instance
>    checkpatch.pl: forbid strerrorname_np()

Modulo my nitpicking comments, series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>