Re: [Xen-API] Xen-API C Bindings, version 0.4.1

[Daniel Veillard <veillard@xxxxxxxxxx>]

On Tue, Aug 08, 2006 at 08:11:07PM +0100, Daniel P. Berrange wrote:
> Wow, that's an incredible amount of code written already ! I managed to
> compile & run the test programs without any significant trouble. The only
> two very minor issues I had were likely just related to differing versions
> of the dependant libraries from our respective OS platforms:
> 
> 1. The Makefile sets -Werror but the xen_common.c spews a tonne of warning
>    messages resulting in a failed compile (I removed the -Werror to complete
>    the compile). FYI the warnings were all along the lines of:
> 
> cc -Iinclude -I/usr/include/libxml2  -W -Wall -Wextra -std=c99 -O2 -fPIC   -c 
> -o src/xen_common.o src/xen_common.c
> src/xen_common.c: In function ?xen_init?:
> src/xen_common.c:102: warning: pointer targets in passing argument 1 of 
> ?xmlXPathCompile? differ in signedness

  Classic case of using libxml2 xmlChar * where a char * is expected and
vice-versa, you need to double check that the string is UTF-8 and then just
add a cast (http://xmlsoft.org/FAQ.html#Developer item 12)

   I also noted at least a few places where xmlChar * were freed using
free() instead of xmlFree(), that usually work on Unix, tend to fail on 
Windows very easilly, and will break if a program using this library
also uses libxml2 and changes the allocator (usually done only on embedded
systems but apache modules do that too).

  When using XPath with constant string, there is a serious optimization 
consisting of compiling first the XPath, and then reusing the compiled
XPath instead for the repetive queries. But it's probably a bit early for
optimization.

> On Tue, Aug 08, 2006 at 05:00:23PM +0100, Ewan Mellor wrote:
> > I've added a record type to each of the classes, such as xen_vm_record, 
> > which
> > will be used for client-side storage of data fields.  I've added getters for
> > these records, such as xen_vm_get_record, which return one of these.
> 
> The design of the API feels a little odd to me - there are now basically two
> ways of getting at every piece of information. Either fetch a record and then
> access the struct members, or call each xen_vm_get_XXX method as required. The
> latter methods in their current impl result in an XML-RPC roundtrip on every
> call which seems rather undesirable. Given this performance overhead, under 
> what circumstances would you anticipate one wanting to use the xen_vm_get_XXX 
> methods at all - accessing struct members is just as easy & less overhead?

  I suggested in July adding client side storage precisely to avoid many round 
trip. But I'm not sure how you can update a locally modified structure, that
still seems to require the accessors.

[...]

   I agree with the other points raised by Dan. I will add another one,
the use of enum within public structures and parameters or return of 
functions, this is a very weak point of C, you don't have a good garantee
of allocation size, this may vary from one compiler to another, or
if you grow the number of items in the enum, I would avoid that and
use int (but document that it's supposed to be an enum), you loose a bit
of typechecking by compiler but gain ABI stability.
   Oh, I like my code very commented, usually that's something impossible
to add after the initial write of the code better do it early than late.

Daniel

-- 
Daniel Veillard      | Red Hat http://redhat.com/
veillard@xxxxxxxxxx  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

_______________________________________________
xen-api mailing list
xen-api@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-api