Re: [PATCH 3/4] libxl: add version_info function [and 1 more messages]

[Vincent Hanquez <vincent.hanquez@eu.citrix.com>]

On 19/04/10 21:43, Andre Przywara wrote:
> I am not fully convinced of this. There is quite a lot of information
> copied (up to 4KB), and some fields (like pagesize or the version
> numbers) are just plain integers. And this approach just propagates the
> underlying design.
> In the xl info implementation I used this feature to just get the
> pagesize. One could think of just querying the version number, too. If
> you don't like the additional parameter, what about a wrapping macro, then?
> I don't see the problem with the complexity, though, as it is hidden
> inside the library.

The query mask is exposed to the user, and the structure is partially 
filled. I can imagine that it's going to create those kind of issues, 
where the wrong mask is passed, or change later, and then the field the 
user was expecting would be left null.

The macro would help in the previous case, but I still feel it's 
unnecessary. As a user, I would just not bother, and call the function
always with full mask, until I reach the optimisation phase, and its 
appears on the graph.

A couple of K of data copied shouldn't make a difference, and would be 
completely lost in noise, however if you really think it makes a 
difference, you could have special dedicated info call to get specific 
things like version number.

> I can use libxl_sprintf function to get rid of the free() function, but
> I personally don't like the idea of accumulated mallocs much, left alone
> the "hidden" free operation.

There is a real advantage of permitting a fast development of libxl (and 
its bindings).

This was to get started at the beggining; when we reach a point where 
everything is working we could improve or remove the memory system. But 
I'm fairly convinced it's not going to make a difference for this kind 
of library.

-- 
Vincent