Re: [libvirt] [PATCH v3 17/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/qemu/*

On 05/10/2013 12:11 PM, Eric Blake wrote: Here, I _think_ it would be okay to change: typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn, virDomainPtr dom, int phase, virDomainEventGraphicsAddressPtr local, virDomainEventGraphicsAddressPtr remote, const char *authScheme, virDomainEventGraphicsSubjectPtr subject, void *opaque); to: typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn, virDomainPtr dom, int phase, const virDomainEventGraphicsAddressPtr local, const virDomainEventGraphicsAddressPtr remote, const char *authScheme, const virDomainEventGraphicsSubjectPtr subject, void *opaque); Although this is an API change, I think that real callers won't be impacted. Why? 1. these callback members are read-only (unlike the ConnectCredential callback struct which was read-write), so it is less likely that someone is trying to assign into the struct members. 2. The only way to register a virConnectDomainEventGraphicsCallback is to cast it through a call to virConnectDomainEventRegisterAny. That is, even if the user's callback function leaves out the const, we never use the typedef as the direct type of any API parameter. Since they are already casting their function pointer into a munged type before registering it, their code will continue to compile. IF you agree with my reasoning about adding 'const' to this particular callback's signature, THEN we can also remove 'const' from the members within those types (that is, your patch did the THEN part; I'd be okay acking if you also add the IF part in the same commit, isolated to an independent patch since it touches user-visible declarations). We can't do that for ConnectCredential, because that type is used directly with no mandated user-casting, and because that type is read-write instead of read-only like these two types. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org