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