On Wed, May 18, 2011 at 10:32:29AM -0600, Eric Blake wrote: > On 05/18/2011 10:14 AM, Daniel P. Berrange wrote: > > > > While the structs are ABI compatible, this still constitutes a change > > in API. ie apps doing this > > > > int np = 5; > > struct _virMemoryParameter p[np]; > > Apps shouldn't be doing this (struct _virMemoryParameter is a private > name). Furthermore, we already deleted that private name in patch 1/7; > applications using the private name will now be declaring an incomplete > type, which explains the warning you saw in your demo program. IMHO 'struct _virMemoryParameter' is just as much a public name as anything else in the public header file. The typedef serves as convenient syntactic sugar, but using either is valid for app developers, so we should not be deleting it. > > will get a compile warning/error > > > > demo.c: In function âmainâ: > > demo.c:35:3: warning: passing argument 2 of âvirDomainGetSchedulerParametersâ from incompatible pointer type > > demo.c:25:12: note: expected âvirTypedParameterPtrâ but argument is of type âstruct _virMemoryParameter *â > > > > so NACK to this change. > > Even if you convince me to not change the public signatures of the > functions that existed prior to 0.9.2, I still think that it is worth > changing the signature of virDomainSetSchedulerFlags (since we haven't > released it yet), as well as all internal signatures. I think we could perhaps achieve the same cleanup using #defines instead of typedefs, which would avoid breaking app code. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list