[libvirt] [PATCH 12/33] Fix error reporting in port profile parsing/formatting APIs

[Daniel P. Berrange]

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 at 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 :|