On Tue, Jan 3, 2012 at 4:50 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> When iterating over xmlNodePtr children to parse an XML document
> describing a libvirt configuration item, we want to ignore blank
> nodes that may have been added to make the initial XML document
> "more human readable". Since this will be useful in several places,
> move this code to a helper function.
Looks good, apart from the "more human readable" sarcasm. :) Oh and
a few small things:
> +static gboolean add_one_feature(xmlNodePtr node, gpointer opaque)
'feature' is singular so 'one_' is pretty much redundant here.
> +void gvir_config_xml_foreach_child(xmlNodePtr node,
> + GVirConfigXmlNodeIterator it_func,
> + gpointer opaque)
I'd suggest using 'iter_func' or just 'func' as the parameter name
but this is entirely subjective so feel free to ignore this
suggestion. :)
> + for (it = node->children; it != NULL; it = it->next) {
> + gboolean cont;
> +
> + if (xmlIsBlankNode(it))
> + continue;
> + cont = it_func(it, opaque);
> + if (!cont)
> + break;
Since 'cont' is only used once and there is no big line (to not fit
in the 120 cols limit), I suggest not using it here.
ACK otherwise.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list