changeset 3752:b32474e522fa

[gaim-migrate @ 3890] From: "William T. Mahan" <[e-mail]> This patch, against CVS HEAD, fixes three bugs in Oscar File Transfer support. I can split it up further if desired. * Send a null checksum when initiating a file transfer, which fixes "files don't match" warnings produced by some versions of WinAIM; add a compile-time option to actually compute the checksum, which is slow but necessary when sending to some Mac clients. * Don't allow sending files to oneself, because it causes all kinds of subtle problems and it's not useful. * Don't crash when there is an error writing to the output file when receiving. From: "William T. Mahan" <[e-mail]> This patch 2 of 3, which applies on top of the first, adds support for reverse connections for Oscar File Transfer, the lack of which has been the biggest complaint so far. Reverse connections are used by newer AIM clients when there is difficulty verifying the IP of the sender. From: "William T. Mahan" <[e-mail]> This patch 3 of 3, which applies on top of the first 2, removes the alarm() and sigaction() calls that were added by my original FT patch to detect transfer timeouts. Besides apparently not working on Windows, they involved a lot of ugly code to handle a special case. My new approach is to add destructors that can called when SNACs are freed; a timeout is detected when a request SNAC is cleaned up before the transfer is accepted. Although this touches several files, it is more generic than the old method. I tried to implement this in an unintrusive manner, so that there is little preformance penalty for SNACs that do not use destructors. My first two patches should work fine without this. If there are any objections to the third patch, I ask that the first two patches be applied, in which case I will set up a SourceForge page for this one. committer: Tailor Script <[e-mail]>

--- a/src/protocols/oscar/aim_cbtypes.h	Sat Oct 19 05:04:58 2002 +0000
+++ b/src/protocols/oscar/aim_cbtypes.h	Sat Oct 19 05:22:30 2002 +0000
@@ -269,8 +269,11 @@
 #define AIM_CB_SPECIAL_FLAPVER 0x0005
 #define AIM_CB_SPECIAL_CONNINITDONE 0x0006
 #define AIM_CB_SPECIAL_IMAGETRANSFER 0x007
+#define AIM_CB_SPECIAL_MSGTIMEOUT 0x008
 #define AIM_CB_SPECIAL_UNKNOWN 0xffff
 #define AIM_CB_SPECIAL_DEFAULT AIM_CB_SPECIAL_UNKNOWN
 
+/* SNAC flags */
+#define AIM_SNACFLAGS_DESTRUCTOR 0x0001
 
 #endif/*__AIM_CBTYPES_H__ */

--- a/src/protocols/oscar/aim_internal.h	Sat Oct 19 05:04:58 2002 +0000
+++ b/src/protocols/oscar/aim_internal.h	Sat Oct 19 05:22:30 2002 +0000
@@ -24,6 +24,8 @@
 	fu16_t flags;
 	char name[AIM_MODULENAME_MAXLEN+1];
 	int (*snachandler)(aim_session_t *sess, struct aim_module_s *mod, aim_frame_t *rx, aim_modsnac_t *snac, aim_bstream_t *bs);
+	int (*snacdestructor)(aim_session_t *sess, aim_conn_t *conn, aim_modsnac_t *snac, void *data);
+
 	void (*shutdown)(aim_session_t *sess, struct aim_module_s *mod);
 	void *priv;
 	struct aim_module_s *next;
@@ -114,6 +116,11 @@
 	struct aim_snac_s *next;
 } aim_snac_t;
 
+struct aim_snac_destructor {
+	aim_conn_t *conn;
+	void *data;
+};
+
 faim_internal void aim_initsnachash(aim_session_t *sess);
 faim_internal aim_snacid_t aim_newsnac(aim_session_t *, aim_snac_t *newsnac);
 faim_internal aim_snacid_t aim_cachesnac(aim_session_t *sess, const fu16_t family, const fu16_t type, const fu16_t flags, const void *data, const int datalen);

