On 08/14/2012 01:10 AM, Laine Stump wrote: > A later patch will be adding a counter that will be > incremented/decremented each time an guest interface starts/stops > using a particular network. For this to work, all types of networks > need to go through a common return sequence rather than returning > early. To setup for this, the existing cleanup: label is renamed to > error:, a new cleanup: label is added (when necessary), and early > returns are changed to goto cleanup (except the case where the > interface type != NETWORK). > --- > src/network/bridge_driver.c | 106 > ++++++++++++++++++++++---------------------- > 1 file changed, 53 insertions(+), 53 deletions(-) > > @@ -3007,7 +3006,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) > dev->dev, dev->connections); > } > ret = 0; > -cleanup: > +error: > for (ii = 0; ii < num_virt_fns; ii++)
It looks weird to have the 'ret = 0' success case fall through into the
'error:' label.
> @@ -3114,8 +3113,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
> dev->dev, dev->connections);
> }
>
> - ret = 0;
> cleanup:
> + ret = 0;
> +error:
and here, wouldn't it be better to name things 'success' for early exit
with ret = 0, and 'cleanup' for all cleanup whether on error or on
fallthrough from the ret = 0 case?
> @@ -3136,7 +3136,7 @@ int
> networkReleaseActualDevice(virDomainNetDefPtr iface)
> {
> struct network_driver *driver = driverState;
> - virNetworkObjPtr network = NULL;
> + virNetworkObjPtr network;
I did a double-take on this one...
> virNetworkDefPtr netdef;
> const char *actualDev;
> int ret = -1;
> @@ -3144,13 +3144,6 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
> if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> return 0;
>
> - if (!iface->data.network.actual ||
> - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
> - VIR_DEBUG("Nothing to release to network %s",
> iface->data.network.name);
> - ret = 0;
> - goto cleanup;
> - }
...but it is correct; the code motion here means you no longer reach the
end labels prior to network being assigned.
> @@ -3199,8 +3198,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
> dev->dev, dev->connections);
> }
>
> - ret = 0;
> cleanup:
> + ret = 0;
> +error:
Another case where this might make more sense:
success:
ret = 0;
cleanup:
The cleanups are mechanical, but I'm not sure I like the resulting naming.
--
Eric Blake [email protected] +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
