ok, fair enough.
http://git.codethink.co.uk/?p=tracker;a=commitdiff;h=739e33a1a7175d8f481e0fa617cc388b7b7803a7
Was this the last remark before commit approval?
On Thu, 2009-02-12 at 11:39 +0000, Martyn Russell wrote:
> Philip Van Hoof wrote:
> > On Thu, 2009-02-12 at 10:15 +0000, Martyn Russell wrote:
> >>>> +void
> >>>> +tracker_push_registrar_set_object (TrackerPushRegistrar *registrar,
> >>>> + GObject *object)
> >>>> +{
> >>>> + TrackerPushRegistrarPrivate *priv;
> >>>> +
> >>>> + priv = TRACKER_PUSH_REGISTRAR_GET_PRIVATE (registrar);
> >>>> +
> >>>> + if (priv->object)
> >>>> + g_object_unref (priv->object);
> >>>> +
> >>>> + if (object)
> >>>> + priv->object = g_object_ref (object);
> >>>> + else
> >>>> + priv->object = NULL;
> >>>> +}
> >>>> +
> >>> So, what is the problem here exactly?
> >> Say priv->object = 0xdeadbeef. Say object (passed in as an argument to
> >> the function) is also 0xdeadbeef, both are the same object. The object
> >> has 1 reference. We then remove that reference (which finalizes it) and
> >> then we try to ref it again. It is better in these conditions to always
> >> ref the new object just in case we are silly enough somewhere to set the
> >> object to the same thing we already have set.
> >
> > Allowing NULL is being used to reset the object in various places in the
> > code. It would make it complicated if I wouldn't deal with NULL.
> >
> > For example these two in tracker_evolution_push_registrar_enable:
> >
> > tracker_push_registrar_set_object (registrar, NULL);
> > tracker_push_registrar_set_manager (registrar, NULL);
> >
> > The reason is that NameOwnerChanged happens in irregular ways for a new
> > or a being-deleted service. It for example sometimes happens twice per
> > service, in an inconsistent way.
> >
> > Regretfully there's no heuristic that I can apply here. Whether you get
> > NameOwnerChanged twice, three times or just once seems to depend on the
> > moon, Ubuntu version, Fedora version, DBus version, amount of other
> > services, Richard Dawkins's thee-pot between Mars and Earth, etc
> >
> > So each time it happens I need to 'destroy' any existing DBus object
> > being registered for the path, and create a new one. Not just unref a
> > DBus object, but instead being sure that it's destroyed instead.
> >
> > That's the reason for the resetting (and using NULL is the method to be
> > used for that resetting)
> >
> > Note that the reference count for that reason can't ever be > 1, unless
> > you use the instances wrong (of course). So setting to NULL always
> > destroys (unless you are using the instances wrong, but that's a bug
> > that must just be fixed then).
>
> That doesn't make the change useless though. It is good practise to do
> it this way especially if someone decides to cut and paste the code - we
> have all been victims of that one in the past :)
>
> I am not saying it can't be NULL either. It is better to have:
>
> if (object) {
> g_object_ref (object);
> }
>
> if (priv->object) {
> g_object_unref (priv->object);
> }
>
> priv->object = object;
>
--
Philip Van Hoof, freelance software developer
home: me at pvanhoof dot be
gnome: pvanhoof at gnome dot org
http://pvanhoof.be/blog
http://codeminded.be
_______________________________________________
tracker-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/tracker-list