--- a/src/protocols/oscar/ft.c	Sat Oct 19 05:04:58 2002 +0000
+++ b/src/protocols/oscar/ft.c	Sat Oct 19 05:22:30 2002 +0000
@@ -1622,7 +1622,7 @@
 
 	faimdprintf(sess, 2, "faim: get_rend: looks like we're done with a transfer (oft 0x0204)\n");
 
-	debug_printf("wtm: they sent csum %x\n", fh->recvcsum);
+	debug_printf("sendfile: receiver sent checksum %x\n", fh->recvcsum);
 
 	if ( (userfunc = aim_callhandler(sess, conn, AIM_CB_FAM_OFT, AIM_CB_OFT_GETFILECOMPLETE)) )
 		userfunc(sess, NULL, conn, fh->bcookie);
@@ -2151,16 +2151,33 @@
   if (!sess || !conn || !name)
     return -1;
 
-  /* Authenticate whomever connected to our listener by checking
-   * that the correct cookie was proivided. */
-  if (!(cook = aim_checkcookie(sess, ft->cookie,
-				  AIM_COOKIETYPE_OFTSEND))) {
-	  return -1;
-  }
-
   if (!(fh = (struct aim_fileheader_t*)calloc(1, sizeof(struct aim_fileheader_t))))
     return -1;
 
+#ifdef DUMB_OFT_CHECKSUM
+  /* Yes, we are supposed to checksum the whole file before sending, and
+   * yes, it's dumb.  This is the only way to get some clients (such as
+   * Mac AIM v4.5.163) to successfully complete the transfer.  With
+   * the WinAIM clients, we seem to be able to get away with just
+   * setting the checksum to zero.
+   * -- wtm
+   */
+  {
+    int fd = open(name, O_RDONLY);
+    if (fd >= 0) {
+       int bytes;
+       char buf[1024];
+       fh->checksum = 0xffff0000;
+       while ((bytes = read(fd, buf, 1024)) > 0) {
+         fh->checksum = aim_oft_checksum(buf, bytes, fh->checksum);
+       }
+    }
+    close(fd);
+  }
+#else
+  fh->checksum    = 0x00000000;
+#endif
+
   fh->encrypt     = 0x0000;
   fh->compress    = 0x0000; 
   fh->totfiles    = numfiles;
@@ -2170,7 +2187,7 @@
   fh->totsize     = totsize;
   fh->size        = size;
   fh->modtime     = (int)time(NULL); /* we'll go with current time for now */
-  fh->checksum    = 0xffffffff; /* XXX: checksum ! */
+  /* fh->checksum set above */
   fh->rfcsum      = 0x00000000;
   fh->rfsize      = 0x00000000;
   fh->cretime     = 0x00000000;
@@ -2198,7 +2215,15 @@
 	  g_free(basename);
   }
 
