[Pidgin] #10891: gevolution plugin adds buddies to random account

[Pidgin]

#10891: gevolution plugin adds buddies to random account
-----------------------------------------+----------------------------------
 Reporter:  cedel                        |        Owner:         
     Type:  patch                        |       Status:  new    
Milestone:  Patches Needing Improvement  |    Component:  plugins
  Version:  2.6.3                        |   Resolution:         
 Keywords:  gevolution Evolution         |  
-----------------------------------------+----------------------------------

Comment(by cedel):

 Replying to [comment:6 darkrain42]:

 First, sorry for the delay.


 >    * This should use the purple_account_get_protocol_id accessor
 function (there are a number of places where this is the case. In general,
 if you're doing "account->something", try to use an accessor function
 instead).

 Corrected (hopefully didn't miss any place).


 >    * Can protocolname be NULL?  Under what circumstances does that
 happen?  (Sorry, I'm not terribly familiar with the dialog in the first
 place.)

 Yes, it can. The dialog displays all contacts in the given eds address
 book, even ones that have no IM info. The original version makes all
 decisions based on presence of IM username in the contact and seems to
 discard protocol hint right after icon for it is created - basically, I
 added a string column into the tree model (that stores protocolname) to
 make comparisons easy (or even possible) and set it to NULL, when there's
 no IM info from eds - no username = NULL protocolname.

 In this case I just thought it safer to check for protocolname (instead of
 username), if the next comparison uses strcmp to compare the value of
 protocolname.


 > The method you use to iterate over linked lists is inefficient.
 > (hopefully that makes sense. If not, I can try to explain again)

 Thanks, it makes perfect sense - corrected (in a slightly different way,
 but hopefully along the lines of the recommendation).


 > On line 445, {{{ t = find_account_for_protocol(dialog, protocolname);
 }}}, but the value 't' does not appear to be used anywhere?  It looks like
 that function only ever returns 0, which is never used.  It should be void
 instead (i.e. returns no value).

 Corrected, it was a leftover from a previous experiment.

 I also made a few other subtle changes.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/10891#comment:7>
Pidgin <http://pidgin.im>
Pidgin