[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt-glib PATCHv2 4/5] Add gvir_config_object_foreach_child helper



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]