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@redhat.com>
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 :|