[libvirt] [PATCH 07/16] snapshot: Add accessors for updating snapshot list relations

[Bjoern Walk]

Eric Blake <eblake at redhat.com> [2019-03-21, 01:03PM -0500]:
> On 3/20/19 4:28 PM, John Ferlan wrote:
> > 
> > 
> > On 3/20/19 1:40 AM, Eric Blake wrote:
> >> Rather than allowing a leaky abstraction where multiple drivers have
> >> to open-code operations that update the relations in a
> >> virDomainSnapshotObjList, it is better to add accessor functions so
> >> that updates to relations are maintained closer to the internals.  The
> >> goal here is to avoid access to, nchildren, first_child, or sibling
> >> outside of the lowest level functions, making it easier to refactor
> >> later on.  While many of the conversions are fairly straightforward,
> >> the MoveChildren refactoring can be a bit interesting to follow.
> > 
> > Understatement ;-)  Some black magic occurs
> > 
> > The qemuDomainSnapshotReparentChildren "snap->parent = rep->parent" gets
> > replaced by the new for loop... Tough to see without watching really
> > closely.
> > 
> 
> >> +/* Take all children of @from and convert them into children of @to. */
> >> +void
> >> +virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from,
> >> +                              virDomainSnapshotObjPtr to)
> >> +{
> >> +    virDomainSnapshotObjPtr child;
> >> +    virDomainSnapshotObjPtr last;
> >> +
> >> +    for (child = from->first_child; child; child = child->sibling) {
> >> +        child->parent = to;
> >> +        if (!child->sibling)
> >> +            last = child;
> >> +    }
> >> +    to->nchildren += from->nchildren;
> >> +    last->sibling = to->first_child;
> > 
> > Silly Coverity compiler gets quite confused thinking that @last couldn't
> > be set while not considering the above loop couldn't end without it
> > unless of course from->first_child == NULL I suppose, which would be a
> > different issue. Still if before the for loop we check "if
> > (!from->first_child) return;", then coverity is happy.
> 
> Good find from Coverity. If there are no children to move, I do need the
> early exit, so I'll squash that in.

Did you forget this? Function is pushed in this (broken?) version and I
get a warning/error on GCC 8.0.1.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 902 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190328/2a710ab7/attachment-0001.sig>