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). Functionally this all looks fine. >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 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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list