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

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

On Tue, Aug 08, 2006 at 04:06:47PM -0400, Daniel Veillard wrote:

> 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).

Yes, I've obviously missed a few of those.  I'll take Daniel B's warnings and
track them down.

>   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.

I'm doing this already, no?  Or is there another optimisation that I don't
know about?

> [Snip]
>    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.

That's an interesting point.  I thought that enums were defined to be the same
size as int, but looking around it seems more complicated than this!

>    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.

More comments on the way, I promise!


xen-api mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.