pidgin: 66e717ad: Does anyone know the purpose of the ui_w...

[markdoliner at pidgin.im]

-----------------------------------------------------------------
Revision: 66e717addcde9a68da52753e7c5767e576c8816d
Ancestor: b9e0791c0283b12619fd4f4bd45400dff5498b87
Author: markdoliner at pidgin.im
Date: 2010-04-06T09:52:27
Branch: im.pidgin.pidgin
URL: http://d.pidgin.im/viewmtn/revision/info/66e717addcde9a68da52753e7c5767e576c8816d

Modified files:
        libpurple/ft.c

ChangeLog: 

Does anyone know the purpose of the ui_write, ui_read and data_not_sent
FT UI callbacks?  It looks like they allow a UI to decide how to safe
incoming file transfers?  They're not used in Pidgin or Finch.  Are
they used elsewhere?

Valgrind is complaining about an invalid free.  I think it happens either
when the local user cancels a file transfer or when the remote user
cancels a file transfer.  I think this change fixes it.

Revision fa4ce539e5025eb07aad3ca824cd4c512010d8a8 is related to these
callbacks and to this change by foufou33@ gee male dot com

The valgrind error is:
==23064== Invalid free() / delete / delete[]
==23064==    at 0x4C24D68: free (vg_replace_malloc.c:325)
==23064==    by 0x9293209: g_array_free (in /lib/libglib-2.0.so.0.2200.3)
==23064==    by 0x95B1995: purple_xfer_priv_data_destroy (ft.c:71)
==23064==    by 0x92AA5D1: ??? (in /lib/libglib-2.0.so.0.2200.3)
==23064==    by 0x92AAE17: g_hash_table_remove_all (in /lib/libglib-2.0.so.0.2200.3)
==23064==    by 0x92AAFC4: g_hash_table_destroy (in /lib/libglib-2.0.so.0.2200.3)
==23064==    by 0x95B579C: purple_xfers_uninit (ft.c:1642)
==23064==    by 0x95ACF08: purple_core_quit (core.c:238)
==23064==    by 0x43EB3E: gtk_blist_delete_cb (gtkblist.c:227)
==23064==    by 0x6F9A727: ??? (in /usr/lib/libgtk-x11-2.0.so.0.1800.3)
==23064==  Address 0x2355f0e0 is not stack'd, malloc'd or (recently) free'd

-------------- next part --------------
============================================================
--- libpurple/ft.c	dd6ec238282cd71144147a7fd1370152e10ccdd5
+++ libpurple/ft.c	8e3a930f8e2a9b560d4f3f573c6c969df6cfb093
@@ -57,6 +57,8 @@ typedef struct _PurpleXferPrivData {
 		PURPLE_XFER_READY_UI   = 0x1,
 		PURPLE_XFER_READY_PRPL = 0x2,
 	} ready;
+
+	/* TODO: Should really use a PurpleCircBuffer for this. */
 	GByteArray *buffer;
 } PurpleXferPrivData;
 
@@ -1147,7 +1149,7 @@ do_transfer(PurpleXfer *xfer)
 		}
 
 		if (priv->buffer) {
-			priv->buffer = g_byte_array_append(priv->buffer, buffer, result);
+			g_byte_array_append(priv->buffer, buffer, result);
 			g_free(buffer);
 			buffer = priv->buffer->data;
 			result = priv->buffer->len;
@@ -1157,7 +1159,10 @@ do_transfer(PurpleXfer *xfer)
 
 		if (r == -1) {
 			purple_xfer_cancel_remote(xfer);
-			g_free(buffer);
+			if (!priv->buffer)
+				/* We don't free buffer if priv->buffer is set, because in
+				   that case buffer doesn't belong to us. */
+				g_free(buffer);
 			return;
 		} else if (r == result) {
 			/*
@@ -1175,10 +1180,10 @@ do_transfer(PurpleXfer *xfer)
 			/*
 			 * Remove what we wrote
 			 * If we wrote the whole buffer the byte array will be empty
-			 * Otherwise we'll kee what wasn't sent for next time.
+			 * Otherwise we'll keep what wasn't sent for next time.
 			 */
 			buffer = NULL;
-			priv->buffer = g_byte_array_remove_range(priv->buffer, 0, r);
+			g_byte_array_remove_range(priv->buffer, 0, r);
 		}
 	}