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

Re: [libvirt] [PATCH] Java bindings for domain events



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


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