Mercurial > pidgin

changeset 25187:6bdcdb77ce77

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Reference-count JabberCapsClientInfo and fix bug. jabber_caps_get_info() wouldn't ever actually trigger the callback if the data were already in the hash. Fix that + a leak of the lookup key and userdata.
author Paul Aurich <paul@darkrain42.org>
date Mon, 15 Dec 2008 20:43:02 +0000
parents ef90728dbae8
children d5b1fede10a0
files libpurple/protocols/jabber/buddy.c libpurple/protocols/jabber/buddy.h libpurple/protocols/jabber/caps.c libpurple/protocols/jabber/caps.h libpurple/protocols/jabber/presence.c
diffstat 5 files changed, 77 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/libpurple/protocols/jabber/buddy.c	Sun Dec 14 21:19:02 2008 +0000
+++ b/libpurple/protocols/jabber/buddy.c	Mon Dec 15 20:43:02 2008 +0000
@@ -181,7 +181,7 @@
 		jbr->commands = g_list_delete_link(jbr->commands, jbr->commands);
 	}
 
-	/* jbr->caps is owned by the caps code */
+	jabber_caps_client_info_unref(jbr->caps);
 	g_free(jbr->name);
 	g_free(jbr->status);
 	g_free(jbr->thread_id);
--- a/libpurple/protocols/jabber/buddy.h	Sun Dec 14 21:19:02 2008 +0000
+++ b/libpurple/protocols/jabber/buddy.h	Mon Dec 15 20:43:02 2008 +0000
@@ -81,7 +81,7 @@
 		char *name;
 		char *os;
 	} client;
-	const JabberCapsClientInfo *caps;
+	JabberCapsClientInfo *caps;
 	GList *commands;
 } JabberBuddyResource;
 
--- a/libpurple/protocols/jabber/caps.c	Sun Dec 14 21:19:02 2008 +0000
+++ b/libpurple/protocols/jabber/caps.c	Mon Dec 15 20:43:02 2008 +0000
@@ -84,9 +84,24 @@
 	g_free(keystruct);
 }
 
-static void
-jabber_caps_client_info_destroy(gpointer data) {
-	JabberCapsClientInfo *info = data;
+void
+jabber_caps_client_info_ref(JabberCapsClientInfo *info)
+{
+	g_return_if_fail(info != NULL);
+	++info->ref;
+}
+
+void
+jabber_caps_client_info_unref(JabberCapsClientInfo *info)
+{
+	if (info == NULL)
+		return;
+
+	g_return_if_fail(info->ref > 0);
+
+	if (--info->ref > 0)
+		return;
+
 	while(info->identities) {
 		JabberIdentity *id = info->identities->data;
 		g_free(id->category);
@@ -97,15 +112,11 @@
 		info->identities = g_list_delete_link(info->identities, info->identities);
 	}
 
-	while(info->features) {
-		g_free(info->features->data);
-		info->features = g_list_delete_link(info->features, info->features);
-	}
+	g_list_foreach(info->features, (GFunc)g_free, NULL);
+	g_list_free(info->features);
 
-	while(info->forms) {
-		g_free(info->forms->data);
-		info->forms = g_list_delete_link(info->forms, info->forms);
-	}
+	g_list_foreach(info->forms, (GFunc)g_free, NULL);
+	g_list_free(info->forms);
 
 #if 0
 	g_hash_table_destroy(valuestruct->ext);
@@ -138,7 +149,7 @@
 
 void jabber_caps_init(void)
 {
-	capstable = g_hash_table_new_full(jabber_caps_hash, jabber_caps_compare, jabber_caps_destroy_key, jabber_caps_client_info_destroy);
+	capstable = g_hash_table_new_full(jabber_caps_hash, jabber_caps_compare, jabber_caps_destroy_key, (GDestroyNotify)jabber_caps_client_info_unref);
 	jabber_caps_load();
 }
 
@@ -167,6 +178,7 @@
 			JabberCapsKey *key = g_new0(JabberCapsKey, 1);
 			JabberCapsClientInfo *value = g_new0(JabberCapsClientInfo, 1);
 			xmlnode *child;
+			jabber_caps_client_info_ref(value);
 			key->node = g_strdup(xmlnode_get_attrib(client,"node"));
 			key->ver  = g_strdup(xmlnode_get_attrib(client,"ver"));
 			key->hash = g_strdup(xmlnode_get_attrib(client,"hash"));
@@ -470,7 +482,7 @@
 		                     xmlnode_get_attrib(packet, "from"));
 
 		userdata->cb(NULL, userdata->cb_data);
-		jabber_caps_client_info_destroy(info);
+		jabber_caps_client_info_unref(info);
 		goto out;
 	}
 
@@ -481,7 +493,8 @@
 	/* Use the copy of this data already in the table if it exists or insert
 	 * a new one if we need to */
 	if ((value = g_hash_table_lookup(capstable, &key))) {
-		jabber_caps_client_info_destroy(info);
+		jabber_caps_client_info_unref(info);
+		jabber_caps_client_info_ref(value);
 		info = value;
 	} else {
 		JabberCapsKey *n_key = g_new(JabberCapsKey, 1);
@@ -495,7 +508,6 @@
 	}
 
 	userdata->cb(info, userdata->cb_data);