-  memcpy(fh->bcookie, cook->cookie, 8);
+  /* XXX we should normally send a null cookie here, and make
+   * the receiver fill it in for authentication -- wtm
+   */
+  memcpy(fh->bcookie, ft->cookie, 8);
+
+  if (!(cook = aim_checkcookie(sess, ft->cookie, AIM_COOKIETYPE_OFTSEND))) {
+    return -1;
+  }
+
   /* Update both headers to be safe. */
   memcpy(&(ft->fh), fh, sizeof(struct aim_fileheader_t));
   memcpy(&(((struct aim_filetransfer_priv *)cook->data)->fh), fh,

--- a/src/protocols/oscar/im.c	Sat Oct 19 05:04:58 2002 +0000
+++ b/src/protocols/oscar/im.c	Sat Oct 19 05:22:30 2002 +0000
@@ -654,6 +654,7 @@
 	fu8_t ck[8];
 	aim_frame_t *fr;
 	aim_snacid_t snacid;
+	struct aim_snac_destructor snacdest;
 
 	if (!sess || !(conn = aim_conn_findbygroup(sess, 0x0004)))
 		return -EINVAL;
@@ -664,9 +665,6 @@
 	if (!(fr = aim_tx_new(sess, conn, AIM_FRAMETYPE_FLAP, 0x02, 10+8+2+1+strlen(sn)+2+2+2+8+16+6+8+6+4+2+2+2+2+4+strlen(filename)+4)))
 		return -ENOMEM;
 
-	snacid = aim_cachesnac(sess, 0x0004, 0x0006, 0x0000, NULL, 0);
-	aim_putsnac(&fr->data, 0x0004, 0x0006, 0x0000, snacid);
-
 	for (i = 0; i < 7; i++)
 		aimutil_put8(ck+i, 0x30 + ((fu8_t) rand() % 10));
 	ck[7] = '\0';
@@ -674,6 +672,16 @@
 	if (ckret)
 		memcpy(ckret, ck, 8);
 
+	/* Fill in the snac destructor so we know if the request
+	 * times out.  Use the cookie in the data field, so we
+	 * know what request to cancel if there is an error.
+	 */
+	snacdest.data = malloc(8);
+	memcpy(snacdest.data, ck, 8);
+	snacdest.conn = conn;
+	snacid = aim_cachesnac(sess, 0x0004, 0x0006, AIM_SNACFLAGS_DESTRUCTOR, &snacdest, sizeof(snacdest));
+	aim_putsnac(&fr->data, 0x0004, 0x0006, 0x0000, snacid);
+
 	/*
 	 * Cookie
 	 */
@@ -735,7 +743,7 @@
 	aimbs_put32(&fr->data, 0x00000000);
 
 #if 0
-	/* from brian's patch (?) -- wtm */
+	/* Newer clients seem to send this (?) -- wtm */
 	aimbs_put32(&fr->data, 0x00030000);
 #endif
 
@@ -2218,6 +2226,20 @@
 	return 0;
 }
 
+static int snacdestructor(aim_session_t *sess, aim_conn_t *conn, aim_modsnac_t *snac, void *data)
+{
+	aim_rxcallback_t userfunc;
+	int ret = 0;
+
+	if (snac->subtype == 0x0006) {
+		if ((userfunc = aim_callhandler(sess, conn, AIM_CB_FAM_SPECIAL, AIM_CB_SPECIAL_MSGTIMEOUT)))
+			ret = userfunc(sess, NULL, conn, data);
+	}
+	/* Note that we return 1 for success, 0 for failure. */
+	return ret;
+}
+
+
 faim_internal int msg_modfirst(aim_session_t *sess, aim_module_t *mod)
 {
 
@@ -2228,6 +2250,7 @@
 	mod->flags = 0;
 	strncpy(mod->name, "messaging", sizeof(mod->name));
 	mod->snachandler = snachandler;
+	mod->snacdestructor = snacdestructor;
 
 	return 0;
 }

--- a/src/protocols/oscar/oscar.c	Sat Oct 19 05:04:58 2002 +0000
+++ b/src/protocols/oscar/oscar.c	Sat Oct 19 05:22:30 2002 +0000
@@ -189,8 +189,6 @@
 	int watcher;
 };
 
-static struct oscar_file_transfer *oft_listening;
-
 struct icon_req {
 	char *user;
 	time_t timestamp;
@@ -383,6 +381,7 @@
 		struct file_transfer *);
 static int oscar_sendfile_request(aim_session_t *sess,
 		struct oscar_file_transfer *oft);
