cpw.darkrain42.xmpp.bosh: b8fdbd25: Reference-count JabberCapsClientInfo and...

paul at darkrain42.org paul at darkrain42.org
Sat Jan 17 23:56:44 EST 2009


-----------------------------------------------------------------
Revision: b8fdbd255c614e7305f835b843a3414675a86a19
Ancestor: c2becef0773a1d9961b7106623b08b021add46b7
Author: paul at darkrain42.org
Date: 2008-12-15T20:43:02
Branch: im.pidgin.cpw.darkrain42.xmpp.bosh
URL: http://d.pidgin.im/viewmtn/revision/info/b8fdbd255c614e7305f835b843a3414675a86a19

Modified 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

ChangeLog: 

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.

-------------- next part --------------
============================================================
--- libpurple/protocols/jabber/buddy.c	16f772dc4ac4a325af5d583b2529c183612649c6
+++ libpurple/protocols/jabber/buddy.c	03d8333c21099668ebf9ee4c399d57cc12e0fd95
@@ -181,7 +181,7 @@ void jabber_buddy_resource_free(JabberBu
 		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);
============================================================
--- libpurple/protocols/jabber/buddy.h	778f1657f43e94ca6df7495886973a9aad3a526f
+++ libpurple/protocols/jabber/buddy.h	b72ccdfffb3ac98b1f2cab34a3fddfef59c086e5
@@ -81,7 +81,7 @@ typedef struct _JabberBuddyResource {
 		char *name;
 		char *os;
 	} client;
-	const JabberCapsClientInfo *caps;
+	JabberCapsClientInfo *caps;
 	GList *commands;
 } JabberBuddyResource;
 
============================================================
--- libpurple/protocols/jabber/caps.c	2ef41ffc69157622becc1d72d69bebc599df4bb4
+++ libpurple/protocols/jabber/caps.c	c36f46eb22f57f253c118411b10cadaf4b705e9f
@@ -84,9 +84,24 @@ void jabber_caps_destroy_key(gpointer ke
 	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 @@ jabber_caps_client_info_destroy(gpointer
 		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)
 
 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 @@ static void jabber_caps_load(void) {
 			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 @@ jabber_caps_client_iqcb(JabberStream *js
 		                     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 @@ jabber_caps_client_iqcb(JabberStream *js
 	/* 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 @@ jabber_caps_client_iqcb(JabberStream *js
 	}
 
 	userdata->cb(info, userdata->cb_data);
-	/* capstable owns info */
 
 out:
 	g_free(userdata->who);
@@ -510,26 +522,37 @@ void jabber_caps_get_info(JabberStream *
 		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;
 
-	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);
+	/* Using this in a read-only fashion, so the cast is OK */
+	key.node = (char *)node;
+	key.ver = (char *)ver;
+	key.hash = (char *)hash;
+
+	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 @@ void jabber_caps_get_info(JabberStream *
 		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 @@ static JabberCapsClientInfo *jabber_caps
 		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 */
============================================================
--- libpurple/protocols/jabber/caps.h	3e4ab804c60f11e9e1d24d79762d92062d6c910e
+++ libpurple/protocols/jabber/caps.h	d3e913179bd9674ddafb54f4d3faeae41a2bc1fe
@@ -32,8 +32,17 @@ struct _JabberCapsClientInfo {
 	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 va
 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);
 
============================================================
--- libpurple/protocols/jabber/presence.c	03d8f295297d0866368dabbc60373ce942a5f468
+++ libpurple/protocols/jabber/presence.c	a161dc405a07a57da27e49599a6ca6c6c340bf81
@@ -395,7 +395,7 @@ static void jabber_presence_set_capabili
 		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")) {


More information about the Commits mailing list