On Wed, Nov 09, 2016 at 05:55:34PM +0100, Pavel Hrdina wrote:
On Wed, Nov 09, 2016 at 05:43:49PM +0100, Martin Kletzander wrote:
> Function qemuDomainAttachShmemDevice() steals the device data if the
> hotplug was successful, but the condition checked for unsuccessful
> execution otherwise.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 38c841420e32..a82e58b29f29 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7615,7 +7615,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> case VIR_DOMAIN_DEVICE_SHMEM:
> ret = qemuDomainAttachShmemDevice(driver, vm,
> dev->data.shmem);
> - if (ret < 0) {
> + if (!ret) {
In this case I would probably use ret == 0. We are not consistent at all, for
example in this function there is more occurrences of !ret, but in case of *int*
I think it's better to check for specific value, not like in case of *pointer*
or *bool* where !ret makes more sense.
ACK
I sent the patch without --notes which made part of the explanation
disappear. But since you "asked for it" :) here goes:
Notes:
This is actually how the code looked like before the last review.
At least now I know that I should stand up for myself. All the
other relevant branches use the same syntax for this logic anyway.
I could also used -W to show the whole function.
Pavel
> alias = dev->data.shmem->info.alias;
> dev->data.shmem = NULL;
> }
> --
> 2.10.2
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
>
https://www.redhat.com/mailman/listinfo/libvir-list