pidgin: b3ac8324: Add purple_{buddy,chat,group,contact}_de...

[Mark Doliner]

On Thu, Apr 9, 2009 at 1:32 PM, Ethan Blanton <elb at pidgin.im> wrote:
> Sadrul Habib Chowdhury spake unto us the following wisdom:
>> > Modified files:
>> >         ChangeLog ChangeLog.API libpurple/blist.c libpurple/blist.h
>> >
>> > ChangeLog:
>> >
>> > Add purple_{buddy,chat,group,contact}_destroy to the blist API, free
>> > blist data on libpurple unload.  Thanks to Nick Hebner for this.
>> >
>> > References #8683
>> >
>>
>> We probably should not make the _destroy functions public. It'd be too
>> easy for a plugin to do purple_buddy_destroy and screw up the tree, when
>> purple_blist_remove_buddy would have been more appropriate.
>
> Then I posit that purple_buddy_new et al. should not be public,
> either.  If we expose one, we really need to expose the other.  We
> discussed this briefly in devel at cpi.

I agree with this 116%.

> Alternately, purple_{buddy,chat,group,contact}_new could make sure
> that the allocated object is properly tracked for destruction.  They
> do not currently seem to do so.
>
> I'm happy to make these static if that's the right thing to do, of
> course -- it just wasn't immediately clear to me that it is.

It seems like the two possible options are:

1. Keep the destroy functions public and have them remove the node
from the tree before destroying it.  This shouldn't require any
additional struct members, right?  I think nodes already have a
reference to their parent.

2. Combine purple_buddy_new() with purple_blist_add_buddy() and
combine purple_buddy_destroy() with purple_blist_remove_buddy().  This
would require remove a public function, which would require a major
version bump.  But I like this because it would make the API simpler,
and our API is kinda hard to use right now.  I'm not sure if there
would be other side effects related to adding the buddy to the tree
and emitting signals before the buddy node has been configured all the
way (alias set, buddy icon set, etc)

-Mark