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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list