Make purple_blist_load() static (#3288) - necessary?
Jay Goel
jpgoel at ncsu.edu
Sun Mar 9 00:29:20 EST 2008
Hello,
I recently submitted (my first!) patch for ticket #3288
(http://developer.pidgin.im/ticket/3288), and the changes have sparked
some interesting discussion on #pidgin; I will outline that below, and
hopefully a decision can be made.
This patch changes the way purple_blist_load() works, and the
ticket+patch outline that pretty clearly. But this raises the question:
are these good/necessary changes to make?
(let me know if i should describe the patch more thoroughly here)
Pros:
Clients implementing libpurple have fewer oddball things to do before
using the client. That is, both pidgin and finch (or the nullclient)
have to make the following calls after calling purple_core_init():
purple_set_blist(purple_blist_new());
purple_blist_load();
This is, well, an extra step, and there is no need to require UIs to
perform this task. Particularly because those functions are modifying
global/static variables; I mean, it doesn't exactly make sense for
someone to call purple_blist_load(), as it depends on the side effect of
the previous set_blist functions.
One could simply make purple_set_blist(), purple_blist_new(), and
purple_blist_load() static, and call them from purple_blist_init(). For
the 2.4.x releases, those functions could just be stubs (to maintain
binary compatibility.) In 3.0.0, they could be removed entirely.
This patch actually does not do all of that - that is, it does not make
blist_new() and blist_load() static or stubs, but that could be fixed
pretty easily (I'll submit another patch - I did not think of these
issues before). The important thing is the "principle."
That said, there are cons:
If libpurple users are, for some reason, calling purple_blist_load()
more than once, then the functionality of libpurple will be changed.
[eg, someone has multiple buddy lists, or something]. Things would
probably break if those functions were not stubbed (that is to say,
calling blist_load() twice is liable to break something.)
Additionally, making those functions static may take away
features+flexibility of a libpurple-based program. Is this really what
we want?
My own thoughts are that, the way the code is structured, it really does
not make sense to call those functions by themselves (knowing full well
that I am very new to this code.) Making the code changes will make UIs
cleaner and give the overall library a more cohesive structure (at
least, it seems that way to me.)
What, then, is the right thing to do?
(there are a couple of other smaller issues, and they are outlined in
the ticket itself.)
Thanks!
Jay
More information about the Devel
mailing list