On Mon, Nov 17, 2008 at 03:55:13PM -0500, David Lively wrote:
On Fri, 2008-11-14 at 12:59 -0500, David Lively wrote:
On Fri, 2008-11-14 at 17:09 +0000, Daniel P. Berrange wrote:
Or have the virConnectDomainEventRegister method take an extra parameter which is a callback void (*freefunc)(void*). libvirt would just invoke that to free the opaque data chunk.
???Yeah, I like this better. The dbus(?) API allows an optional destructor (freefunc) to be specified for callback userdata. So let's allow it to be null (in which case we obviously don't call it at remove time).
I think we need a similar thing with the event loops APIs for timers and file handle watches, to make it easier to free the opaque data blob they have.
Sounds good too.
I can make the DomainEvent changes today / this weekend while working on the Java bindings (since I need them to plug the Java leak), and submit them on Monday (or perhaps later today, if I don't get diverted).
The attached patch implements this change (adds a "freefunc" arg to virConnectDomainEventRegister and calls it on Deregister (or Close)).
It also modifies the event-test.c example to register a freefunc and deregister callbacks when interrupted or terminated (to verify the freefuncs are properly called).
From a style point of view, we should keep consistency with the other virEventAddHandle func in terms of typing / param ordering. I prefer to have a typedef for the 'freefunc', even though its trivial, because I hate reading function prototypes :-) Whether we have the freefunc, before or after the 'void opaque' in the register method I don't really mind one way or the other as long as we're consistent. Having
Functionally this all looks fine. the freefunc last is probably best, since its very often just going to be NULL. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|