Hi,
On Tue, Feb 21, 2017 at 05:16:26PM +0100, Christophe Fergeau wrote:
> On Mon, Feb 20, 2017 at 03:49:02PM +0100, Victor Toso wrote:
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > The custom WeakRef structure was introduced in 9baba9fd89 (2012) to
> > fix rhbz#743773. Since glib 2.32, GWeakRef was introduced and it
> > behaves similarly to our WeakRef.
> >
> > Moving to GWeakRef to remove some code which exists in glib.
> >
> > Note that I'm keeping to utility functions:
Fixed typo: to -> two
> > - get_weak_ref(gpointer object): which returns a newly allocated
> > GWeakRef, initialized with @object;
> > - free_weak_ref(gpointer pdata): which frees the GWeakRef and returns
> > the original object or NULL. It also takes care of an extra
> > reference to the object.
>
> Yes, this dropping of the strong ref is a bit odd at first, but in the
> end it's no different than how the code was working before, we expect
> the object to stay alive for the duration of the callback if it was
> alive when entering in the callback.
>
> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > ---
> > src/spice-gtk-session.c | 52 +++++++++++++++++++------------------------------
> > 1 file changed, 20 insertions(+), 32 deletions(-)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 5688cba..88f4488 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -575,31 +575,26 @@ static const struct {
> > }
> > };
> >
> > -typedef struct _WeakRef {
> > - GObject *object;
> > -} WeakRef;
> > -
> > -static void weak_notify_cb(WeakRef *weakref, GObject *object)
> > -{
> > - weakref->object = NULL;
> > -}
> > -
> > -static WeakRef* weak_ref(GObject *object)
> > +static GWeakRef* get_weak_ref(gpointer object)
> > {
> > - WeakRef *weakref = g_new(WeakRef, 1);
> > -
> > - g_object_weak_ref(object, (GWeakNotify)weak_notify_cb, weakref);
> > - weakref->object = object;
> > -
> > + GWeakRef *weakref = g_new(GWeakRef, 1);
> > + g_weak_ref_init(weakref, object);
> > return weakref;
> > }
> >
> > -static void weak_unref(WeakRef* weakref)
> > +static gpointer free_weak_ref(gpointer data)
> > {
> > - if (weakref->object)
> > - g_object_weak_unref(weakref->object, (GWeakNotify)weak_notify_cb, weakref);
> > + GWeakRef *weakref = data;
> > + gpointer object = g_weak_ref_get(weakref);
> >
> > + g_weak_ref_clear(weakref);
> > g_free(weakref);
> > + if (object != NULL) {
> > + /* The main referece still exists as object is not NULL, so we can
>
> 'reference'
Fixed
>
> Looks good to me, though I'd probably return a ref to the callers, and
> add a g_object_unref() to the end of the callbacks.
>
> Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Yeah, it was my first option too but I didn't like to outcome as much as
having the unref in the helper function.
Thanks, I'll be pushing it soon
toso
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel