changeset 19573:affacee881e8

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.
author Daniel Atallah <daniel.atallah@gmail.com>
date Sun, 02 Sep 2007 17:54:44 +0000
parents cfc4e56a6a1e
children 917b6f45c458
files libpurple/protocols/bonjour/bonjour.c libpurple/protocols/bonjour/buddy.c libpurple/protocols/bonjour/buddy.h libpurple/protocols/bonjour/mdns_avahi.c libpurple/protocols/bonjour/mdns_howl.c libpurple/protocols/bonjour/mdns_win32.c
diffstat 6 files changed, 84 insertions(+), 62 deletions(-) [+]
line wrap: on
line diff
--- 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 */
--- 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);
--- 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
--- 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 <avahi-glib/glib-malloc.h>
 #include <avahi-glib/glib-watch.h>
 
-/* 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);
--- 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");
--- 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);
 	}
 }