[Libguestfs] [PATCH v11 5/6] mllib: add XPath helper xpath_get_nodes()
Richard W.M. Jones
rjones at redhat.com
Mon Oct 9 09:48:42 UTC 2017
On Sun, Oct 08, 2017 at 08:15:51PM +0100, Richard W.M. Jones wrote:
> The subject says ‘xpath_get_nodes()‘, but this function doesn't
> actually take a unit parameter, so it's better to drop ‘()’.
>
> On Thu, Oct 05, 2017 at 04:58:29PM +0200, Cédric Bosdonnat wrote:
> > +
> > +let xpath_get_nodes xpathctx expr =
> > + let obj = Xml.xpath_eval_expression xpathctx expr in
> > + let nodes = ref [] in
> > + for i = 0 to Xml.xpathobj_nr_nodes obj - 1 do
> > + let node = Xml.xpathobj_node obj i in
> > + push_back nodes node
> > + done;
> > + !nodes
>
> ‘push_back’ is unfortunately O(n) and no tail recursive, and so the
> whole loop is O(n²). It's going to be much more efficient therefore
> to build the list up in reverse and reverse it at the end:
>
> for ...
> ...
> push_front node nodes
> done;
> List.rev !nodes
>
> Despite that problem this seems like quite a useful function, it would
> be nice to extend this commit so it changes existing code to use the
> function. Grepping the code for ‘Xml.xpathobj_nr_nodes.*- 1’ shows
> some candidates. In v2v/output_libvirt.ml we presently have:
>
> (* Get guest/features/* nodes. *)
> let obj = Xml.xpath_eval_expression xpathctx "features/*" in
>
> let features = ref [] in
> for i = 0 to Xml.xpathobj_nr_nodes obj - 1 do
> let feature_node = Xml.xpathobj_node obj i in
> let feature_name = Xml.node_name feature_node in
> push_front feature_name features
> done;
> !features
>
> I think this can be rewritten as:
>
> (* Get guest/features/* nodes. *)
> let features = xpath_get_nodes xpathctx "features/*" in
> let features = List.map Xml.node_name features in
This patch can be pushed separately if we sort this out.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list