Eric Blake <eblake@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.