Re: [libvirt] [PATCH 12/33] Fix error reporting in port profile parsing/formatting APIs
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(a)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 :|