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

[Bjoern Walk]

Bjoern Walk <bwalk at linux.ibm.com> [2019-03-28, 08:31AM +0100]:
> 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.

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 at redhat.com
> 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
-------------- 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/728443da/attachment-0001.sig>