Re: [libvirt] [PATCH 2.5/7] maint: prefer newer API names internally
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 :|