Re: [Opensync-devel] leak in osync_member_get_config_or_default?

[Graham C. <g+o...@co...>]

On Friday 25 June 2010 09:24:49 Daniel Gollub wrote:
> The idea how this works is like this:

Hmm.  This explanation works but it doesn't seem quite the right approach to 
me.  I would have thought that any time a function provides a pointer to an 
object to another piece of code, it should have incremented the ref count.  
When the piece of code receiving the pointer no longer needs it it should 
decrement the reference count.

> All osync_XXX_new() calls create "objects" with a refrence count of 1.
> So on the first osync_XXX_unref() call the count decrease to 0 and the
> "object" get freed.

This is consistent with my approach: osync_XXX_new has created an object and 
returned a pointer: so it has set the ref-count to 1.  The caller must unref 
the object when it no longer needs it.

> Ref get increased as soon some other "object" or code starts using this
> "object" as well.

I would have said, ref gets increased as soon as some other code gets a 
pointer to the object (whether it uses it or not).

> In this case the reference get increased since the OSyncMember is going to
> use OSyncPluginConfig*. Since it's no longer used within the
> osync_member_get_config_or_default() function which allocated
> OSyncPluginConfig* we have to decrease the reference once
> osync_member_set_config() get called:

That might be true.   But I would have thought that because the config is 
going to be returned to the caller, it will have increment the ref again, 
just before returning the pointer to the caller.  Once the caller has 
finished doing whatever it wants with the config it unref's it and throws 
away the pointer.  Now the config will be deleted as soon as the member is 
deleted.

In the same way, in the case where it is returning an existing member->config, 
osync_member_get_config_or_default should increase the reference just before 
returning the pointer to the caller.

To understand why I think this way, consider if the caller of 
osync_member_get_config_or_default wants to look at the config but, for some 
reason, decides it doesn't need the member itself any more.  It could unref 
the member but still expect the config pointer to be valid.  

I agree that this particular case is unlikely but it feels like there is a 
general principle: any function that returns a pointer to an object should 
have incremented the ref count on the object; the caller must unref the 
object when it is about to discard the pointer.  After all, it is 
a "reference" count -- a count of the number of pointers to the object which 
exist.

Graham