* [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio()
@ 2014-01-29 13:16 Dan Carpenter
2014-01-29 13:57 ` walter harms
2014-01-30 10:57 ` Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2014-01-29 13:16 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Paolo Bonzini, kvm, kernel-janitors
If kvm_io_bus_register_dev() fails then it returns success but it should
return an error code.
I also did a little cleanup like removing an impossible NULL test.
Fixes: 2b3c246a682c ('KVM: Make coalesced mmio use a device per zone')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 88b2fe3ddf42..00d86427af0f 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -154,17 +154,13 @@ int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,
list_add_tail(&dev->list, &kvm->coalesced_zones);
mutex_unlock(&kvm->slots_lock);
- return ret;
+ return 0;
out_free_dev:
mutex_unlock(&kvm->slots_lock);
-
kfree(dev);
- if (dev == NULL)
- return -ENXIO;
-
- return 0;
+ return ret;
}
int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio()
2014-01-29 13:16 [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio() Dan Carpenter
@ 2014-01-29 13:57 ` walter harms
2014-01-29 14:29 ` Dan Carpenter
2014-01-30 10:57 ` Paolo Bonzini
1 sibling, 1 reply; 4+ messages in thread
From: walter harms @ 2014-01-29 13:57 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Gleb Natapov, Paolo Bonzini, kvm, kernel-janitors
Am 29.01.2014 14:16, schrieb Dan Carpenter:
> If kvm_io_bus_register_dev() fails then it returns success but it should
> return an error code.
>
> I also did a little cleanup like removing an impossible NULL test.
>
> Fixes: 2b3c246a682c ('KVM: Make coalesced mmio use a device per zone')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
> index 88b2fe3ddf42..00d86427af0f 100644
> --- a/virt/kvm/coalesced_mmio.c
> +++ b/virt/kvm/coalesced_mmio.c
> @@ -154,17 +154,13 @@ int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,
> list_add_tail(&dev->list, &kvm->coalesced_zones);
> mutex_unlock(&kvm->slots_lock);
>
> - return ret;
> + return 0;
>
> out_free_dev:
> mutex_unlock(&kvm->slots_lock);
> -
> kfree(dev);
>
> - if (dev == NULL)
> - return -ENXIO;
> -
> - return 0;
> + return ret;
> }
>
> int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
I did not see the rest of the code, be careful ..
You can get rid of the return 0 by protecting the free(dev) like
if (ret != 0)
kfree(dev);
that will make the
mutex_unlock(&kvm->slots_lock);
return 0;
obsolet. (simply set ret=0 if needed).
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio()
2014-01-29 13:57 ` walter harms
@ 2014-01-29 14:29 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2014-01-29 14:29 UTC (permalink / raw)
To: walter harms; +Cc: Gleb Natapov, Paolo Bonzini, kvm, kernel-janitors
On Wed, Jan 29, 2014 at 02:57:21PM +0100, walter harms wrote:
>
>
> Am 29.01.2014 14:16, schrieb Dan Carpenter:
> > If kvm_io_bus_register_dev() fails then it returns success but it should
> > return an error code.
> >
> > I also did a little cleanup like removing an impossible NULL test.
> >
> > Fixes: 2b3c246a682c ('KVM: Make coalesced mmio use a device per zone')
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
> > index 88b2fe3ddf42..00d86427af0f 100644
> > --- a/virt/kvm/coalesced_mmio.c
> > +++ b/virt/kvm/coalesced_mmio.c
> > @@ -154,17 +154,13 @@ int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,
> > list_add_tail(&dev->list, &kvm->coalesced_zones);
> > mutex_unlock(&kvm->slots_lock);
> >
> > - return ret;
> > + return 0;
> >
> > out_free_dev:
> > mutex_unlock(&kvm->slots_lock);
> > -
> > kfree(dev);
> >
> > - if (dev == NULL)
> > - return -ENXIO;
> > -
> > - return 0;
> > + return ret;
> > }
> >
> > int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
>
>
> I did not see the rest of the code, be careful ..
>
> You can get rid of the return 0 by protecting the free(dev) like
>
> if (ret != 0)
> kfree(dev);
>
> that will make the
> mutex_unlock(&kvm->slots_lock);
> return 0;
>
> obsolet. (simply set ret=0 if needed).
It's better to have a clean separation of success and error path. The
success path should "return 0;" and not "return ret;"
[ I spent thirty minutes here ranting about the right way to do error
paths but then I deleted it. :P ].
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio()
2014-01-29 13:16 [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio() Dan Carpenter
2014-01-29 13:57 ` walter harms
@ 2014-01-30 10:57 ` Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-01-30 10:57 UTC (permalink / raw)
To: Dan Carpenter, Gleb Natapov; +Cc: kvm, kernel-janitors, stable
Il 29/01/2014 14:16, Dan Carpenter ha scritto:
> If kvm_io_bus_register_dev() fails then it returns success but it should
> return an error code.
>
> I also did a little cleanup like removing an impossible NULL test.
>
> Fixes: 2b3c246a682c ('KVM: Make coalesced mmio use a device per zone')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
> index 88b2fe3ddf42..00d86427af0f 100644
> --- a/virt/kvm/coalesced_mmio.c
> +++ b/virt/kvm/coalesced_mmio.c
> @@ -154,17 +154,13 @@ int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,
> list_add_tail(&dev->list, &kvm->coalesced_zones);
> mutex_unlock(&kvm->slots_lock);
>
> - return ret;
> + return 0;
>
> out_free_dev:
> mutex_unlock(&kvm->slots_lock);
> -
> kfree(dev);
>
> - if (dev == NULL)
> - return -ENXIO;
> -
> - return 0;
> + return ret;
> }
>
> int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
>
Applying to kvm/master, and adding a "Cc: stable@vger.kernel.org" while
at it.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-30 10:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-29 13:16 [patch] KVM: return an error code in kvm_vm_ioctl_register_coalesced_mmio() Dan Carpenter
2014-01-29 13:57 ` walter harms
2014-01-29 14:29 ` Dan Carpenter
2014-01-30 10:57 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox