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 _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
