* [PATCH 0/2] riscv,kvm: remove another strerrorname_np()
@ 2024-04-24 9:46 Daniel Henrique Barboza
2024-04-24 9:46 ` [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance Daniel Henrique Barboza
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Daniel Henrique Barboza @ 2024-04-24 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, philmd, mjt, ajones,
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance
2024-04-24 9:46 [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Daniel Henrique Barboza
@ 2024-04-24 9:46 ` Daniel Henrique Barboza
2024-04-24 9:51 ` Thomas Huth
2024-04-24 9:55 ` Philippe Mathieu-Daudé
2024-04-24 9:47 ` [PATCH 2/2] checkpatch.pl: forbid strerrorname_np() Daniel Henrique Barboza
2024-04-24 9:58 ` [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Philippe Mathieu-Daudé
2 siblings, 2 replies; 10+ messages in thread
From: Daniel Henrique Barboza @ 2024-04-24 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, philmd, mjt, ajones,
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, ®);
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] checkpatch.pl: forbid strerrorname_np()
2024-04-24 9:46 [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Daniel Henrique Barboza
2024-04-24 9:46 ` [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance Daniel Henrique Barboza
@ 2024-04-24 9:47 ` Daniel Henrique Barboza
2024-04-24 9:52 ` Thomas Huth
2024-04-24 9:57 ` Philippe Mathieu-Daudé
2024-04-24 9:58 ` [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Philippe Mathieu-Daudé
2 siblings, 2 replies; 10+ messages in thread
From: Daniel Henrique Barboza @ 2024-04-24 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, philmd, mjt, ajones,
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance
2024-04-24 9:46 ` [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance Daniel Henrique Barboza
@ 2024-04-24 9:51 ` Thomas Huth
2024-04-24 9:55 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2024-04-24 9:51 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, alex.bennee, philmd, mjt, ajones, QEMU Trivial
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, ®);
> 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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] checkpatch.pl: forbid strerrorname_np()
2024-04-24 9:47 ` [PATCH 2/2] checkpatch.pl: forbid strerrorname_np() Daniel Henrique Barboza
@ 2024-04-24 9:52 ` Thomas Huth
2024-04-24 9:57 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2024-04-24 9:52 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, alex.bennee, philmd, mjt, ajones
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance
2024-04-24 9:46 ` [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance Daniel Henrique Barboza
2024-04-24 9:51 ` Thomas Huth
@ 2024-04-24 9:55 ` Philippe Mathieu-Daudé
2024-04-24 17:18 ` Daniel Henrique Barboza
1 sibling, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-24 9:55 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, mjt, ajones
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, ®);
> 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);
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] checkpatch.pl: forbid strerrorname_np()
2024-04-24 9:47 ` [PATCH 2/2] checkpatch.pl: forbid strerrorname_np() Daniel Henrique Barboza
2024-04-24 9:52 ` Thomas Huth
@ 2024-04-24 9:57 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-24 9:57 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, mjt, ajones
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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] riscv,kvm: remove another strerrorname_np()
2024-04-24 9:46 [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Daniel Henrique Barboza
2024-04-24 9:46 ` [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance Daniel Henrique Barboza
2024-04-24 9:47 ` [PATCH 2/2] checkpatch.pl: forbid strerrorname_np() Daniel Henrique Barboza
@ 2024-04-24 9:58 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-24 9:58 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, mjt, ajones
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance
2024-04-24 9:55 ` Philippe Mathieu-Daudé
@ 2024-04-24 17:18 ` Daniel Henrique Barboza
2024-04-24 17:54 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Henrique Barboza @ 2024-04-24 17:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, mjt, ajones
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, ®);
>> 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);
>> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance
2024-04-24 17:18 ` Daniel Henrique Barboza
@ 2024-04-24 17:54 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-24 17:54 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, thuth, alex.bennee, mjt, ajones
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, ®);
>>> 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-24 17:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-24 9:46 [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Daniel Henrique Barboza
2024-04-24 9:46 ` [PATCH 1/2] target/riscv/kvm: remove sneaky strerrorname_np() instance Daniel Henrique Barboza
2024-04-24 9:51 ` Thomas Huth
2024-04-24 9:55 ` Philippe Mathieu-Daudé
2024-04-24 17:18 ` Daniel Henrique Barboza
2024-04-24 17:54 ` Philippe Mathieu-Daudé
2024-04-24 9:47 ` [PATCH 2/2] checkpatch.pl: forbid strerrorname_np() Daniel Henrique Barboza
2024-04-24 9:52 ` Thomas Huth
2024-04-24 9:57 ` Philippe Mathieu-Daudé
2024-04-24 9:58 ` [PATCH 0/2] riscv,kvm: remove another strerrorname_np() Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).