
On Tue, Jan 3, 2012 at 4:50 PM, Christophe Fergeau <cfergeau@redhat.com> 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