-	/* capstable owns info */
 
 out:
 	g_free(userdata->who);
@@ -510,26 +522,37 @@
 		const char *ver, const char *hash, jabber_caps_get_info_cb cb,
 		gpointer user_data)
 {
-	JabberCapsClientInfo *client;
-	JabberCapsKey *key = g_new0(JabberCapsKey, 1);
-	jabber_caps_cbplususerdata *userdata = g_new0(jabber_caps_cbplususerdata, 1);
-	userdata->cb = cb;
-	userdata->cb_data = user_data;
-	userdata->who = g_strdup(who);
-	userdata->node = g_strdup(node);
-	userdata->ver = g_strdup(ver);
-	userdata->hash = g_strdup(hash);
+	JabberCapsClientInfo *info;
+	JabberCapsKey key;
+
+	/* Using this in a read-only fashion, so the cast is OK */
+	key.node = (char *)node;
+	key.ver = (char *)ver;
+	key.hash = (char *)hash;
 
-	key->node = g_strdup(node);
-	key->ver = g_strdup(ver);
-	key->hash = g_strdup(hash);
-	
-	client = g_hash_table_lookup(capstable, key);
-	
-	if(!client) {
-		JabberIq *iq = jabber_iq_new_query(js,JABBER_IQ_GET,"http://jabber.org/protocol/disco#info");
-		xmlnode *query = xmlnode_get_child_with_namespace(iq->node,"query","http://jabber.org/protocol/disco#info");
-		char *nodever = g_strdup_printf("%s#%s", node, ver);
+	info = g_hash_table_lookup(capstable, &key);
+	if (info) {
+		jabber_caps_client_info_ref(info);
+		cb(info, user_data);
+	} else {
+		jabber_caps_cbplususerdata *userdata;
+		JabberIq *iq;
+		xmlnode *query;
+		char *nodever;
+
+		userdata = g_new0(jabber_caps_cbplususerdata, 1);
+		userdata->cb = cb;
+		userdata->cb_data = user_data;
+		userdata->who = g_strdup(who);
+		userdata->node = g_strdup(node);
+		userdata->ver = g_strdup(ver);
+		userdata->hash = g_strdup(hash);
+
+		iq = jabber_iq_new_query(js, JABBER_IQ_GET,
+					"http://jabber.org/protocol/disco#info");
+		query = xmlnode_get_child_with_namespace(iq->node, "query",
+					"http://jabber.org/protocol/disco#info");
+		nodever = g_strdup_printf("%s#%s", node, ver);
 		xmlnode_set_attrib(query, "node", nodever);
 		g_free(nodever);
 		xmlnode_set_attrib(iq->node, "to", who);
@@ -538,6 +561,8 @@
 		jabber_iq_send(iq);
 	}
 #if 0
+	/* The above check was originally simply "if (!info)", so this was executed
+	 * on info being non-null */
 	} else {
 		GList *iter;
 		/* fetch unknown exts only */
@@ -651,7 +676,8 @@
 		return 0;
 	
 	info = g_new0(JabberCapsClientInfo, 1);
-	
+	jabber_caps_client_info_ref(info);
+
 	for(child = query->child; child; child = child->next) {
 		if (!strcmp(child->name,"identity")) {
 			/* parse identity */
--- a/libpurple/protocols/jabber/caps.h	Sun Dec 14 21:19:02 2008 +0000
+++ b/libpurple/protocols/jabber/caps.h	Mon Dec 15 20:43:02 2008 +0000
@@ -32,8 +32,17 @@
 	GList *identities; /* JabberIdentity */
 	GList *features; /* char * */
 	GList *forms; /* xmlnode * */
+	guint ref;
 };
 
+/**
+ * Adjust the refcount for JabberCapsClientInfo. When the refcount reaches
+ * 0, the data will be destroyed.
+ */
+void jabber_caps_client_info_unref(JabberCapsClientInfo *info);
+void jabber_caps_client_info_ref(JabberCapsClientInfo *info);
+
+
 #if 0
 typedef struct _JabberCapsClientInfo JabberCapsValueExt;
 #endif
@@ -46,7 +55,10 @@
 void jabber_caps_destroy_key(gpointer value);
 
 /**
- *	Main entity capabilites function to get the capabilities of a contact.
+ * Main entity capabilites function to get the capabilities of a contact.
+ *
+ * The callback will be called synchronously if we already have the capabilities for
+ * the specified (node,ver,hash).
  */
 void jabber_caps_get_info(JabberStream *js, const char *who, const char *node, const char *ver, const char *hash, jabber_caps_get_info_cb cb, gpointer user_data);
 
--- a/libpurple/protocols/jabber/presence.c	Sun Dec 14 21:19:02 2008 +0000
+++ b/libpurple/protocols/jabber/presence.c	Mon Dec 15 20:43:02 2008 +0000
@@ -395,7 +395,7 @@
 		return;
 	}
 
-	/* old value in jbr->caps is owned by caps code */
+	jabber_caps_client_info_unref(jbr->caps);
 	jbr->caps = info;
 
 	if (jabber_resource_has_capability(jbr, "http://jabber.org/protocol/commands")) {