Re: [libvirt] [PATCH 02/10] conf: Network - add ability to read/write XML DHCP state

On 02/27/2013 07:57 PM, TJ wrote: > From: TJ <linux(a)iam.tj&gt; > > Maintain backwards XML compatibility by assuming existing default values > and only adding the additional XML properties if settings are not > default. > > Signed-off-by: TJ <linux(a)iam.tj&gt; > --- > src/conf/network_conf.c | 28 ++++++++++++++++++++++++---- > 1 file changed, 24 insertions(+), 4 deletions(-) > > + def->dhcp_enabled = true; > + if ((tmp = virXMLPropString(node, "enabled"))) { > + def->dhcp_enabled = strncmp("no", tmp, 2) == 0 ? false : def->dhcp_enabled; Yuck. This lets a user pass in trailing garbage. Use STREQ, not strncmp. For that matter, assigning def->dhcp_enabled to itself looks odd. I'd probably write: def->dhcp_enabled = true; if ((tmp = virXMLPropString(node, "enabled"))) { if (STREQ(tmp, "no")) def->dhcp_enabled = false; VIR_FREE(tmp); so that it doesn't look so screwy.

> + !def->dhcp_enabled || def->dhcp_relay) { > int ii; > - virBufferAddLit(buf, "<dhcp>\n"); > + virBufferAddLit(buf, "<dhcp"); > + if (!def->dhcp_enabled) > + virBufferAddLit(buf, " enabled='no'"); > + if (def->dhcp_relay) > + virBufferAddLit(buf, " relay='yes'"); > + virBufferAddLit(buf, ">\n"); > + > virBufferAdjustIndent(buf, 2); > > - for (ii = 0 ; ii < def->nranges ; ii++) { > + for (ii = 0 ; def->nranges && ii < def->nranges ; ii++) { This line is a spurious change.