On Fri, Apr 17, 2015 at 07:06:04PM +0200, Paolo Bonzini wrote:
>
>
> On 17/04/2015 18:11, Daniel P. Berrange wrote:
> >
> > + task = qio_task_new(OBJECT(ioc),
> > + func, opaque, destroy);
> > +
> > + qio_channel_tls_handshake_task(ioc, task);
> > +
> > + object_unref(OBJECT(task));
> >
> > The second ref taken at time of qio_channel_add_watch_full() gets released
> > by the GDestroyNotify that is passed to that method.
>
> /me is blind.
>
> So this means you really do not need the reference counting;
> qio_channel_tls_handshake_task (which is static anyway) is what manages
> the lifetime of the QIOTask, it can take ownership of the task right
> after it is created.
>
> In effect QIOTask is just wrapping the (func, opaque, destroy)
> continuation. It makes sense that it doesn't need reference counting,
> because it has a well-defined point of death (just before the
> continuation is called, or just before you find out it will never be
> called). So I think it's okay to remove the reference counting and make
> it a simple heap-allocated struct. gio_channel_tls_handshake_task can
> free it after calling gio_task_{abort,complete}.
>
> Without the reference counting the GDestroyNotify also becomes
> unnecessary (and the fact that you're using QIOTask as a simple
> continuation explains why you didn't need GDestroyNotify), but it's okay
> if you want to leave it in.
>
> I would also have QIOTaskFunc take the "exploded" struct, i.e. typedef
> void QIOTaskFunc(QMyObject *, gpointer, Error *), and hide QIOTask
> entirely from the user.
Yeah, I can try that approach and see how it works out.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
