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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

You should instead be using the public typedef:

int np = 5;
virMemoryParameter p[np];

> 
>   virDomainGetSchedulerParameters(dom, p, &np);

And since virMemoryParameter and virTypedParameter are two typedefs for
the same type, they shouldn't get a compiler warning.

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

Does your NACK still hold with my explanation?  Do I need to prepare a
v2 that just changes internal names but not the public API use of the
old names?

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]