Re: [libvirt] [PATCH 05/10] network: Bridge - use IPv4 and IPv6 active DHCP stanza pointers

On 02/27/2013 09:57 PM, TJ wrote: > From: TJ <linux(a)iam.tj&gt; > > Rather than iterate through virNetworkIPDef arrays multiple times > use the new virNetworkDef ipv4_dhcp and ipv6_dhcp active stanza > pointers. > > Signed-off-by: TJ <linux(a)iam.tj&gt; > --- > src/network/bridge_driver.c | 63 +++++++++++---------------------------------- > 1 file changed, 15 insertions(+), 48 deletions(-) > > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 0932cf8..8410c93 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -810,24 +810,16 @@ networkDnsmasqConfContents(virNetworkObjPtr network, > } > } > > - /* Find the first dhcp for both IPv4 and IPv6 */ > - for (ii = 0, ipv4def = NULL, ipv6def = NULL, ipv6SLAAC = false; Your refactoring has eliminated the initialization of ipv6SLAAC to false. > - (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); > - ii++) { > - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) { > - if (ipdef->nranges || ipdef->nhosts) { > - if (ipv4def) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("For IPv4, multiple DHCP definitions " > - "cannot be specified.")); > - goto cleanup; > - } else { > - ipv4def = ipdef; > - } > - } > - } > - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { > - if (ipdef->nranges || ipdef->nhosts) { I'm starting to warm to the idea of a convenience pointer, but still dislike the fact that it could get out of sync due to careless programming. How about a convenience *function* instead. It would be just as inefficient, but the code would look cleaner (and the inefficiency isn't a big problem anyway, since it's very rarely done, and takes a miniscule amount of time anyway).