On Wed, Nov 09, 2011 at 02:58:32AM -0500, Laine Stump wrote: > After applying this patch, make fails with: > > CC libvirt_util_la-network.lo > cc1: warnings being treated as errors > util/network.c: In function 'virNetDevVPortProfileParse': > util/network.c:712:23: error: assignment makes pointer from integer > without a cast > util/network.c:712:69: error: ordered comparison of pointer with > integer zero [-Wextra] > > > On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: > >From: "Daniel P. Berrange"<berrange@xxxxxxxxxx> > > > >The virtual port profile parsing/formatting APIs do not > >correctly handle unknown profile type strings/numbers. > >They behave as a no-op, instead of raising an error > > Actually I've noticed a *lot* of the *Format functions ignore the > possibility of bad values in the vir*Def objects (e.g. invalid enum > values), and have debated with myself about whether to ignore or > report invalid data. If we work from the assumption that the vir*Def objects can only be populated as a result of parsing an XML document, then we can assume the enum values in the vir*Def are all valid & within range. If we allow for the possibility that code can populate the vir*Def objects programmatically, then if it exclusively uses the enum constants we will again be safe. Only if we somehow populate vir*Def objects directly using int values, instead of constants or the formal string<->int conversion APIs, do we need to consider invalid values. This particular code *was* assuming the worst and explicitly trying to handle bogus values, but doing so in a really lame manner. IMHO it is not neccessary to handle invalid enum values in the formatting code, but if you do decide to handle them, then you *must* report the errors correctly and not just pretend they don't exist after you detected them. The latter is what this patch is fixing. > >@@ -700,13 +699,19 @@ virNetDevVPortProfileParse(xmlNodePtr node, > > > > if (VIR_ALLOC(virtPort)< 0) { > > virReportOOMError(); > >- return -1; > >+ return NULL; > > } > > > > virtPortType = virXMLPropString(node, "type"); > > if (!virtPortType) { > > virSocketError(VIR_ERR_XML_ERROR, "%s", > >- _("missing virtualportprofile type")); > >+ _("missing virtualportprofile type")); > >+ goto error; > >+ } > > The following should be "virtPort->virPortType = ....": Opps, yes. Fixed in a later patch, will pull the fix back here. > Anyway, ACK with the compile problem fixed. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list