Bjoern Walk <bwalk@xxxxxxxxxxxxx> [2019-03-28, 08:31AM +0100]:
> Eric Blake <eblake@xxxxxxxxxx> [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.
Sorry, no morning coffee yet, the fix is in, but I still get a GCC
warning:
CC conf/libvirt_conf_la-virdomainmomentobjlist.lo
../../src/conf/virdomainmomentobjlist.c: In function 'virDomainMomentMoveChildren':
../../src/conf/virdomainmomentobjlist.c:178:19: error: 'last' may be used uninitialized in this function [-Werror=maybe-uninitialized]
last->sibling = to->first_child;
~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[5]: *** [Makefile:9650: conf/libvirt_conf_la-virdomainmomentobjlist.lo] Error 1
This fixes it:
diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
index 92cf52dd..2e9343ff 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -165,7 +165,7 @@ virDomainMomentMoveChildren(virDomainMomentObjPtr from,
virDomainMomentObjPtr to)
{
virDomainMomentObjPtr child;
- virDomainMomentObjPtr last;
+ virDomainMomentObjPtr last = NULL;
if (!from->first_child)
return;
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
