[Pidgin] #1653: Fetch and update Yahoo server-based aliases

[Pidgin]

#1653: Fetch and update Yahoo server-based aliases
------------------------+---------------------------------------------------
  Reporter:  johnmoody  |       Owner:  seanegan   
      Type:  patch      |      Status:  assigned   
  Priority:  minor      |   Milestone:  2.1.0      
 Component:  libpurple  |     Version:  2.0.1      
Resolution:             |    Keywords:  yahoo alias
   Pending:  0          |  
------------------------+---------------------------------------------------
Comment (by johnmoody):

 Replying to [comment:8 seanegan]:
 > Thanks! This all looks great. A few more comments, mostly memory leaks.
 Sorry for all the trouble, but it's not every day I get to review entirely
 new source files! ;)
 >
 > You can't copy the PurpleFetchUri struct from util.c, as if that
 changes, we're likely to forget about this one. Either move the struct
 from util.h to util.c, or the preferred thing to do would be to add new
 functionality to util.c to allow you to do what you need (such as
 specifying aribtrary headers).
 A: Your right.  Have coded around this.  No reference anymore to
 PurpleFetchUrl!
 > 109:  url = g_strdup(url_text);
 >  'url' is never freed
 A: fixed
 > 142: full_name = g_strstrip(g_strdup_printf("%s %s", (fn != NULL ? fn :
 "") , (ln != NULL ? ln : "")));
 >   Neither is full_name. Also, is "fn ln" definitely the proper way to
 assemble this? I know many Asian languages for instance, put the family
 name first, but here it seems like the abbreviations are definitely
 "first" and "last," right? Also, is it best to prefer nickname over full
 name?
 A:  The asian thing has me challenged.  Still trying to find out how the
 Yahoo client handles this.  As for preferring nickname, I worked on the
 theory if someone went to the trouble of giving someone a nickname, they
 would prefer to see it.  And for updates, I only update the remote
 nickname field as I didn't feel like trying to split a PurpleBuddy alias
 into a First and Last name  :)
 > 146:                if (strlen(nick_name) != 0)
 >  "if (*nick_name != '/0')" is a more efficient check here.
 A: fixed
 > 156:                 /* If we don't have the buddy locally (highly
 unlikely), let's add them */
 >  I'm not sure this is the best thing to do. I'd silently ignore the
 buddy instead, and trust the Yahoo protocol over this.
 A: I agree.  The new code highlights how many aliases we get back with no
 matching buddy.  Yahoo obviously doesn't do anything to clean up the
 aliases you have stored when you delete a buddy.
 > 162:                yu = g_new0(struct YahooUser, 1);
 > yu is never freed when the PurpleBuddy is destroyed or in line 167 if
 you're updating a new alias.
 A:  Do you mean destroying when someone deletes a buddy from their list?
 I tidied up some logic so the structure isn't created if there is no
 matching buddy.
 > 151:                    alias = "";  /* No nickname, first name or last
 name, then you get no alias !!  */
 >   alias = NULL; would be more consistent
 A: fixed
 > 270:     cb = g_new0(struct callback_data, 1);
 >   Make sure to free this in yahoo_update_alias_cb
 A: fixed
 > 281:     content = g_strdup_printf("<?xml version=\"1.0\"
 encoding=\"utf-8\"?><ab k=\"%s\" cc=\"1\">\n"
 >   Another leak
 A: fixed
 > 285:     ur->request = g_strdup_printf(
 >   And another
 A: fixed.  Also found a few other leaks and tidied them all up
 (hopefully).

 Thanks for the feedback.  It has been 14 years since they took my compiler
 away and started calling me a manager.  This is my first coding in that
 time and am loving being back  :)

 The latest yahoo_alias.c is attached to this ticket.

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