[libvirt] [PATCH 2.5/7] maint: prefer newer API names internally

Daniel P. Berrange berrange at redhat.com
Wed May 18 16:55:11 UTC 2011


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




More information about the libvir-list mailing list