# HG changeset patch # User Daniel Atallah # Date 1188755684 0 # Node ID affacee881e82f79eb37903047cd4b1a2f5213b3 # Parent cfc4e56a6a1ec3f50d1b37e05cd579d7203bce80 Fix a couple potential leaks and prevent the avahi record resolver from being freed multiple times. Thanks to Lennart from the Avahi team for suggestions on how to improve the avahi implementation. diff -r cfc4e56a6a1e -r affacee881e8 libpurple/protocols/bonjour/bonjour.c --- a/libpurple/protocols/bonjour/bonjour.c Sun Sep 02 14:40:04 2007 +0000 +++ b/libpurple/protocols/bonjour/bonjour.c Sun Sep 02 17:54:44 2007 +0000 @@ -235,6 +235,13 @@ g_free(stripped); } +static void bonjour_remove_buddy(PurpleConnection *pc, PurpleBuddy *buddy, PurpleGroup *group) { + if (buddy->proto_data) { + bonjour_buddy_delete(buddy->proto_data); + buddy->proto_data = NULL; + } +} + static GList * bonjour_status_types(PurpleAccount *account) { @@ -395,7 +402,7 @@ NULL, /* change_passwd */ NULL, /* add_buddy */ NULL, /* add_buddies */ - NULL, /* remove_buddy */ + bonjour_remove_buddy, /* remove_buddy */ NULL, /* remove_buddies */ NULL, /* add_permit */ NULL, /* add_deny */ diff -r cfc4e56a6a1e -r affacee881e8 libpurple/protocols/bonjour/buddy.c --- a/libpurple/protocols/bonjour/buddy.c Sun Sep 02 14:40:04 2007 +0000 +++ b/libpurple/protocols/bonjour/buddy.c Sun Sep 02 17:54:44 2007 +0000 @@ -121,9 +121,8 @@ * the buddy. */ void -bonjour_buddy_add_to_purple(BonjourBuddy *bonjour_buddy) +bonjour_buddy_add_to_purple(BonjourBuddy *bonjour_buddy, PurpleBuddy *buddy) { - PurpleBuddy *buddy; PurpleGroup *group; PurpleAccount *account = bonjour_buddy->account; const char *status_id, *old_hash, *new_hash; @@ -147,7 +146,8 @@ } /* Make sure the buddy exists in our buddy list */ - buddy = purple_find_buddy(account, bonjour_buddy->name); + if (buddy == NULL) + buddy = purple_find_buddy(account, bonjour_buddy->name); if (buddy == NULL) { buddy = purple_buddy_new(account, bonjour_buddy->name, NULL); diff -r cfc4e56a6a1e -r affacee881e8 libpurple/protocols/bonjour/buddy.h --- a/libpurple/protocols/bonjour/buddy.h Sun Sep 02 14:40:04 2007 +0000 +++ b/libpurple/protocols/bonjour/buddy.h Sun Sep 02 17:54:44 2007 +0000 @@ -92,8 +92,9 @@ /** * If the buddy doesn't previoulsy exists, it is created. Else, its data is changed (???) + * purple_buddy is optional; it saves an additional lookup if we already have it */ -void bonjour_buddy_add_to_purple(BonjourBuddy *buddy); +void bonjour_buddy_add_to_purple(BonjourBuddy *bonjour_buddy, PurpleBuddy *purple_buddy); /** * We got the buddy icon data; deal with it diff -r cfc4e56a6a1e -r affacee881e8 libpurple/protocols/bonjour/mdns_avahi.c --- a/libpurple/protocols/bonjour/mdns_avahi.c Sun Sep 02 14:40:04 2007 +0000 +++ b/libpurple/protocols/bonjour/mdns_avahi.c Sun Sep 02 17:54:44 2007 +0000 @@ -33,7 +33,7 @@ #include #include -/* For some reason, this is missing from the Avahi type defines */ +/* Avahi only defines the types that it actually uses (which at this time doesn't include NULL) */ #ifndef AVAHI_DNS_TYPE_NULL #define AVAHI_DNS_TYPE_NULL 0x0A #endif @@ -58,7 +58,8 @@ const char *host_name, const AvahiAddress *a, uint16_t port, AvahiStringList *txt, AvahiLookupResultFlags flags, void *userdata) { - BonjourBuddy *buddy; + PurpleBuddy *pb; + BonjourBuddy *bb; PurpleAccount *account = userdata; AvahiStringList *l; size_t size; @@ -67,44 +68,62 @@ g_return_if_fail(r != NULL); + pb = purple_find_buddy(account, name); + bb = (pb != NULL) ? pb->proto_data : NULL; + switch (event) { case AVAHI_RESOLVER_FAILURE: purple_debug_error("bonjour", "_resolve_callback - Failure: %s\n", avahi_strerror(avahi_client_errno(avahi_service_resolver_get_client(r)))); + avahi_service_resolver_free(r); + if (bb != NULL) { + /* We've already freed the resolver */ + if (r == ((AvahiBuddyImplData *)bb->mdns_impl_data)->resolver) + ((AvahiBuddyImplData *)bb->mdns_impl_data)->resolver = NULL; + purple_blist_remove_buddy(pb); + } break; case AVAHI_RESOLVER_FOUND: /* create a buddy record */ - buddy = bonjour_buddy_new(name, account); + if (bb == NULL) + bb = bonjour_buddy_new(name, account); - ((AvahiBuddyImplData *)buddy->mdns_impl_data)->resolver = r; + /* If we're reusing an existing buddy, make sure if it is a different resolver to clean up the old one. + * I don't think this should ever happen, but I'm afraid we might get events out of sequence. */ + if (((AvahiBuddyImplData *)bb->mdns_impl_data)->resolver != NULL + && ((AvahiBuddyImplData *)bb->mdns_impl_data)->resolver != r) { + avahi_service_resolver_free(((AvahiBuddyImplData *)bb->mdns_impl_data)->resolver); + } + ((AvahiBuddyImplData *)bb->mdns_impl_data)->resolver = r; + g_free(bb->ip); /* Get the ip as a string */ - buddy->ip = g_malloc(AVAHI_ADDRESS_STR_MAX); - avahi_address_snprint(buddy->ip, AVAHI_ADDRESS_STR_MAX, a); + bb->ip = g_malloc(AVAHI_ADDRESS_STR_MAX); + avahi_address_snprint(bb->ip, AVAHI_ADDRESS_STR_MAX, a); - buddy->port_p2pj = port; + bb->port_p2pj = port; /* Obtain the parameters from the text_record */ - clear_bonjour_buddy_values(buddy); - l = txt; - while (l != NULL) { - ret = avahi_string_list_get_pair(l, &key, &value, &size); - l = l->next; - if (ret < 0) + clear_bonjour_buddy_values(bb); + for(l = txt; l != NULL; l = l->next) { + if ((ret = avahi_string_list_get_pair(l, &key, &value, &size)) < 0) continue; - set_bonjour_buddy_value(buddy, key, value, size); + set_bonjour_buddy_value(bb, key, value, size); /* TODO: Since we're using the glib allocator, I think we * can use the values instead of re-copying them */ avahi_free(key); avahi_free(value); } - if (!bonjour_buddy_check(buddy)) - bonjour_buddy_delete(buddy); - else + if (!bonjour_buddy_check(bb)) { + if (pb != NULL) + purple_blist_remove_buddy(pb); + else + bonjour_buddy_delete(bb); + } else /* Add or update the buddy in our buddy list */ - bonjour_buddy_add_to_purple(buddy); + bonjour_buddy_add_to_purple(bb, pb); break; default: @@ -120,7 +139,7 @@ AvahiLookupResultFlags flags, void *userdata) { PurpleAccount *account = userdata; - PurpleBuddy *gb = NULL; + PurpleBuddy *pb = NULL; switch (event) { case AVAHI_BROWSER_FAILURE: @@ -132,7 +151,7 @@ /* A new peer has joined the network and uses iChat bonjour */ purple_debug_info("bonjour", "_browser_callback - new service\n"); /* Make sure it isn't us */ - if (g_ascii_strcasecmp(name, account->username) != 0) { + if (purple_utf8_strcasecmp(name, account->username) != 0) { if (!avahi_service_resolver_new(avahi_service_browser_get_client(b), interface, protocol, name, type, domain, AVAHI_PROTO_UNSPEC, 0, _resolver_callback, account)) { @@ -143,16 +162,12 @@ break; case AVAHI_BROWSER_REMOVE: purple_debug_info("bonjour", "_browser_callback - Remove service\n"); - gb = purple_find_buddy(account, name); - if (gb != NULL) { - bonjour_buddy_delete(gb->proto_data); - purple_blist_remove_buddy(gb); - } + pb = purple_find_buddy(account, name); + if (pb != NULL) + purple_blist_remove_buddy(pb); break; case AVAHI_BROWSER_ALL_FOR_NOW: case AVAHI_BROWSER_CACHE_EXHAUSTED: - purple_debug_warning("bonjour", "(Browser) %s\n", - event == AVAHI_BROWSER_CACHE_EXHAUSTED ? "CACHE_EXHAUSTED" : "ALL_FOR_NOW"); break; default: purple_debug_info("bonjour", "Unrecognized Service browser event: %d.\n", event); diff -r cfc4e56a6a1e -r affacee881e8 libpurple/protocols/bonjour/mdns_howl.c --- a/libpurple/protocols/bonjour/mdns_howl.c Sun Sep 02 14:40:04 2007 +0000 +++ b/libpurple/protocols/bonjour/mdns_howl.c Sun Sep 02 17:54:44 2007 +0000 @@ -71,6 +71,7 @@ char value[SW_TEXT_RECORD_MAX_LEN]; sw_uint32 value_length; + /* TODO: We want to keep listening for updates*/ sw_discovery_cancel(discovery, oid); /* create a buddy record */ @@ -100,7 +101,7 @@ } /* Add or update the buddy in our buddy list */ - bonjour_buddy_add_to_purple(buddy); + bonjour_buddy_add_to_purple(buddy, NULL); return SW_OKAY; } @@ -149,10 +150,7 @@ purple_debug_info("bonjour", "_browser_reply --> Remove service\n"); gb = purple_find_buddy(account, name); if (gb != NULL) - { - bonjour_buddy_delete(gb->proto_data); purple_blist_remove_buddy(gb); - } break; case SW_DISCOVERY_BROWSE_RESOLVED: purple_debug_info("bonjour", "_browse_reply --> Resolved\n"); diff -r cfc4e56a6a1e -r affacee881e8 libpurple/protocols/bonjour/mdns_win32.c --- a/libpurple/protocols/bonjour/mdns_win32.c Sun Sep 02 14:40:04 2007 +0000 +++ b/libpurple/protocols/bonjour/mdns_win32.c Sun Sep 02 17:54:44 2007 +0000 @@ -79,23 +79,23 @@ uint32_t ttl, void *context) { - if (kDNSServiceErr_NoError != errorCode) + if (kDNSServiceErr_NoError != errorCode) { purple_debug_error("bonjour", "record query - callback error.\n"); - else if (flags & kDNSServiceFlagsAdd) - { + /* TODO: Probably should remove the buddy when this happens */ + } else if (flags & kDNSServiceFlagsAdd) { if (rrtype == kDNSServiceType_TXT) { /* New Buddy */ - BonjourBuddy *buddy = (BonjourBuddy*) context; - _mdns_parse_text_record(buddy, rdata, rdlen); - bonjour_buddy_add_to_purple(buddy); + BonjourBuddy *bb = (BonjourBuddy*) context; + _mdns_parse_text_record(bb, rdata, rdlen); + bonjour_buddy_add_to_purple(bb, NULL); } else if (rrtype == kDNSServiceType_NULL) { /* Buddy Icon response */ - BonjourBuddy *buddy = (BonjourBuddy*) context; - Win32BuddyImplData *idata = buddy->mdns_impl_data; + BonjourBuddy *bb = (BonjourBuddy*) context; + Win32BuddyImplData *idata = bb->mdns_impl_data; g_return_if_fail(idata != NULL); - bonjour_buddy_got_buddy_icon(buddy, rdata, rdlen); + bonjour_buddy_got_buddy_icon(bb, rdata, rdlen); /* We've got what we need; stop listening */ purple_input_remove(idata->null_query_handler); @@ -110,32 +110,34 @@ _mdns_resolve_host_callback(GSList *hosts, gpointer data, const char *error_message) { ResolveCallbackArgs* args = (ResolveCallbackArgs*)data; + BonjourBuddy* bb = args->buddy; - if (!hosts || !hosts->data) + if (!hosts || !hosts->data) { purple_debug_error("bonjour", "host resolution - callback error.\n"); - else { + bonjour_buddy_delete(bb); + } else { struct sockaddr_in *addr = (struct sockaddr_in*)g_slist_nth_data(hosts, 1); - BonjourBuddy* buddy = args->buddy; - Win32BuddyImplData *idata = buddy->mdns_impl_data; + Win32BuddyImplData *idata = bb->mdns_impl_data; g_return_if_fail(idata != NULL); - buddy->ip = g_strdup(inet_ntoa(addr->sin_addr)); + g_free(bb->ip); + bb->ip = g_strdup(inet_ntoa(addr->sin_addr)); /* finally, set up the continuous txt record watcher, and add the buddy to purple */ if (kDNSServiceErr_NoError == DNSServiceQueryRecord(&idata->txt_query, kDNSServiceFlagsLongLivedQuery, kDNSServiceInterfaceIndexAny, args->full_service_name, kDNSServiceType_TXT, - kDNSServiceClass_IN, _mdns_record_query_callback, buddy)) { + kDNSServiceClass_IN, _mdns_record_query_callback, bb)) { - purple_debug_info("bonjour", "Found buddy %s at %s:%d\n", buddy->name, buddy->ip, buddy->port_p2pj); + purple_debug_info("bonjour", "Found buddy %s at %s:%d\n", bb->name, bb->ip, bb->port_p2pj); idata->txt_query_handler = purple_input_add(DNSServiceRefSockFD(idata->txt_query), PURPLE_INPUT_READ, _mdns_handle_event, idata->txt_query); - bonjour_buddy_add_to_purple(buddy); + bonjour_buddy_add_to_purple(bb, NULL); } else - bonjour_buddy_delete(buddy); + bonjour_buddy_delete(bb); } @@ -202,18 +204,19 @@ DNSServiceErrorType errorCode, const char *serviceName, const char *regtype, const char *replyDomain, void *context) { PurpleAccount *account = (PurpleAccount*)context; - PurpleBuddy *gb = NULL; + PurpleBuddy *pb = NULL; if (kDNSServiceErr_NoError != errorCode) purple_debug_error("bonjour", "service browser - callback error"); else if (flags & kDNSServiceFlagsAdd) { /* A presence service instance has been discovered... check it isn't us! */ - if (g_ascii_strcasecmp(serviceName, account->username) != 0) { + if (purple_utf8_strcasecmp(serviceName, account->username) != 0) { /* OK, lets go ahead and resolve it to add to the buddy list */ ResolveCallbackArgs *args = g_new0(ResolveCallbackArgs, 1); args->buddy = bonjour_buddy_new(serviceName, account); - if (kDNSServiceErr_NoError != DNSServiceResolve(&args->resolver, 0, 0, serviceName, regtype, replyDomain, _mdns_service_resolve_callback, args)) { + if (kDNSServiceErr_NoError != DNSServiceResolve(&args->resolver, 0, 0, serviceName, regtype, + replyDomain, _mdns_service_resolve_callback, args)) { bonjour_buddy_delete(args->buddy); g_free(args); purple_debug_error("bonjour", "service browser - failed to resolve service.\n"); @@ -226,11 +229,9 @@ } else { /* A peer has sent a goodbye packet, remove them from the buddy list */ purple_debug_info("bonjour", "service browser - remove notification\n"); - gb = purple_find_buddy(account, serviceName); - if (gb != NULL) { - bonjour_buddy_delete(gb->proto_data); - purple_blist_remove_buddy(gb); - } + pb = purple_find_buddy(account, serviceName); + if (pb != NULL) + purple_blist_remove_buddy(pb); } }