Re: [PATCH] snapashot: Improve logic of virDomainMomentMoveChildren

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/28/19 10:18 AM, Ján Tomko wrote:
> s/snapashot/snapshot/
> 

I've been making that one a lot; will fix.

> On Thu, Mar 28, 2019 at 09:05:31AM -0500, Eric Blake wrote:
>> Even though Coverity can prove that 'last' is always set if the prior
>> loop executed, gcc 8.0.1 cannot:
>>

>> +++ b/src/conf/virdomainmomentobjlist.c
>> @@ -164,18 +164,17 @@ void
>> virDomainMomentMoveChildren(virDomainMomentObjPtr from,
>>                             virDomainMomentObjPtr to)
>> {
>> -    virDomainMomentObjPtr child;
>> -    virDomainMomentObjPtr last;
>> +    virDomainMomentObjPtr child = from->first_child;
>>
>> -    if (!from->first_child)
>> -        return;
> 
> From the code-change point-of view, by removing this condition,
> 
>> -    for (child = from->first_child; child; child = child->sibling) {
>> +    while (child) {
>>         child->parent = to;
>> -        if (!child->sibling)
>> -            last = child;
>> +        if (!child->sibling) {
>> +            child->sibling = to->first_child;
>> +            break;
>> +        }
>> +        child = child->sibling;
>>     }
>>     to->nchildren += from->nchildren;
>> -    last->sibling = to->first_child;
>>     to->first_child = from->first_child;
> 
> this possibly erases 'to->first_child' if 'from' does not have any.

Oh, good point. I'll keep the early exit for (!from->nchildren) then,

> But the callers are reasonable and only call this if (from->nchildren)

because I'm not certain that all future callers will be reasonable.

> 
>>     from->nchildren = 0;
>>     from->first_child = NULL;
> 
> Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

I'll go ahead and push this one with those fixes.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux