On Tue, Dec 04, 2007 at 04:06:32PM +0000, Daniel P. Berrange wrote:
On Tue, Dec 04, 2007 at 10:46:50AM -0500, Daniel Veillard wrote:
On Tue, Dec 04, 2007 at 12:57:39PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
virConnectPtr virConnectOpenAuth (const char *name, virConnectAuthPtr auth, int flags);
I'm a fan of callers passing in the size of the structure (as they see it). Allows the structure to be expanded in future, and if done right can allow both forwards and backwards compatibility.
cf: http://www.libvirt.org/html/libvirt-libvirt.html#virDomainInterfaceStats
Hum, honnestly, that's not my preferred way. If you really think there should be room for expansion, I would either: - add a version number to the structure and allocator/destructor functions as part of the API (prefered)
This adds more complexity and the single sizeof(*auth) param IMHO.
not really, I could argue that most users would prefer a function to allocate the data needed, instead of doing the right malloc/sizeof/return check themselve
- add padding at tyhe end of the structure which could allow a future growth
adding the size of the structure as the argument moves the complexity away from the library implementor to the library user,
It doesn't really - the user just has to add sizeof(*auth) as an arg and all the rest of the complexity is in the library internals.
and I'm afraid they won't naturally do sizeof(*auth) and could very well put a number in situ
In fact it doesn't need to add any complexity, because we could make it a compile time macro
#define virConnectOpenAuth(name,auth,flags) virConnectOpenAuth(name,auth,sizeof(*auth),flags)
Okay except I really prefer macros all uppercase, even if I see why you would do this. It's not a function anymore, you could hide or expose it, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/