On 01/15/2013 01:35 PM, John Ferlan wrote:
Remove extraneous check for 'netdef' when dereferencing for vlan.nTags. Prior code would already check if netdef was NULL.
Coverity complained about a path where the 'vlan' was potentially valid, but a prior checks may not have allocated 'iface->data.network.actual', so like other paths it needs to be allocated on the fly. --- src/network/bridge_driver.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f1be954..e2b8d06 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4005,11 +4005,21 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) vlan = &iface->vlan; else if (portgroup && portgroup->vlan.nTags > 0) vlan = &portgroup->vlan; - else if (netdef && netdef->vlan.nTags > 0) + else if (netdef->vlan.nTags > 0) vlan = &netdef->vlan;
Correct. If netdef was NULL, that would mean (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK), and in that case we would have skipped over this code down to validate:
- if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) - goto error; + if (vlan) { + /* data.network.actual may be NULL here when netdef->foward.type is + * VIR_NETWORK_FORWARD_{NONE|NAT|ROUTE} + */ + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { + virReportOOMError(); + goto error; + }
Hmm. This ends up giving us an actual with no type set, which tells me we should have allocated this prior to now, which points out that I added this bit of code in the wrong place. I'll send an alternate patch momentarily.
+ if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) + goto error; + }
validate: /* make sure that everything now specified for the device is