[libvirt] [PATCH] qemu: Fix double free when live-attaching shmem

[Martin Kletzander]

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 at 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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161109/d39198fc/attachment-0001.sig>