+static int oscar_sendfile_timeout(aim_session_t *sess, aim_frame_t *fr, ...);
 
 static char *msgerrreason[] = {
 	"Invalid error",
@@ -761,29 +760,16 @@
 
 static void oscar_ask_send_file(struct gaim_connection *gc, char *destsn) {
 	struct oscar_data *od = (struct oscar_data *)gc->proto_data;
-	struct oscar_file_transfer *oft = oft_listening;
-
-	/* Kludge:  if we try to send a file to a client that doesn't
-	 * support it, the BOS server sends us back an error without
-	 * any information identifying which transfer was aborted.  So
-	 * we only allow one sendfile request at a time, to ensure that
-	 * the transfer referenced by an error is unambiguous.  It's ugly,
-	 * but what else can we do? -- wtm
-	 */
-	if (oft) {
-		do_error_dialog(_("Sorry, you already have an outgoing transfer pending.  Due to limitations of the Oscar protocol, only one outgoing transfer request is permitted at a time."), NULL, GAIM_ERROR);
-	}
-	else {
-		struct oscar_file_transfer *oft = g_new0(struct oscar_file_transfer,
-				1);
-
-		oft->type = OFT_SENDFILE_OUT;
-		oft->sn = g_strdup(destsn);
-
-		od->file_transfers = g_slist_append(od->file_transfers, oft);
-
-		oft->xfer = transfer_out_add(gc, oft->sn);
-	}
+
+	struct oscar_file_transfer *oft = g_new0(struct oscar_file_transfer,
+			1);
+
+	oft->type = OFT_SENDFILE_OUT;
+	oft->sn = g_strdup(destsn);
+
+	od->file_transfers = g_slist_append(od->file_transfers, oft);
+
+	oft->xfer = transfer_out_add(gc, oft->sn);
 }
 
 static int gaim_parse_auth_resp(aim_session_t *sess, aim_frame_t *fr, ...) {
@@ -892,6 +878,7 @@
 	aim_conn_addhandler(sess, bosconn, AIM_CB_FAM_SSI, AIM_CB_SSI_RIGHTSINFO, gaim_ssi_parserights, 0);
 	aim_conn_addhandler(sess, bosconn, AIM_CB_FAM_SSI, AIM_CB_SSI_LIST, gaim_ssi_parselist, 0);
 	aim_conn_addhandler(sess, bosconn, AIM_CB_FAM_SSI, AIM_CB_SSI_NOLIST, gaim_ssi_parselist, 0);
+	aim_conn_addhandler(sess, bosconn, AIM_CB_FAM_SPECIAL, AIM_CB_SPECIAL_MSGTIMEOUT, oscar_sendfile_timeout, 0);
 
 	((struct oscar_data *)gc->proto_data)->conn = bosconn;
 	for (i = 0; i < (int)strlen(info->bosip); i++) {
@@ -1540,9 +1527,7 @@
 	struct oscar_file_transfer *oft;
 	va_list ap;
 	aim_conn_t *conn, *listenerconn;
-#ifndef NOSIGALARM
-	alarm(0); /* reset timeout alarm */
-#endif
+
 	va_start(ap, fr);
 	conn = va_arg(ap, aim_conn_t *);
 	listenerconn = va_arg(ap, aim_conn_t *);
@@ -1553,8 +1538,6 @@
 	/* Stop watching listener conn; watch transfer conn instead */
 	gaim_input_remove(oft->watcher);
 	aim_conn_kill(sess, &listenerconn);
-	/* We no longer need to block other outgoing transfers. */
-	oft_listening = NULL;
 
 	aim_conn_addhandler(od->sess, oft->conn, AIM_CB_FAM_OFT,
 			AIM_CB_OFT_GETFILEFILESEND,
@@ -1573,27 +1556,27 @@
 	return 0;
 }
 
-void oscar_sendfile_timeout(int sig)
-{
-	struct oscar_file_transfer *oft = oft_listening;
-
-	if (oft) {
-		aim_session_t *sess = aim_conn_getsess(oft->conn);
-		aim_conn_t *bosconn;
-		{
-			/* XXX is this valid? is there a better way? -- wtm */
-			struct gaim_connection *gc = sess->aux_data;
-			struct oscar_data *odata = (struct oscar_data *)gc->proto_data;
-			bosconn = odata->conn;
-		}
-
-		oft_listening = NULL;
+static int oscar_sendfile_timeout(aim_session_t *sess, aim_frame_t *fr, ...) {
+	struct gaim_connection *gc = sess->aux_data;
+	va_list ap;
+	struct oscar_file_transfer *oft;
+	char *cookie;
+	aim_conn_t *bosconn;
+
+	va_start(ap, fr);
+	bosconn = va_arg(ap, aim_conn_t *);
+	cookie = va_arg(ap, char *);
+	va_end(ap);
+
+	if ((oft = find_oft_by_cookie(gc, cookie))) {
 		aim_canceltransfer(sess, bosconn, oft->cookie,
 				oft->sn, AIM_CAPS_SENDFILE);
 
 		transfer_abort(oft->xfer, _("Transfer timed out"));
 		oscar_file_transfer_disconnect(sess, oft->conn);
 	}
+
+	return 1; /* success */
 }
 
 /* Called once at the beginning of an outgoing transfer session. */
@@ -1607,7 +1590,6 @@
 	oft->totsize = totsize;
 	oft->totfiles = totfiles;
 	oft->filesdone = 0;
-	oft_listening = oft;
 
 	oft->conn = aim_sendfile_initiate(od->sess, oft->sn,
 			name, totfiles, oft->totsize, oft->cookie);
@@ -1617,18 +1599,7 @@
 				GAIM_ERROR);
 		return;
 	}
-#ifndef NOSIGALARM
-	{
-		/* XXX is there a good glib-oriented way of doing this?
-		 * -- wtm */
-		struct sigaction act;
-		act.sa_handler = oscar_sendfile_timeout;
-		act.sa_flags = SA_RESETHAND;
-		sigemptyset (&act.sa_mask);
-		sigaction(SIGALRM, &act, NULL);
-		alarm(OFT_TIMEOUT);
-	}
-#endif
+
 	aim_conn_addhandler(od->sess, oft->conn, AIM_CB_FAM_OFT,
 			AIM_CB_OFT_GETFILEINITIATE,
 			oscar_sendfile_accepted,
@@ -1860,6 +1831,7 @@
 		return 0;
 	}
 	else if (args->status != AIM_RENDEZVOUS_PROPOSE) {
+		debug_printf("unknown rendezvous status\n");
 		return 1;
 	}
 
@@ -1879,20 +1851,65 @@
 			g_free(name);
 	} else if (args->reqclass & AIM_CAPS_SENDFILE) {
 		struct oscar_file_transfer *oft;
-
-		if (!args->verifiedip) {
-			/* It seems that Trillian sends a message
-			 * with no file data during multiple-file
-			 * transfers.
+		struct oscar_data *od = gc->proto_data;
+
+		if ((oft = find_oft_by_cookie(sess->aux_data, args->cookie)))
+		{
+			/* This is a request for a reverse connection,
+			 * which is used by newer clients when for some
+			 * reason they are unable to connect to our listener
+			 * (e.g. they are behind a firewall).
+			 */
+			if (oft->type != OFT_SENDFILE_OUT)
+				return -1;
+
+			/* It seems that Trillian sends some weird
+			 * packets.  Sanity check.
+			 */
+			if (!args->verifiedip)
+				return -1;
+
+			/* This connection isn't used for anything, since
+			 * we're using a reverse connection instead.
 			 */
-			debug_printf("sendfile: didn't get any data\n");
-			return -1;
+			gaim_input_remove(oft->watcher);
+			aim_conn_kill(sess, &oft->conn);
+
+			debug_printf("sendfile: doing reverse connection to %s:%d\n", args->verifiedip, args->port);
+
+			oft->conn = aim_accepttransfer(sess, od->conn,
+				userinfo->sn,
+				args->cookie, args->verifiedip,
+				args->port,
+				AIM_CAPS_SENDFILE);
+
+			/* XXX: this is a bit of a hack: ideally
+			 * we should wait on GAIM_INPUT_WRITE. -- wtm
+			 */
+			aim_conn_completeconnect(sess, oft->conn);
+
+			oscar_sendfile_request(sess, oft);
+
+			aim_conn_addhandler(sess, oft->conn,
+				AIM_CB_FAM_OFT,
+				AIM_CB_OFT_GETFILECOMPLETE,
+				oscar_sendfile_out_done,
+				0);
+			aim_conn_addhandler(sess, oft->conn,
+				AIM_CB_FAM_OFT,
+				AIM_CB_OFT_GETFILEFILESEND,
+				oscar_file_transfer_do,
+				0);
+			oft->watcher = gaim_input_add(oft->conn->fd,
+				GAIM_INPUT_READ, oscar_callback,
+				oft->conn);
+			return 0;
 		}
 
-		oft = g_new0(struct oscar_file_transfer, 1);
-
 		debug_printf("%s (%s) requests to send a file to %s\n",
 				userinfo->sn, args->verifiedip, gc->username);
+
+		oft = g_new0(struct oscar_file_transfer, 1);
 		
 		oft->type = OFT_SENDFILE_IN;
 		oft->sn = g_strdup(userinfo->sn);
@@ -1900,11 +1917,7 @@
 		oft->port = args->port;
 		memcpy(oft->cookie, args->cookie, 8);
 
-		{ /* XXX ugly... */
-			struct gaim_connection *gc = sess->aux_data;
-			struct oscar_data *od = gc->proto_data;
-			od->file_transfers = g_slist_append(od->file_transfers, oft);
-		}
+		od->file_transfers = g_slist_append(od->file_transfers, oft);
 
 		oft->xfer = transfer_in_add(gc, userinfo->sn, 
 				args->info.sendfile.filename,
@@ -2224,10 +2237,6 @@
 			if (oft) {
 				buf = g_strdup_printf(_("%s has declined to receive a file from %s.\n"),
 						who, gc->username);
-#ifndef NOSIGALARM
-				alarm(0); /* reset timeout alarm */
-#endif
-				oft_listening = NULL;
 				transfer_abort(oft->xfer, buf);
 				g_free(buf);
 				oscar_file_transfer_disconnect(sess, oft->conn);
@@ -2328,23 +2337,19 @@
 
 static int gaim_parse_msgerr(aim_session_t *sess, aim_frame_t *fr, ...) {
 	va_list ap;
-	char *destn;
+	char *data;
 	fu16_t reason;
 	char buf[1024];
-	struct oscar_file_transfer *oft = oft_listening;
-
+	struct gaim_connection *gc = sess->aux_data;
+	struct oscar_file_transfer *oft;
+	
 	va_start(ap, fr);
 	reason = (fu16_t)va_arg(ap, unsigned int);
-	destn = va_arg(ap, char *);
+	data = va_arg(ap, char *);
 	va_end(ap);
 
-	if (oft) {
-		/* If we try to send a file but it isn't supported, then
-		 * we get this error without any information identifying
-		 * the failed connection.  So we can only have one outgoing
-		 * transfer at a time.  Ugh. -- wtm
-		 */
-		oft_listening = NULL;
+	/* If this was a file transfer request, data is a cookie. */
+	if ((oft = find_oft_by_cookie(gc, data))) {
 		transfer_abort(oft->xfer,
 				(reason < msgerrreasonlen) ? msgerrreason[reason] : _("No reason was given."));
 
@@ -2352,7 +2357,8 @@
 		return 1;
 	}
 
-	snprintf(buf, sizeof(buf), _("Your message to %s did not get sent:"), destn);
+	/* Data is assumed to be the destination sn. */
+	snprintf(buf, sizeof(buf), _("Your message to %s did not get sent:"), data);
 	do_error_dialog(buf, (reason < msgerrreasonlen) ? msgerrreason[reason] : _("No reason was given."), GAIM_ERROR);
 
 	return 1;
@@ -4165,13 +4171,9 @@
 		oscar_file_transfer_disconnect(sess, conn);
 	}
 	else if (oft->type == OFT_SENDFILE_OUT) { 
-#if 0
 		/* Wait for response before closing connection. */
 		oft->watcher = gaim_input_add(conn->fd, GAIM_INPUT_READ,
 				oscar_callback, conn);
-#else	
-		oscar_file_transfer_disconnect(sess, conn);
-#endif
 	}
 }
 
@@ -4180,6 +4182,7 @@
 	va_list ap;
 	aim_conn_t *conn;
 	struct oscar_file_transfer *oft;
+	int err;
 
 	va_start(ap, fr);
 	conn = va_arg(ap, aim_conn_t *);
@@ -4193,16 +4196,23 @@
 	if (oft->type == OFT_SENDFILE_IN) {
 		const char *name = va_arg(ap, const char *);
 		int size = va_arg(ap, int);
-		if (transfer_in_do(oft->xfer, conn->fd, name, size))
-			oscar_file_transfer_disconnect(sess, oft->conn);
+		err = transfer_in_do(oft->xfer, conn->fd, name, size);
 	}
 	else {
 		int offset = va_arg(ap, int);
-		if (transfer_out_do(oft->xfer, conn->fd, offset))
-			oscar_file_transfer_disconnect(sess, oft->conn);
+		err = transfer_out_do(oft->xfer, conn->fd, offset);
 	}
 	va_end(ap);
 
+	if (err) {
+		/* There was an error; cancel the transfer. */
+		struct oscar_data *od = (struct oscar_data *)gc->proto_data;
+		aim_conn_t *bosconn = od->conn;
+		aim_canceltransfer(sess, bosconn, oft->cookie,
+			oft->sn, AIM_CAPS_SENDFILE);
+		oscar_file_transfer_disconnect(sess, oft->conn);
+	}
+
 	return 0;
 }
 
@@ -4431,13 +4441,13 @@
 			pbm->callback = oscar_ask_direct_im;
 			pbm->gc = gc;
 			m = g_list_append(m, pbm);
+		
+			pbm = g_new0(struct proto_buddy_menu, 1);
+			pbm->label = _("Send File");
+			pbm->callback = oscar_ask_send_file;
+			pbm->gc = gc;
+			m = g_list_append(m, pbm);
 		}
-			
-		pbm = g_new0(struct proto_buddy_menu, 1);
-		pbm->label = _("Send File");
-		pbm->callback = oscar_ask_send_file;
-		pbm->gc = gc;
-		m = g_list_append(m, pbm);
 	}
 
 	pbm = g_new0(struct proto_buddy_menu, 1);

--- a/src/protocols/oscar/snac.c	Sat Oct 19 05:04:58 2002 +0000
+++ b/src/protocols/oscar/snac.c	Sat Oct 19 05:22:30 2002 +0000
@@ -89,6 +89,11 @@
 	for (prev = (aim_snac_t **)&sess->snac_hash[index]; (cur = *prev); ) {
 		if (cur->id == id) {
 			*prev = cur->next;
+			if (cur->flags & AIM_SNACFLAGS_DESTRUCTOR) {
+				struct aim_snac_destructor *asd = cur->data;
+				cur->data = asd->data;
+				free(asd);
+			}
 			return cur;
 		} else
 			prev = &cur->next;
@@ -97,6 +102,42 @@
 	return cur;
 }
 
+/* Free a SNAC, and call the appropriate destructor if necessary.
+ * XXX perhaps this should be inline? -- wtm
+ */
+faim_internal void aim_cleansnac(aim_session_t *sess, aim_snac_t *snac)
+{
+	aim_module_t *cur;
+
+	if (snac->flags & AIM_SNACFLAGS_DESTRUCTOR) {
+		struct aim_snac_destructor *d = snac->data;
+		aim_modsnac_t modsnac;
+
+		modsnac.id = snac->id;
+		modsnac.subtype = snac->type;
+		modsnac.family = snac->family;
+		modsnac.flags = snac->flags;
+
+		for (cur = (aim_module_t *)sess->modlistv; cur; cur = cur->next)
+		{
+			if (!cur->snacdestructor)
+				continue;
+			if (!(cur->flags & AIM_MODFLAG_MULTIFAMILY) &&
+				(cur->family != modsnac.family))
+				continue;
+			if (cur->snacdestructor(sess, d->conn, &modsnac,
+						d->data))
+				break;
+		}
+		free(d->data);
+		free(d);
+	}
+
+	free(snac->data);
+	free(snac);
+}
+
+
 /*
  * This is for cleaning up old SNACs that either don't get replies or
  * a reply was never received for.  Garabage collection. Plain and simple.
@@ -122,10 +163,7 @@
 
 				*prev = cur->next;
 
-				/* XXX should we have destructors here? */
-				free(cur->data);
-				free(cur);
-
+				aim_cleansnac(sess, cur);
 			} else
 				prev = &cur->next;
 		}