Re: [libvirt] [PATCH 3/3] virStream: Remove callbacks on Abort/Finish

On Thu, Jun 01, 2017 at 01:29:45PM +0100, Daniel P. Berrange wrote: > On Thu, Jun 01, 2017 at 02:24:19PM +0200, Martin Kletzander wrote: > > On Thu, Jun 01, 2017 at 01:19:05PM +0100, Daniel P. Berrange wrote: > > > On Thu, Jun 01, 2017 at 02:08:34PM +0200, Martin Kletzander wrote: > > > > Users need to remove their callbacks before calling virStreamAbort() > > > > or virStreamFinish() even though that's not documented anywhere. > > > > Since it makes no sense to keep those callbacks, we can remove them > > > > when the stream is being aborted or finished. That way it is also > > > > more intuitive for developers as that removes some confusing errors > > > > being reported. > > > > > > This changes the semantics of a public API though, so even though > > > the suggested behavious would be useful, we mustn't do this as it > > > creates an API incompatability across versions. > > > > > > > I couldn't find any case that could be broken by this change. Do you > > have any in mind? > > Someone writes an app that relies on this behaviour and it runs on a different > libvirt version it'll never unregister the callbacks. This means any freefunc > associated with the callback won't be triggered and thus opaque memory will > leak. So apps will end up having todo "if libvirt version == x then .. else.." > to deal with the semantic change of the API, or just never rely on this new > behaviour at all. > OK. That would most likely happen on downgrade, but this would not be very different to some other leak that we fix at some point. We could document this, but I have a feeling that would not help making my case, would it?