* [PATCH] KVM: Fix the warning by the min()
@ 2021-11-16 12:10 zhaoxiao
2021-11-16 15:56 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: zhaoxiao @ 2021-11-16 12:10 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-kernel, zhaoxiao
Fix following coccicheck warning:
virt/kvm/kvm_main.c:4995:10-11: WARNING opportunity for min()
virt/kvm/kvm_main.c:4924:10-11: WARNING opportunity for min()
Signed-off-by: zhaoxiao <zhaoxiao@uniontech.com>
---
virt/kvm/kvm_main.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d31724500501..bd646c64722d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4910,7 +4910,6 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
{
struct kvm_io_bus *bus;
struct kvm_io_range range;
- int r;
range = (struct kvm_io_range) {
.addr = addr,
@@ -4920,8 +4919,8 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
if (!bus)
return -ENOMEM;
- r = __kvm_io_bus_write(vcpu, bus, &range, val);
- return r < 0 ? r : 0;
+
+ return min(__kvm_io_bus_write(vcpu, bus, &range, val), 0);
}
EXPORT_SYMBOL_GPL(kvm_io_bus_write);
@@ -4981,7 +4980,6 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
{
struct kvm_io_bus *bus;
struct kvm_io_range range;
- int r;
range = (struct kvm_io_range) {
.addr = addr,
@@ -4991,8 +4989,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
if (!bus)
return -ENOMEM;
- r = __kvm_io_bus_read(vcpu, bus, &range, val);
- return r < 0 ? r : 0;
+
+ return min(__kvm_io_bus_read(vcpu, bus, &range, val), 0);
}
/* Caller must hold slots_lock. */
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] KVM: Fix the warning by the min()
2021-11-16 12:10 [PATCH] KVM: Fix the warning by the min() zhaoxiao
@ 2021-11-16 15:56 ` Paolo Bonzini
2021-11-17 22:19 ` Sean Christopherson
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2021-11-16 15:56 UTC (permalink / raw)
To: zhaoxiao; +Cc: kvm, linux-kernel
On 11/16/21 13:10, zhaoxiao wrote:
> Fix following coccicheck warning:
> virt/kvm/kvm_main.c:4995:10-11: WARNING opportunity for min()
> virt/kvm/kvm_main.c:4924:10-11: WARNING opportunity for min()
>
> Signed-off-by: zhaoxiao <zhaoxiao@uniontech.com>
No, this is unreadable for two reasons:
First, the code in parentheses for min(func(), 0) is very long. Usually
min has very short arguments. By the time you have reached the closing
parentheses, you have completely forgotten that there was a min() at the
beginning. So perhaps one possibility could be
return min(r, 0);
However, the second reason is that "r < 0" is a very common way to
express "if there was an error". In this case that would be
r = __kvm_io_bus_write(vcpu, bus, &range, val);
if (r < 0) // "if __kvm_io_bus_write failed"
return r;
return 0;
That "r < 0" is what will catch the attention of the person that is
reading the code, no matter if it is an "if" or (as in the existing
code), a "return". Using "min" removes the idiom that tells the person
"this is checking for errors".
Thanks,
Paolo
> ---
> virt/kvm/kvm_main.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index d31724500501..bd646c64722d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4910,7 +4910,6 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
> {
> struct kvm_io_bus *bus;
> struct kvm_io_range range;
> - int r;
>
> range = (struct kvm_io_range) {
> .addr = addr,
> @@ -4920,8 +4919,8 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
> bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
> if (!bus)
> return -ENOMEM;
> - r = __kvm_io_bus_write(vcpu, bus, &range, val);
> - return r < 0 ? r : 0;
> +
> + return min(__kvm_io_bus_write(vcpu, bus, &range, val), 0);
> }
> EXPORT_SYMBOL_GPL(kvm_io_bus_write);
>
> @@ -4981,7 +4980,6 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
> {
> struct kvm_io_bus *bus;
> struct kvm_io_range range;
> - int r;
>
> range = (struct kvm_io_range) {
> .addr = addr,
> @@ -4991,8 +4989,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
> bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
> if (!bus)
> return -ENOMEM;
> - r = __kvm_io_bus_read(vcpu, bus, &range, val);
> - return r < 0 ? r : 0;
> +
> + return min(__kvm_io_bus_read(vcpu, bus, &range, val), 0);
> }
>
> /* Caller must hold slots_lock. */
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] KVM: Fix the warning by the min()
2021-11-16 15:56 ` Paolo Bonzini
@ 2021-11-17 22:19 ` Sean Christopherson
0 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2021-11-17 22:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: zhaoxiao, kvm, linux-kernel
On Tue, Nov 16, 2021, Paolo Bonzini wrote:
> However, the second reason is that "r < 0" is a very common way to express
> "if there was an error". In this case that would be
>
> r = __kvm_io_bus_write(vcpu, bus, &range, val);
> if (r < 0) // "if __kvm_io_bus_write failed"
> return r;
>
> return 0;
>
> That "r < 0" is what will catch the attention of the person that is reading
> the code, no matter if it is an "if" or (as in the existing code), a
> "return". Using "min" removes the idiom that tells the person "this is
> checking for errors".
+1, there is zero chance that I would realize "min(r, 0)" is "handling" errors.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] KVM: Fix the warning by the min()
@ 2021-11-17 0:55 zhaoxiao
0 siblings, 0 replies; 4+ messages in thread
From: zhaoxiao @ 2021-11-17 0:55 UTC (permalink / raw)
To: kvm; +Cc: zhaoxiao
Fix following coccicheck warning:
virt/kvm/kvm_main.c:4995:10-11: WARNING opportunity for min()
virt/kvm/kvm_main.c:4924:10-11: WARNING opportunity for min()
Signed-off-by: zhaoxiao <zhaoxiao@uniontech.com>
---
virt/kvm/kvm_main.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d31724500501..bd646c64722d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4910,7 +4910,6 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
{
struct kvm_io_bus *bus;
struct kvm_io_range range;
- int r;
range = (struct kvm_io_range) {
.addr = addr,
@@ -4920,8 +4919,8 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
if (!bus)
return -ENOMEM;
- r = __kvm_io_bus_write(vcpu, bus, &range, val);
- return r < 0 ? r : 0;
+
+ return min(__kvm_io_bus_write(vcpu, bus, &range, val), 0);
}
EXPORT_SYMBOL_GPL(kvm_io_bus_write);
@@ -4981,7 +4980,6 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
{
struct kvm_io_bus *bus;
struct kvm_io_range range;
- int r;
range = (struct kvm_io_range) {
.addr = addr,
@@ -4991,8 +4989,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
if (!bus)
return -ENOMEM;
- r = __kvm_io_bus_read(vcpu, bus, &range, val);
- return r < 0 ? r : 0;
+
+ return min(__kvm_io_bus_read(vcpu, bus, &range, val), 0);
}
/* Caller must hold slots_lock. */
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-17 22:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 12:10 [PATCH] KVM: Fix the warning by the min() zhaoxiao
2021-11-16 15:56 ` Paolo Bonzini
2021-11-17 22:19 ` Sean Christopherson
2021-11-17 0:55 zhaoxiao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.