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"<berra...@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.
* src/util/network.c, src/util/network.h: Fix error
handling of port profile APIs
* src/conf/domain_conf.c, src/conf/network_conf.c: Update
for API changes
---
src/conf/domain_conf.c | 16 +++++++-----
src/conf/network_conf.c | 20 +++++++-------
src/util/network.c | 61 ++++++++++++++++++++++++-----------------------
src/util/network.h | 8 +++---
4 files changed, 54 insertions(+), 51 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 023eb9f..5e38bee 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -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 = ....":
+
+ if ((virtPortType = virNetDevVPortTypeFromString(virtPortType))<= 0) {
+ virSocketError(VIR_ERR_XML_ERROR,
+ _("unknown virtualportprofile type %s"), virtPortType);
goto error;
}
@@ -879,23 +877,20 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a,
virNetDevVPortProfilePtr
return true;
}
-void
+
+int
virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
virBufferPtr buf)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
if (!virtPort || virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)
- return;
+ return 0;
virBufferAsprintf(buf, "<virtualport type='%s'>\n",
virNetDevVPortTypeToString(virtPort->virtPortType));
switch (virtPort->virtPortType) {
- case VIR_NETDEV_VPORT_PROFILE_NONE:
- case VIR_NETDEV_VPORT_PROFILE_LAST:
- break;
-
case VIR_NETDEV_VPORT_PROFILE_8021QBG:
virUUIDFormat(virtPort->u.virtPort8021Qbg.instanceID,
uuidstr);
@@ -913,9 +908,15 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr
virtPort,
"<parameters profileid='%s'/>\n",
virtPort->u.virtPort8021Qbh.profileID);
break;
+
+ default:
+ virSocketError(VIR_ERR_XML_ERROR,
+ _("unexpected virtualport type %d"),
virtPort->virtPortType);
+ return -1;
}
A bit of digression:
In the example here, you're doing something with the enum value aside
from just converting it to a string, so there's a ready place to put in
the check and return failure if it's out of bounds/unknown, but there
are many many places where an XXXTypeToString() macro is used directly
as an argument in a printf with no check for validity. On one hand, this
is a bit sloppy if we consider that the [whatever]Def objects may
contain unverified data (downright dangerous if you think about
platforms that don't protect against NULL dereferences in printf!); on
the other hand, if we changed every occurrence of that to get the string
value into a temp and check for non-NULL before using it in a printf, it
would add significant clutter to the code (domain_conf.c seems to be the
biggest offender here). Even in this case, we're calling
virBufferAsprintf with the unqualified return from
virNetDevVPortTypeToString before we eventually vet it in the following
switch statement.
So do we consider objects sent to the Format functions to contain
qualified data or not? If not, there's quite a large patch waiting in
the wings :-)
Anyway, ACK with the compile problem fixed.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
