Proposal for an extended callbacks field

Ethan Blanton elb at pidgin.im
Wed Jul 25 12:07:29 EDT 2007


Augie Fackler spake unto us the following wisdom:
> _The Problem_
> 
> Libpurple uses a static struct to describe the callbacks that define  
> a prpl (this is only an excerpt):

[snip]

> This works fine as long as you don't want to change anything. One  
> important thing to be careful about is to not change the struct's  
> size, if you don't want to force the plugin devs to recompile their  
> work. This means that adding new callbacks is limited to four  
> functions right now. Since this is very little, Sean tries to avoid  
> adding things at any cost, effectively nullifying the ability to add  
> new callbacks. This has already added a workaround for setting the  
> registration callback for XMPP, where an additional function outside  
> this struct is used. This creates a problematic inconsistency that  
> complicates the use of the libpurple API for newcomers.

So, without having talked to Sean, I'm going to go ahead and assume
that he *doesn't* try to avoid adding things at any cost, because that
would be dumb, and Sean is far from dumb.  I suspect, rather, that
there is more to this story.

Those reserved pointers are there to be *used*, specifically so that
we don't have to grow the data structure; if they are inviolate, then
effectively we are in the same situation as we were without them.  I
have noticed a tendency toward padding worship, but we really need to
avoid that, and I don't think it has affected any serious decisions
to date.

Why don't you explain what you need additional callbacks for, so that
we can all discuss it.  The only callback I recall coming up before
was a "Now Playing" callback, which was shut down not because it would
have used padding, but because it really wasn't a good conceptual
solution to the problem.  (I believe that, in the process, some
limitations of our status API were exposed, but that is an entirely
different story -- status is the right place to put this.)  If there
have been additional callbacks since then, either they have not been
discussed in the libpurple community, or I missed them (if it is
simply the latter, my apologies).

> _The Solution_
> 
> The proposed solution would use a hashtable, mapping char * -> void  
> (*)(...):

[snip]

> Callbacks other than the ones listed in the struct directly would  
> then be added to the hashtable, therefore preventing any further need  
> to spend precious padding pointers on callback functions. This allows  
> further growth of the library without having to run the risk of  
> binary incompatibilities or requiring inconsistent hacks.

I think this is a bad idea.  We already abuse types far more than we
really should, and this is just Yet Another place where we would be
voluntarily throwing away compile-time type checking.  Type checking
is valuable, and powerful, and we should use it wherever we can.

> _Conclusion_
> 
> This solution will solve the issue at hand until a larger update can  
> be done (like libpurple 3.0). Sean told us that libpurple is planned  
> to move to GObject then, so things will have to be reevaluated at  
> that point. However, setting the callbacks in stone stands in the way  
> of enhancements to the libpurple library, and thus should be avoided.  
> However, it is possible to fix that in the current code, without  
> sacrificing binary compatibility.

Assuming you only need a couple of callbacks, the padding will solve
any callback issues until 3.0, as well.  Let's discuss your needs,
before we talk about hackish solutions.

Ethan

-- 
The laws that forbid the carrying of arms are laws [that have no remedy
for evils].  They disarm only those who are neither inclined nor
determined to commit crimes.
		-- Cesare Beccaria, "On Crimes and Punishments", 1764
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://pidgin.im/pipermail/devel/attachments/20070725/19b17e33/attachment.sig>


More information about the Devel mailing list