Fix for Novell bug #178631

2006-05-26  Jeffrey Stedfast  <fejj@novell.com>

	Fix for Novell bug #178631

	* e-attachment-bar.c: Changed the EAttachmentBarPrivate struct, we
	no longer use a linked list, instead we use a GPtrArray - faster,
	simpler (plus all the code used indexes anyway, so it was really
	bizarre).
	(free_attachment_list): Removed.
	(attachment_destroy): New GWeakNotify callback for when an
	EAttachment object gets destroyed. Remove the attachment from the
	attachments array.
	(add_common): Updated to add to an array rather than a linked
	list. Also weak_ref the attachment object.
	(remove_attachment): Removed.
	(update): Updated to use the array instead of linked list of
	attachments.
	(e_attachment_bar_remove_selected): Same.
	(e_attachment_bar_set_width): Same.
	(e_attachment_bar_edit_selected): Same.
	(e_attachment_bar_get_selected): Same.
	(e_attachment_bar_get_attachment): Same.
	(e_attachment_bar_get_all_attachments): Same and also optimised
	since we can cheat now without having to g_slist_reverse.
	(e_attachment_bar_get_parts): Same.
	(destroy): Same.
	(eab_drag_data_get): Same.
	(init): Init attachments to a g_ptr_array_new
	(e_attachment_bar_to_multipart): Updated to use the attachments
	array.
	(e_attachment_bar_get_num_attachments): Updated to return the
	attachments->len.
	(e_attachment_bar_get_download_count): Updated to use the array.

	* e-attachment.c (finalise): Close the editor dialog if it is
	open.
	(async_progress_update_cb): If the phase is COMPLETE but the
	file_size is 0, then treat it as an error. In the error case,
	unref the attachment object (this will magically remove it from
	the EAttachmentBar).
	(close_cb): Don't unref the attachment object here anymore.
	(e_attachment_edit): Don't ref the EAttachment anymore.

svn path=/trunk/; revision=32042

4 files changed, 451 insertions, 478 deletions

diff --git a/widgets/misc/ChangeLog b/widgets/misc/ChangeLog
index cd7f654ef3..e90f9b7375 100644
--- a/widgets/misc/ChangeLog
+++ b/widgets/misc/ChangeLog
@@ -1,3 +1,46 @@
+2006-05-26  Jeffrey Stedfast  <fejj@novell.com>
+
+	Fix for Novell bug #178631
+
+	* e-attachment-bar.c: Changed the EAttachmentBarPrivate struct, we
+	no longer use a linked list, instead we use a GPtrArray - faster,
+	simpler (plus all the code used indexes anyway, so it was really
+	bizarre).
+	(free_attachment_list): Removed.
+	(attachment_destroy): New GWeakNotify callback for when an
+	EAttachment object gets destroyed. Remove the attachment from the
+	attachments array.
+	(add_common): Updated to add to an array rather than a linked
+	list. Also weak_ref the attachment object.
+	(remove_attachment): Removed.
+	(update): Updated to use the array instead of linked list of
+	attachments.
+	(e_attachment_bar_remove_selected): Same.
+	(e_attachment_bar_set_width): Same.
+	(e_attachment_bar_edit_selected): Same.
+	(e_attachment_bar_get_selected): Same.
+	(e_attachment_bar_get_attachment): Same.
+	(e_attachment_bar_get_all_attachments): Same and also optimised
+	since we can cheat now without having to g_slist_reverse.
+	(e_attachment_bar_get_parts): Same.
+	(destroy): Same.
+	(eab_drag_data_get): Same.
+	(init): Init attachments to a g_ptr_array_new
+	(e_attachment_bar_to_multipart): Updated to use the attachments
+	array.
+	(e_attachment_bar_get_num_attachments): Updated to return the
+	attachments->len.
+	(e_attachment_bar_get_download_count): Updated to use the array.
+
+	* e-attachment.c (finalise): Close the editor dialog if it is
+	open.
+	(async_progress_update_cb): If the phase is COMPLETE but the
+	file_size is 0, then treat it as an error. In the error case,
+	unref the attachment object (this will magically remove it from
+	the EAttachmentBar).
+	(close_cb): Don't unref the attachment object here anymore.
+	(e_attachment_edit): Don't ref the EAttachment anymore.
+
 2006-05-25  Srinivasa Ragavan  <sragavan@novell.com>
 
 	* e-attachment-bar.c (update, eab_icon_clicked_cb),
diff --git a/widgets/misc/e-attachment-bar.c b/widgets/misc/e-attachment-bar.c
index b8dda048d5..02b78f1c76 100644
--- a/widgets/misc/e-attachment-bar.c
+++ b/widgets/misc/e-attachment-bar.c
@@ -70,10 +70,10 @@ static GnomeIconListClass *parent_class = NULL;
 
 struct _EAttachmentBarPrivate {
 	GtkWidget *attach;	/* attachment file dialogue, if active */
-
-	GList *attachments;
-	guint num_attachments;
-	gchar *path;
+	
+	gboolean batch_unref;
+	GPtrArray *attachments;
+	char *path;
 };
 
 
@@ -89,7 +89,7 @@ static void update (EAttachmentBar *bar);
 
 
 static char *
-size_to_string (gulong size)
+size_to_string (size_t size)
 {
 	char *size_string;
 	
@@ -120,17 +120,13 @@ size_to_string (gulong size)
 /* Attachment handling functions.  */
 
 static void
-free_attachment_list (EAttachmentBar *bar)
+attachment_destroy (EAttachmentBar *bar, EAttachment *attachment)
 {
-	EAttachmentBarPrivate *priv;
-	GList *p;
-	
-	priv = bar->priv;
-	
-	for (p = priv->attachments; p != NULL; p = p->next)
-		g_object_unref (p->data);
+	if (bar->priv->batch_unref)
+		return;
 	
-	priv->attachments = NULL;
+	if (g_ptr_array_remove (bar->priv->attachments, attachment))
+		g_signal_emit (bar, signals[CHANGED], 0);
 }
 
 static void
@@ -141,18 +137,13 @@ attachment_changed_cb (EAttachment *attachment,
 }
 
 static void
-add_common (EAttachmentBar *bar,
-	    EAttachment *attachment)
+add_common (EAttachmentBar *bar, EAttachment *attachment)
 {
 	g_return_if_fail (attachment != NULL);
 	
-	g_signal_connect (attachment, "changed",
-			  G_CALLBACK (attachment_changed_cb),
-			  bar);
-	
-	bar->priv->attachments = g_list_append (bar->priv->attachments,
-						attachment);
-	bar->priv->num_attachments++;
+	g_ptr_array_add (bar->priv->attachments, attachment);
+	g_object_weak_ref ((GObject *) attachment, (GWeakNotify) attachment_destroy, bar);
+	g_signal_connect (attachment, "changed", G_CALLBACK (attachment_changed_cb), bar);
 	
 	update (bar);
 	
@@ -160,53 +151,30 @@ add_common (EAttachmentBar *bar,
 }
 
 static void
-add_from_mime_part (EAttachmentBar *bar,
-		    CamelMimePart *part)
+add_from_mime_part (EAttachmentBar *bar, CamelMimePart *part)
 {
 	add_common (bar, e_attachment_new_from_mime_part (part));
 }
 
 static void
-add_from_file (EAttachmentBar *bar,
-	       const char *file_name,
-	       const char *disposition)
+add_from_file (EAttachmentBar *bar, const char *file_name, const char *disposition)
 {
 	EAttachment *attachment;
 	CamelException ex;
 	
 	camel_exception_init (&ex);
-	attachment = e_attachment_new (file_name, disposition, &ex);
-	if (attachment) {
+	
+	if ((attachment = e_attachment_new (file_name, disposition, &ex))) {
 		add_common (bar, attachment);
 	} else {
 		/* FIXME: Avoid using error from mailer */
-		e_error_run((GtkWindow *)gtk_widget_get_toplevel((GtkWidget *)bar), "mail-composer:no-attach",
-			    file_name, camel_exception_get_description(&ex), NULL);
+		e_error_run ((GtkWindow *) gtk_widget_get_toplevel ((GtkWidget *) bar), "mail-composer:no-attach",
+			     file_name, camel_exception_get_description (&ex), NULL);
 		camel_exception_clear (&ex);
 	}
 }
 
-static void
-remove_attachment (EAttachmentBar *bar,
-		   EAttachment *attachment)
-{
-	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
-	g_return_if_fail (g_list_find (bar->priv->attachments, attachment) != NULL);
-
-	bar->priv->attachments = g_list_remove (bar->priv->attachments,
-						attachment);
-	bar->priv->num_attachments--;
-	if (attachment->editor_gui != NULL) {
-		GtkWidget *dialog = glade_xml_get_widget (attachment->editor_gui, "dialog");
-		g_signal_emit_by_name (dialog, "response", GTK_RESPONSE_CLOSE);
-	}
-	
-	g_object_unref(attachment);
-	
-	g_signal_emit (bar, signals[CHANGED], 0);
-}
 
-
 /* Icon list contents handling.  */
 
 static void
@@ -215,7 +183,7 @@ calculate_height_width(EAttachmentBar *bar, int *new_width, int *new_height)
         int width, height, icon_width;
         PangoFontMetrics *metrics;
         PangoContext *context;
-			
+	
         context = gtk_widget_get_pango_context ((GtkWidget *) bar);
         metrics = pango_context_get_metrics (context, ((GtkWidget *) bar)->style->font_desc, pango_context_get_language (context));
         width = PANGO_PIXELS (pango_font_metrics_get_approximate_char_width (metrics)) * 15;
@@ -296,10 +264,10 @@ e_attachment_bar_create_attachment_cache (EAttachment *attachment)
 static void
 update (EAttachmentBar *bar)
 {
-	EAttachmentBarPrivate *priv;
+	struct _EAttachmentBarPrivate *priv;
 	GnomeIconList *icon_list;
-	GList *p;
 	int bar_width, bar_height;
+	int i;
 	
 	priv = bar->priv;
 	icon_list = GNOME_ICON_LIST (bar);
@@ -309,29 +277,28 @@ update (EAttachmentBar *bar)
 	gnome_icon_list_clear (icon_list);
 	
 	/* FIXME could be faster, but we don't care.  */
-	for (p = priv->attachments; p != NULL; p = p->next) {
+	for (i = 0; i < priv->attachments->len; i++) {
 		EAttachment *attachment;
 		CamelContentType *content_type;
 		char *size_string, *label;
-		GdkPixbuf *pixbuf=NULL;
+		GdkPixbuf *pixbuf = NULL;
 		const char *desc;
 		
-		attachment = p->data;
-
+		attachment = priv->attachments->pdata[i];
+		
 		if (!attachment->is_available_local) {
 			/* stock_attach would be better, but its fugly scaled up */
-			pixbuf = e_icon_factory_get_icon("stock_unknown", E_ICON_SIZE_DIALOG);
-			if (pixbuf) {
+			if ((pixbuf = e_icon_factory_get_icon("stock_unknown", E_ICON_SIZE_DIALOG))) {
 				attachment->index = gnome_icon_list_append_pixbuf (icon_list, pixbuf, NULL, "");
-				g_object_unref(pixbuf);
+				g_object_unref (pixbuf);
 			}
 			continue;
 		}
+		
 		content_type = camel_mime_part_get_content_type (attachment->body);
 		/* Get the image out of the attachment 
 		   and create a thumbnail for it */
-		pixbuf = attachment->pixbuf_cache;
-		if (pixbuf) {
+		if ((pixbuf = attachment->pixbuf_cache)) {
 			g_object_ref(pixbuf);
 		} else if (camel_content_type_is(content_type, "image", "*")) {
 			CamelDataWrapper *wrapper;
@@ -370,13 +337,10 @@ update (EAttachmentBar *bar)
 					}
 				}
 				
-				attachment->pixbuf_cache = gdk_pixbuf_scale_simple 
-					(pixbuf,
-					 width,
-					 height,
-					 GDK_INTERP_BILINEAR);
+				attachment->pixbuf_cache = gdk_pixbuf_scale_simple (pixbuf, width, height,
+										    GDK_INTERP_BILINEAR);
 				pixbuf = attachment->pixbuf_cache;
-				g_object_ref(pixbuf);
+				g_object_ref (pixbuf);
 			} else {
 				pixbuf = NULL;
 				g_warning ("GdkPixbufLoader Error");
@@ -398,8 +362,7 @@ update (EAttachmentBar *bar)
 		if (!desc)
 			desc = _("attachment");
 		
-		if (attachment->size
-		    && (size_string = size_to_string (attachment->size))) {
+		if (attachment->size && (size_string = size_to_string (attachment->size))) {
 			label = g_strdup_printf ("%s (%s)", desc, size_string);
 			g_free (size_string);
 		} else
@@ -417,65 +380,65 @@ update (EAttachmentBar *bar)
 			}
 			g_free (mime_type);
 		}
-
+		
 		if (pixbuf) {
-			GdkPixbuf* pixbuf_orig = pixbuf;
+			GdkPixbuf *pixbuf_orig = pixbuf;
 			pixbuf = gdk_pixbuf_add_alpha (pixbuf_orig, TRUE, 255, 255, 255);
-
+			
 			/* gdk_pixbuf_add_alpha returns a newly allocated pixbuf,
 			   free the original one. 
 			*/
 			g_object_unref (pixbuf_orig);
-
+			
 			/* In case of a attachment bar, in a signed/encrypted part, display the status as a emblem*/
 			if (attachment->sign) {
 				/* Show the signature status at the right-bottom.*/
 				GdkPixbuf *sign = NULL;
-				int x,y;
-
+				int x, y;
+				
 				if (attachment->sign == CAMEL_CIPHER_VALIDITY_SIGN_BAD)
-					sign = e_icon_factory_get_icon("stock_signature-bad", E_ICON_SIZE_MENU);
+					sign = e_icon_factory_get_icon ("stock_signature-bad", E_ICON_SIZE_MENU);
 				else if (attachment->sign == CAMEL_CIPHER_VALIDITY_SIGN_GOOD)
-					sign = e_icon_factory_get_icon("stock_signature-ok", E_ICON_SIZE_MENU);
+					sign = e_icon_factory_get_icon ("stock_signature-ok", E_ICON_SIZE_MENU);
 				else
-					sign = e_icon_factory_get_icon("stock_signature", E_ICON_SIZE_MENU);
-
-				x = gdk_pixbuf_get_width(pixbuf) - 17;
-				y = gdk_pixbuf_get_height(pixbuf) - 17;
+					sign = e_icon_factory_get_icon "stock_signature", E_ICON_SIZE_MENU);
 				
-				gdk_pixbuf_copy_area(sign, 0, 0, 16, 16, pixbuf, x, y);
+				x = gdk_pixbuf_get_width (pixbuf) - 17;
+				y = gdk_pixbuf_get_height (pixbuf) - 17;
+				
+				gdk_pixbuf_copy_area (sign, 0, 0, 16, 16, pixbuf, x, y);
 				g_object_unref (sign);
 			}
-
+			
 			if (attachment->encrypt) {
 				/* Show the encryption status at the top left.*/
-				GdkPixbuf *encrypt = e_icon_factory_get_icon("stock_lock-ok", E_ICON_SIZE_MENU);
+				GdkPixbuf *encrypt = e_icon_factory_get_icon ("stock_lock-ok", E_ICON_SIZE_MENU);
 				
-				gdk_pixbuf_copy_area(encrypt, 0, 0, 16, 16, pixbuf, 1, 1);
+				gdk_pixbuf_copy_area (encrypt, 0, 0, 16, 16, pixbuf, 1, 1);
 				g_object_unref (encrypt);
 			}
-
+			
 			gnome_icon_list_append_pixbuf (icon_list, pixbuf, NULL, label);
-			g_object_unref(pixbuf);
+			g_object_unref (pixbuf);
 		}
 		
 		g_free (label);
 	}
 	
 	gnome_icon_list_thaw (icon_list);
-
+	
 	/* Resize */
 	if (bar->expand) {
-		gtk_widget_get_size_request ((GtkWidget *)bar, &bar_width, &bar_height);
-	
-		if (bar->priv->num_attachments) {
+		gtk_widget_get_size_request ((GtkWidget *) bar, &bar_width, &bar_height);
+		
+		if (bar->priv->attachments->len) {
 			int per_col, rows, height, width;
-	
+			
 			calculate_height_width(bar, &width, &height);
 			per_col = bar_width / width;
 			per_col = (per_col ? per_col : 1);
-			rows = (bar->priv->num_attachments + per_col -1 )/ per_col;
-			gtk_widget_set_size_request ((GtkWidget *)bar, bar_width, rows * height);
+			rows = (bar->priv->attachments->len + per_col -1) / per_col;
+			gtk_widget_set_size_request ((GtkWidget *) bar, bar_width, rows * height);
 		}
 	}
 }
@@ -512,47 +475,35 @@ update_remote_file (EAttachment *attachment, EAttachmentBar *bar)
 void
 e_attachment_bar_remove_selected (EAttachmentBar *bar)
 {
-	GnomeIconList *icon_list;
+	struct _EAttachmentBarPrivate *priv;
 	EAttachment *attachment;
-	GList *attachment_list, *p;
-	int num = 0, left, dlen;
-
-	g_return_if_fail (bar != NULL);
+	int id, left, nrem = 0;
+	GList *items;
+	
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
 	
-	icon_list = GNOME_ICON_LIST (bar);
+	priv = bar->priv;
 	
-	/* Weee!  I am especially proud of this piece of cheesy code: it is
-           truly awful.  But unless one attaches a huge number of files, it
-           will not be as greedy as intended.  FIXME of course.  */
+	if (!(items = gnome_icon_list_get_selection ((GnomeIconList *) bar)))
+		return;
 	
-	attachment_list = NULL;
-	p = gnome_icon_list_get_selection (icon_list);
-	dlen = g_list_length (p);
-	for ( ; p != NULL; p = p->next) {
-		num = GPOINTER_TO_INT (p->data);
-		attachment = E_ATTACHMENT (g_list_nth_data (bar->priv->attachments, num));
-
-		/* We need to check if there are duplicated index in the return list of 
-		   gnome_icon_list_get_selection() because of gnome bugzilla bug #122356.
-		   FIXME in the future. */
-
-		if (g_list_find (attachment_list, attachment) == NULL) {
-			attachment_list = g_list_prepend (attachment_list, attachment);
+	while (items != NULL) {
+		if ((id = GPOINTER_TO_INT (items->data) - nrem) < priv->attachments->len) {
+			/* Note: this removes the item from the array due to the weak_ref callback */
+			attachment = priv->attachments->pdata[id];
+			g_object_unref (attachment);
+			nrem++;
 		}
+		
+		items = items->next;
 	}
 	
-	for (p = attachment_list; p != NULL; p = p->next)
-		remove_attachment (bar, E_ATTACHMENT (p->data));
-	
-	g_list_free (attachment_list);
-	
 	update (bar);
 	
-	left = gnome_icon_list_get_num_icons (icon_list);
-	num = num - dlen + 1;
-	if (left > 0)
-		gnome_icon_list_focus_icon (icon_list, left > num ? num : left - 1);
+	id++;
+	
+	if ((left = gnome_icon_list_get_num_icons ((GnomeIconList *) bar)) > 0)
+		gnome_icon_list_focus_icon ((GnomeIconList *) bar, left > id ? id : left - 1);
 }
 
 void
@@ -563,120 +514,136 @@ e_attachment_bar_set_width(EAttachmentBar *bar, int bar_width)
 	calculate_height_width(bar, &width, &height);
 	per_col = bar_width / width;
 	per_col = (per_col ? per_col : 1);
-	rows = (bar->priv->num_attachments + per_col - 1) / per_col;
+	rows = (bar->priv->attachments->len + per_col - 1) / per_col;
 	gtk_widget_set_size_request ((GtkWidget *)bar, bar_width, rows * height);
 }
 
 void
 e_attachment_bar_edit_selected (EAttachmentBar *bar)
 {
-	GnomeIconList *icon_list;
-	GList *selection, *attach;
-	int num;
-
-	g_return_if_fail (bar != NULL);
+	struct _EAttachmentBarPrivate *priv;
+	EAttachment *attachment;
+	GList *items;
+	int id;
+	
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
 	
-	icon_list = GNOME_ICON_LIST (bar);
+	priv = bar->priv;
 	
-	selection = gnome_icon_list_get_selection (icon_list);
-	if (selection) {
-		num = GPOINTER_TO_INT (selection->data);
-		attach = g_list_nth (bar->priv->attachments, num);
-		if (attach)
-			e_attachment_edit ((EAttachment *)attach->data, GTK_WIDGET (bar));
+	items = gnome_icon_list_get_selection ((GnomeIconList *) bar);
+	while (items != NULL) {
+		if ((id = GPOINTER_TO_INT (items->data)) < priv->attachments->len) {
+			attachment = priv->attachments->pdata[id];
+			e_attachment_edit (attachment, GTK_WIDGET (bar));
+		}
+		
+		items = items->next;
 	}
 }
 
 GtkWidget **
 e_attachment_bar_get_selector(EAttachmentBar *bar)
 {
-	g_return_val_if_fail (bar != NULL, 0);
-	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
+	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), NULL);
 	
 	return &bar->priv->attach;
 }
 
+
+/**
+ * e_attachment_bar_get_selected:
+ * @bar: an #EAttachmentBar object
+ *
+ * Returns a newly allocated #GSList of ref'd #EAttachment objects
+ * representing the selected items in the #EAttachmentBar Icon List.
+ **/
 GSList *
 e_attachment_bar_get_selected (EAttachmentBar *bar)
 {
+	struct _EAttachmentBarPrivate *priv;
 	GSList *attachments = NULL;
-	GList *p;
-
-	g_return_val_if_fail (bar != NULL, 0);
-	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
-
-	p = gnome_icon_list_get_selection((GnomeIconList *)bar);
-	for ( ; p != NULL; p = p->next) {
-		int num = GPOINTER_TO_INT(p->data);
-		EAttachment *attachment = g_list_nth_data(bar->priv->attachments, num);
-			
-		if (attachment && g_slist_find(attachments, attachment) == NULL) {
-			g_object_ref(attachment);
-			attachments = g_slist_prepend(attachments, attachment);
+	EAttachment *attachment;
+	GList *items;
+	int id;
+	
+	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), NULL);
+	
+	priv = bar->priv;
+	
+	items = gnome_icon_list_get_selection ((GnomeIconList *) bar);
+	
+	while (items != NULL) {
+		if ((id = GPOINTER_TO_INT (items->data)) < priv->attachments->len) {
+			attachment = priv->attachments->pdata[id];
+			attachments = g_slist_prepend (attachments, attachment);
+			g_object_ref (attachment);
 		}
+		
+		items = items->next;
 	}
-	attachments = g_slist_reverse(attachments);
+	
+	attachments = g_slist_reverse (attachments);
 	
 	return attachments;
 }
 
 /* FIXME: Cleanup this, since there is a api to get selected attachments */
-/* if id != -1, then use it as an index for target of the popup */
+/**
+ * e_attachment_bar_get_attachment:
+ * @bar: an #EAttachmentBar object
+ * @id: Index of the desired attachment or -1 to request all selected attachments
+ *
+ * Returns a newly allocated #GSList of ref'd #EAttachment objects
+ * representing the requested item(s) in the #EAttachmentBar Icon
+ * List.
+ **/
 GSList *
 e_attachment_bar_get_attachment (EAttachmentBar *bar, int id)
 {
-	GSList *attachments = NULL;
-	GList *p;
+	struct _EAttachmentBarPrivate *priv;
 	EAttachment *attachment;
-
-	g_return_val_if_fail (bar != NULL, 0);
-	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
-
-	/* We need to check if there are duplicated index in the return list of 
-	   gnome_icon_list_get_selection() because of gnome bugzilla bug #122356.
-	   FIXME in the future. */
-
-	if (id == -1
-	    || (attachment = g_list_nth_data(bar->priv->attachments, id)) == NULL) {
-		p = gnome_icon_list_get_selection((GnomeIconList *)bar);
-		for ( ; p != NULL; p = p->next) {
-			int num = GPOINTER_TO_INT(p->data);
-			EAttachment *attachment = g_list_nth_data(bar->priv->attachments, num);
-			
-			if (attachment && g_slist_find(attachments, attachment) == NULL) {
-				g_object_ref(attachment);
-				attachments = g_slist_prepend(attachments, attachment);
-			}
-		}
-		attachments = g_slist_reverse(attachments);
-	} else {
-		g_object_ref(attachment);
-		attachments = g_slist_prepend(attachments, attachment);
-	}
+	GSList *attachments;
+	
+	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), NULL);
+	
+	priv = bar->priv;
+	
+	if (id == -1 || id > priv->attachments->len)
+		return e_attachment_bar_get_selected (bar);
+	
+	attachment = priv->attachments->pdata[id];
+	attachments = g_slist_prepend (NULL, attachment);
+	g_object_ref (attachment);
 	
 	return attachments;
 }
 
+
+/**
+ * e_attachment_bar_get_all_attachments:
+ * @bar: an #EAttachmentBar object
+ *
+ * Returns a newly allocated #GSList of ref'd #EAttachment objects.
+ **/
 GSList *
 e_attachment_bar_get_all_attachments (EAttachmentBar *bar)
 {
+	struct _EAttachmentBarPrivate *priv;
 	GSList *attachments = NULL;
-	GList *p;
 	EAttachment *attachment;
-
-	g_return_val_if_fail (bar != NULL, 0);
-	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
-
-        for ( p = bar->priv->attachments; p!= NULL; p = p->next) {
-                attachment = p->data;
-                if (attachment && attachment->is_available_local) {
+	int i;
+	
+	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), NULL);
+	
+	priv = bar->priv;
+	
+	for (i = priv->attachments->len - 1; i >= 0; i--) {
+		attachment = priv->attachments->pdata[i];
+		if (attachment->is_available_local) {
+			attachments = g_slist_prepend (attachments, attachment);
 			g_object_ref (attachment);
-                        attachments= g_slist_prepend(attachments, attachment);
 		}
-        }
-
-	attachments = g_slist_reverse(attachments);
+	}
 	
 	return attachments;
 }
@@ -685,20 +652,22 @@ e_attachment_bar_get_all_attachments (EAttachmentBar *bar)
 GSList *
 e_attachment_bar_get_parts (EAttachmentBar *bar)
 {
-        EAttachment *attachment;
-        GList *p = NULL;
-	GSList *part_list = NULL;
-
-	g_return_val_if_fail (bar != NULL, 0);
-	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
-
-        for ( p = bar->priv->attachments; p!= NULL; p = p->next) {
-                attachment = p->data;
-                if (attachment && attachment->is_available_local)
-                        part_list = g_slist_prepend(part_list, attachment->body);
-        }
+	struct _EAttachmentBarPrivate *priv;
+	EAttachment *attachment;
+	GSList *parts = NULL;
+	int i;
+	
+	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), NULL);
+	
+	priv = bar->priv;
+	
+	for (i = 0; i < priv->attachments->len; i++) {
+		attachment = priv->attachments->pdata[i];
+		if (attachment->is_available_local)
+			parts = g_slist_prepend (parts, attachment->body);
+	}
 	
-        return part_list;
+        return parts;
 }
 
 /* GtkObject methods.  */
@@ -706,20 +675,26 @@ e_attachment_bar_get_parts (EAttachmentBar *bar)
 static void
 destroy (GtkObject *object)
 {
-	EAttachmentBar *bar;
-	
-	bar = E_ATTACHMENT_BAR (object);
+	EAttachmentBar *bar = (EAttachmentBar *) object;
+	struct _EAttachmentBarPrivate *priv = bar->priv;
+	EAttachment *attachment;
+	int i;
 	
-	if (bar->priv) {
-		free_attachment_list (bar);
-
-		if (bar->priv->attach)
-			gtk_widget_destroy(bar->priv->attach);
-
-		if (bar->priv->path)
-			g_free (bar->priv->path);
-
-		g_free (bar->priv);
+	if ((priv = bar->priv)) {
+		priv->batch_unref = TRUE;
+		for (i = 0; i < priv->attachments->len; i++) {
+			attachment = priv->attachments->pdata[i];
+			g_object_unref (attachment);
+		}
+		g_ptr_array_free (priv->attachments, TRUE);
+		
+		if (priv->attach)
+			gtk_widget_destroy (priv->attach);
+		
+		if (priv->path)
+			g_free (priv->path);
+		
+		g_free (priv);
 		bar->priv = NULL;
 	}
 	
@@ -728,20 +703,17 @@ destroy (GtkObject *object)
 }
 
 static char *
-temp_save_part(CamelMimePart *part)
+temp_save_part (CamelMimePart *part)
 {
 	const char *filename;
 	char *tmpdir, *path, *mfilename = NULL, *utf8_mfilename = NULL;
 	CamelStream *stream;
 	CamelDataWrapper *wrapper;
-
-	tmpdir = e_mkdtemp("evolution-tmp-XXXXXX");
-	if (tmpdir == NULL) {
+	
+	if (!(tmpdir = e_mkdtemp ("evolution-tmp-XXXXXX")))
 		return NULL;
-	}
-
-	filename = camel_mime_part_get_filename (part);
-	if (filename == NULL) {
+	
+	if (!(filename = camel_mime_part_get_filename (part))) {
 		/* This is the default filename used for temporary file creation */
 		filename = _("Unknown");
 	} else {
@@ -751,14 +723,14 @@ temp_save_part(CamelMimePart *part)
 		g_free (utf8_mfilename);
 		filename = (const char *) mfilename;
 	}
-
-	path = g_build_filename(tmpdir, filename, NULL);
-	g_free(tmpdir);
-	g_free(mfilename);
-
+	
+	path = g_build_filename (tmpdir, filename, NULL);
+	g_free (tmpdir);
+	g_free (mfilename);
+	
 	wrapper = camel_medium_get_content_object (CAMEL_MEDIUM (part));
 	stream = camel_stream_fs_new_with_name (path, O_RDWR|O_CREAT|O_TRUNC, 0600);
-
+	
 	if (!stream) {
 		/* TODO handle error conditions */
 		g_message ("DEBUG: could not open the file to write\n");
@@ -783,51 +755,54 @@ temp_save_part(CamelMimePart *part)
 static void
 eab_drag_data_get(EAttachmentBar *bar, GdkDragContext *drag, GtkSelectionData *data, guint info, guint time)
 {
-	char *path;
-	GList *tmp;
-	gchar **uris;
-	int length, i=0;
-
+	struct _EAttachmentBarPrivate *priv = bar->priv;
+	EAttachment *attachment;
+	char *path, **uris;
+	int len, n, i = 0;
+	CamelURL *url;
+	GList *items;
+	
 	if (info)
 		return;
 	
-	tmp = gnome_icon_list_get_selection (GNOME_ICON_LIST(bar));
-	length = g_list_length (tmp);
-
-	uris = g_malloc0(sizeof(bar) * (length+1));
-
-	for (; tmp; tmp = tmp->next) {
-		int num = GPOINTER_TO_INT(tmp->data);
-		EAttachment *attachment = g_list_nth_data(bar->priv->attachments, num);	
-		CamelURL *curl;
-
+	items = gnome_icon_list_get_selection (GNOME_ICON_LIST (bar));
+	len = g_list_length (items);
+	
+	uris = g_malloc0 (sizeof (char *) * (len + 1));
+	
+	for ( ; items != NULL; items = items->next) {
+		if (!((n = GPOINTER_TO_INT (items->data)) < priv->attachments->len))
+			continue;
+		
+		attachment = priv->attachments->pdata[n];
+		
 		if (!attachment->is_available_local)
 			continue;
-
+		
 		if (attachment->store_uri) {
-			uris[i] = attachment->store_uri;
-			i++;
+			uris[i++] = attachment->store_uri;
 			continue;
 		}
-		path = temp_save_part(attachment->body);
-		/* If we are not able to save, ignore it*/
-		if (path == NULL) 
+		
+		/* If we are not able to save, ignore it */
+		if (!(path = temp_save_part (attachment->body)))
 			continue;
 		
-		curl = camel_url_new("file:", NULL);
-		camel_url_set_path (curl, path);
-		attachment->store_uri = camel_url_to_string (curl, 0);
-		camel_url_free(curl);		
-		g_free(path);
+		url = camel_url_new ("file:", NULL);
+		camel_url_set_path (url, path);
+		attachment->store_uri = camel_url_to_string (url, 0);
+		camel_url_free (url);
+		g_free (path);
 		
-		uris[i] = attachment->store_uri;
-		i++;
+		uris[i++] = attachment->store_uri;
 	}
-	uris[i]=0;
-	gtk_selection_data_set_uris(data, uris);
+	
+	uris[i] = NULL;
+	
+	gtk_selection_data_set_uris (data, uris);
 	
 	g_free (uris);
-
+	
 	return;
 }
 
@@ -865,17 +840,17 @@ eab_button_press_event(EAttachmentBar *bar, GdkEventButton *event, gpointer dumm
 	};	
 
 	selected = gnome_icon_list_get_selection(icon_list);
-	length = g_list_length(selected);
+	length = g_list_length (selected);
 
 	if (event) {
 		icon_number = gnome_icon_list_get_icon_at(icon_list, event->x, event->y);
 		if (icon_number < 0) { 
-			/* When nothing is selected deselect all*/
-			gnome_icon_list_unselect_all(icon_list);
+			/* When nothing is selected, deselect all */
+			gnome_icon_list_unselect_all (icon_list);
 			length = 0;
 			selected = NULL;
 		}
-	
+		
 		if (event->button == 1) {
 			/* If something is selected, then allow drag or else help to select */
 			if (length)
@@ -884,11 +859,11 @@ eab_button_press_event(EAttachmentBar *bar, GdkEventButton *event, gpointer dumm
 				gtk_drag_source_unset((GtkWidget *)bar);
 			return FALSE;
 		}
-	
+		
 		/* If not r-click dont progress any more.*/
 		if (event->button != 3)
-			return FALSE;	
-
+			return FALSE;
+		
 		/* When a r-click on something, if it is in the already selected list, consider a r-click of multiple things
 		 * or deselect all and select only this for r-click 
 		 */
@@ -897,42 +872,42 @@ eab_button_press_event(EAttachmentBar *bar, GdkEventButton *event, gpointer dumm
 				if (GPOINTER_TO_INT(tmp->data) == icon_number)
 					take_selected = TRUE;
 			}
-	
+			
 			if (!take_selected) {
 				gnome_icon_list_unselect_all(icon_list);
 				gnome_icon_list_select_icon(icon_list, icon_number);
 			}
 		}
-	} 
+	}
+	
 	return FALSE;
 }
 
 static gboolean
 eab_icon_clicked_cb (EAttachmentBar *bar, GdkEvent *event, gpointer *dummy)
 {
-	GSList *p = NULL;
+	EAttachment *attachment;
 	GError *error = NULL;
 	gboolean ret = FALSE;
-
+	CamelURL *url;
+	char *path;
+	GSList *p;
+	
 	if (E_IS_ATTACHMENT_BAR (bar) && event->type == GDK_2BUTTON_PRESS) {
 		p = e_attachment_bar_get_selected (bar);
-		if (p && g_slist_length(p) == 1) {
-			EAttachment *attachment;
-			char *path = NULL;
-
-			attachment = (EAttachment *)p->data;
-
+		if (p && p->next == NULL) {
+			attachment = p->data;
+			
 			/* Check if the file is stored already */
 			if (!attachment->store_uri) {
-				CamelURL *curl;
-				
-				path = temp_save_part (attachment->body);
-				curl  = camel_url_new ("file://", NULL);
-				camel_url_set_path ( curl, path);
-				attachment->store_uri = camel_url_to_string (curl, 0);
-				camel_url_free (curl);
+				path = temp_save_part (attachment->body);				
+				url = camel_url_new ("file:", NULL);
+				camel_url_set_path (url, path);
+				attachment->store_uri = camel_url_to_string (url, 0);
+				camel_url_free (url);
+				g_free (path);
 			}
-
+			
 			/* launch the url now */
 			gnome_url_show (attachment->store_uri, &error);
 			if (error) {
@@ -940,13 +915,12 @@ eab_icon_clicked_cb (EAttachmentBar *bar, GdkEvent *event, gpointer *dummy)
 				g_error_free (error);
 				error = NULL;
 			}
-
-			g_free (path);
+			
 			ret = TRUE;
 		}
-
+		
 		if (p) {
-			g_slist_foreach (p, (GFunc)g_object_unref, NULL);
+			g_slist_foreach (p, (GFunc) g_object_unref, NULL);
 			g_slist_free (p);
 		}
 	}
@@ -982,13 +956,13 @@ class_init (EAttachmentBarClass *klass)
 static void
 init (EAttachmentBar *bar)
 {
-	EAttachmentBarPrivate *priv;
+	struct _EAttachmentBarPrivate *priv;
 	
-	priv = g_new (EAttachmentBarPrivate, 1);
+	priv = g_new (struct _EAttachmentBarPrivate, 1);
 	
 	priv->attach = NULL;
-	priv->attachments = NULL;
-	priv->num_attachments = 0;
+	priv->batch_unref = FALSE;
+	priv->attachments = g_ptr_array_new ();
 	priv->path = NULL;
 	
 	bar->priv = priv;
@@ -1045,8 +1019,8 @@ e_attachment_bar_new (GtkAdjustment *adj)
 	gnome_icon_list_set_selection_mode (icon_list, GTK_SELECTION_MULTIPLE);
 
 	atk_object_set_name (gtk_widget_get_accessible (GTK_WIDGET (new)), 
-			_("Attachment Bar"));
-
+			     _("Attachment Bar"));
+	
 	g_signal_connect (new, "button_release_event", G_CALLBACK(eab_button_release_event), NULL);
 	g_signal_connect (new, "button_press_event", G_CALLBACK(eab_button_press_event), NULL);
 	g_signal_connect (new, "drag-data-get", G_CALLBACK(eab_drag_data_get), NULL);
@@ -1149,54 +1123,46 @@ attach_to_multipart (CamelMultipart *multipart,
 }
 
 void
-e_attachment_bar_to_multipart (EAttachmentBar *bar,
-			       CamelMultipart *multipart,
-			       const char *default_charset)
+e_attachment_bar_to_multipart (EAttachmentBar *bar, CamelMultipart *multipart, const char *default_charset)
 {
-	EAttachmentBarPrivate *priv;
-	GList *p;
+	struct _EAttachmentBarPrivate *priv;
+	EAttachment *attachment;
+	int i;
 	
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
 	g_return_if_fail (CAMEL_IS_MULTIPART (multipart));
 	
 	priv = bar->priv;
 	
-	for (p = priv->attachments; p != NULL; p = p->next) {
-		EAttachment *attachment;
-		
-		attachment = E_ATTACHMENT (p->data);
+	for (i = 0; i < priv->attachments->len; i++) {
+		attachment = priv->attachments->pdata[i];
 		if (attachment->is_available_local)
 			attach_to_multipart (multipart, attachment, default_charset);
 	}
 }
 
-
+
 guint
 e_attachment_bar_get_num_attachments (EAttachmentBar *bar)
 {
-	g_return_val_if_fail (bar != NULL, 0);
 	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
 	
-	return bar->priv->num_attachments;
+	return bar->priv->attachments->len;
 }
 
-
+
 void
-e_attachment_bar_attach (EAttachmentBar *bar,
-			 const gchar *file_name,
-			 char *disposition)
+e_attachment_bar_attach (EAttachmentBar *bar, const char *file_name, const char *disposition)
 {
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
-	g_return_if_fail ( file_name != NULL && disposition != NULL);
+	g_return_if_fail (file_name != NULL && disposition != NULL);
 	
 	add_from_file (bar, file_name, disposition);
 }
 
 void
-e_attachment_bar_add_attachment (EAttachmentBar *bar,
-				 EAttachment *attachment)
+e_attachment_bar_add_attachment (EAttachmentBar *bar, EAttachment *attachment)
 {
-	g_return_if_fail (bar != NULL);
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
 	
 	add_common (bar, attachment);
@@ -1205,56 +1171,48 @@ e_attachment_bar_add_attachment (EAttachmentBar *bar,
 int 
 e_attachment_bar_get_download_count (EAttachmentBar *bar)
 {
-	EAttachmentBarPrivate *priv;
-	GList *p;
-	int count=0;
+	struct _EAttachmentBarPrivate *priv;
+	EAttachment *attachment;
+	int i, n = 0;
 	
-	g_return_val_if_fail (bar != NULL, 0);
 	g_return_val_if_fail (E_IS_ATTACHMENT_BAR (bar), 0);
 	
 	priv = bar->priv;
 	
-	for (p = priv->attachments; p != NULL; p = p->next) {
-		EAttachment *attachment;
-		
-		attachment = p->data;
+	for (i = 0; i < priv->attachments->len; i++) {
+		attachment = priv->attachments->pdata[i];
 		if (!attachment->is_available_local)
-			count++;
+			n++;
 	}
-
-	return count;
+	
+	return n;
 }
 
-void 
-e_attachment_bar_attach_remote_file (EAttachmentBar *bar,
-				     const gchar *url, const char *disposition)
+void
+e_attachment_bar_attach_remote_file (EAttachmentBar *bar, const char *url, const char *disposition)
 {
 	EAttachment *attachment;
 	CamelException ex;
-
-	g_return_if_fail ( bar!=NULL );
+	
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
-
+	
 	if (!bar->priv->path)
-		bar->priv->path = e_mkdtemp("attach-XXXXXX");
-
+		bar->priv->path = e_mkdtemp ("attach-XXXXXX");
+	
 	camel_exception_init (&ex);
-	attachment = e_attachment_new_remote_file (url, disposition, bar->priv->path, &ex);
-	g_signal_connect (attachment, "update", G_CALLBACK(update_remote_file), bar);
-	if (attachment) {
+	if ((attachment = e_attachment_new_remote_file (url, disposition, bar->priv->path, &ex))) {
 		add_common (bar, attachment);
+		g_signal_connect (attachment, "update", G_CALLBACK (update_remote_file), bar);
 	} else {
-		e_error_run((GtkWindow *)gtk_widget_get_toplevel((GtkWidget *)bar), "mail-composer:no-attach",
-			    attachment->file_name, camel_exception_get_description(&ex), NULL);
+		e_error_run ((GtkWindow *) gtk_widget_get_toplevel ((GtkWidget *) bar), "mail-composer:no-attach",
+			     attachment->file_name, camel_exception_get_description (&ex), NULL);
 		camel_exception_clear (&ex);
 	}
 }
 
 void
-e_attachment_bar_attach_mime_part (EAttachmentBar *bar,
-				   CamelMimePart *part)
+e_attachment_bar_attach_mime_part (EAttachmentBar *bar, CamelMimePart *part)
 {
-	g_return_if_fail ( bar!=NULL );
 	g_return_if_fail (E_IS_ATTACHMENT_BAR (bar));
 	
 	add_from_mime_part (bar, part);
diff --git a/widgets/misc/e-attachment-bar.h b/widgets/misc/e-attachment-bar.h
index 21f568c186..0acccf742a 100644
--- a/widgets/misc/e-attachment-bar.h
+++ b/widgets/misc/e-attachment-bar.h
@@ -48,13 +48,12 @@ extern "C" {
 
 typedef struct _EAttachmentBar EAttachmentBar;
 typedef struct _EAttachmentBarClass EAttachmentBarClass;
-typedef struct _EAttachmentBarPrivate EAttachmentBarPrivate;
 
 struct _EAttachmentBar {
 	GnomeIconList parent;
 	gboolean expand;
-
-	EAttachmentBarPrivate *priv;
+	
+	struct _EAttachmentBarPrivate *priv;
 };
 
 struct _EAttachmentBarClass {
@@ -68,12 +67,12 @@ GtkType e_attachment_bar_get_type (void);
 
 GtkWidget *e_attachment_bar_new (GtkAdjustment *adj);
 void e_attachment_bar_to_multipart (EAttachmentBar *bar, CamelMultipart *multipart,
-						 const char *default_charset);
+				    const char *default_charset);
 guint e_attachment_bar_get_num_attachments (EAttachmentBar *bar);
-void e_attachment_bar_attach (EAttachmentBar *bar, const char *file_name, char *disposition);
+void e_attachment_bar_attach (EAttachmentBar *bar, const char *file_name, const char *disposition);
 void e_attachment_bar_attach_mime_part (EAttachmentBar *bar, CamelMimePart *part);
 int e_attachment_bar_get_download_count (EAttachmentBar *bar);
-void e_attachment_bar_attach_remote_file (EAttachmentBar *bar,const gchar *url, const char *disposition);
+void e_attachment_bar_attach_remote_file (EAttachmentBar *bar, const char *url, const char *disposition);
 GSList *e_attachment_bar_get_attachment (EAttachmentBar *bar, int id);
 void e_attachment_bar_add_attachment (EAttachmentBar *bar, EAttachment *attachment);
 void e_attachment_bar_edit_selected (EAttachmentBar *bar);
diff --git a/widgets/misc/e-attachment.c b/widgets/misc/e-attachment.c
index 7dae3d8e21..570c2e3332 100644
--- a/widgets/misc/e-attachment.c
+++ b/widgets/misc/e-attachment.c
@@ -64,6 +64,7 @@ enum {
 	UPDATE,
 	LAST_SIGNAL
 };
+
 static guint signals[LAST_SIGNAL] = { 0 };
 
 static GObjectClass *parent_class = NULL;
@@ -74,16 +75,20 @@ changed (EAttachment *attachment)
 	g_signal_emit (attachment, signals[CHANGED], 0);
 }
 
-
+
 /* GtkObject methods.  */
 
 static void
-finalise(GObject *object)
+finalise (GObject *object)
 {
-	EAttachment *attachment;
+	EAttachment *attachment = (EAttachment *) object;
+	GtkWidget *dialog;
+	
+	if (attachment->editor_gui != NULL) {
+		dialog = glade_xml_get_widget (attachment->editor_gui, "dialog");
+		g_signal_emit_by_name (dialog, "response", GTK_RESPONSE_CLOSE);
+	}
 	
-	attachment = E_ATTACHMENT (object);
-
 	if (attachment->is_available_local) {
 		camel_object_unref (attachment->body);
 		if (attachment->pixbuf_cache != NULL)
@@ -93,14 +98,14 @@ finalise(GObject *object)
 			gnome_vfs_async_cancel(attachment->handle);
 		g_free (attachment->description);
 	}
-
+	
 	g_free (attachment->file_name);
 	g_free (attachment->store_uri);
-
+	
 	G_OBJECT_CLASS (parent_class)->finalize (object);
 }
 
-
+
 /* Signals.  */
 
 static void
@@ -115,7 +120,7 @@ real_update_attachment (EAttachment *attachment, char *msg)
 	g_return_if_fail (E_IS_ATTACHMENT (attachment));
 }
 
-
+
 static void
 class_init (EAttachmentClass *klass)
 {
@@ -218,9 +223,7 @@ attachment_guess_mime_type (const char *file_name)
  * Return value: the new attachment, or %NULL on error
  **/
 EAttachment *
-e_attachment_new (const char *file_name,
-			       const char *disposition,
-			       CamelException *ex)
+e_attachment_new (const char *file_name, const char *disposition, CamelException *ex)
 {
 	EAttachment *new;
 	CamelMimePart *part;
@@ -229,7 +232,7 @@ e_attachment_new (const char *file_name,
 	struct stat statbuf;
 	char *mime_type;
 	char *filename;
-	CamelURL *curl;
+	CamelURL *url;
 	
 	g_return_val_if_fail (file_name != NULL, NULL);
 	
@@ -248,16 +251,14 @@ e_attachment_new (const char *file_name,
 		return NULL;
 	}
 	
-	stream = camel_stream_fs_new_with_name (file_name, O_RDONLY, 0);
-	if (!stream) {
+	if (!(stream = camel_stream_fs_new_with_name (file_name, O_RDONLY, 0))) {
 		camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
 				      _("Cannot attach file %s: %s"),
 				      file_name, g_strerror (errno));
 		return NULL;
 	}
 	
-	mime_type = attachment_guess_mime_type (file_name);
-	if (mime_type) {
+	if ((mime_type = attachment_guess_mime_type (file_name))) {
 		if (!g_ascii_strcasecmp (mime_type, "message/rfc822")) {
 			wrapper = (CamelDataWrapper *) camel_mime_message_new ();
 		} else {
@@ -303,10 +304,10 @@ e_attachment_new (const char *file_name,
 	new->is_available_local = TRUE;
 	new->file_name = filename;
 	
-	curl = camel_url_new ("file:", NULL);
-	camel_url_set_path (curl, file_name);
-	new->store_uri = camel_url_to_string (curl, 0);
-	camel_url_free (curl);
+	url = camel_url_new ("file:", NULL);
+	camel_url_set_path (url, file_name);
+	new->store_uri = camel_url_to_string (url, 0);
+	camel_url_free (url);
 	
 	return new;
 }
@@ -314,8 +315,8 @@ e_attachment_new (const char *file_name,
 
 typedef struct DownloadInfo {
 	EAttachment *attachment;
-	gchar *file_name;
-}DownloadInfo;
+	char *file_name;
+} DownloadInfo;
 
 static int
 async_progress_update_cb (GnomeVFSAsyncHandle      *handle,
@@ -324,7 +325,6 @@ async_progress_update_cb (GnomeVFSAsyncHandle      *handle,
 {
 	switch (info->status) {
 	case GNOME_VFS_XFER_PROGRESS_STATUS_OK:
-	{
 		if (info->file_size) {
 			download_info->attachment->percentage = info->bytes_copied*100/info->file_size;
 			g_signal_emit (download_info->attachment, signals[UPDATE], 0);
@@ -336,6 +336,9 @@ async_progress_update_cb (GnomeVFSAsyncHandle      *handle,
 		if (info->phase == GNOME_VFS_XFER_PHASE_COMPLETED) {
 			CamelException ex;
 			
+			if (!info->file_size)
+				goto error;
+			
 			download_info->attachment->is_available_local = TRUE;
 			download_info->attachment->handle = NULL;
 			camel_exception_init (&ex);
@@ -346,13 +349,12 @@ async_progress_update_cb (GnomeVFSAsyncHandle      *handle,
 			g_free (download_info);
 		}
 		return TRUE;
-	}
 	case GNOME_VFS_XFER_PROGRESS_STATUS_VFSERROR:
-		gnome_vfs_async_cancel (handle);
+	error:
+		g_object_unref (download_info->attachment);
 		g_free (download_info->file_name);
 		g_free (download_info);
 		return FALSE;
-
 	default:
 		break;
 	}
@@ -361,46 +363,42 @@ async_progress_update_cb (GnomeVFSAsyncHandle      *handle,
 }
 
 static void
-download_to_local_path (GnomeVFSURI  *source_uri, GnomeVFSURI  *target_uri, DownloadInfo *download_info)
+download_to_local_path (GnomeVFSURI *source_uri, GnomeVFSURI *target_uri, DownloadInfo *download_info)
 			
 {
-	GnomeVFSResult       result;
-	GList               *source_uri_list = NULL;
-	GList               *target_uri_list = NULL;
-
-	source_uri_list = g_list_prepend (source_uri_list, source_uri);
-	target_uri_list = g_list_prepend (target_uri_list, target_uri);
-
+	GList *source_uri_list;
+	GList *target_uri_list;
+	
+	source_uri_list = g_list_append (NULL, source_uri);
+	target_uri_list = g_list_append (NULL, target_uri);
+	
 	/* Callback info */
-	/* Maybe check the result here somewhere? */
-	result = gnome_vfs_async_xfer (&download_info->attachment->handle,                        /* handle_return   */
-				       source_uri_list,                       /* source_uri_list */
-				       target_uri_list,                       /* target_uri_list */
-				       GNOME_VFS_XFER_DEFAULT,                /* xfer_options    */
-				       GNOME_VFS_XFER_ERROR_MODE_ABORT,       /* error_mode      */ 
-				       GNOME_VFS_XFER_OVERWRITE_MODE_REPLACE, /* overwrite_mode  */ 
-				       GNOME_VFS_PRIORITY_DEFAULT,            /* priority        */
-				       (GnomeVFSAsyncXferProgressCallback) async_progress_update_cb,              /* progress_update_callback */
-				       download_info,                         /* update_callback_data     */
-				       NULL,                                  /* progress_sync_callback   */
-				       NULL);                                 /* sync_callback_data       */
+	gnome_vfs_async_xfer (&download_info->attachment->handle,    /* handle_return   */
+			      source_uri_list,                       /* source_uri_list */
+			      target_uri_list,                       /* target_uri_list */
+			      GNOME_VFS_XFER_DEFAULT,                /* xfer_options    */
+			      GNOME_VFS_XFER_ERROR_MODE_ABORT,       /* error_mode      */ 
+			      GNOME_VFS_XFER_OVERWRITE_MODE_REPLACE, /* overwrite_mode  */ 
+			      GNOME_VFS_PRIORITY_DEFAULT,            /* priority        */
+			      (GnomeVFSAsyncXferProgressCallback) async_progress_update_cb,
+			      download_info,                         /* update_callback_data   */
+			      NULL,                                  /* progress_sync_callback */
+			      NULL);                                 /* sync_callback_data     */
 }
 
 EAttachment *
-e_attachment_new_remote_file (const char *url,
-			      const char *disposition,
-			      const char *path,
-			      CamelException *ex)
+e_attachment_new_remote_file (const char *uri, const char *disposition, const char *path, CamelException *ex)
 {
 	EAttachment *new;
 	DownloadInfo *download_info;
-	gchar *base;
-	CamelURL *curl;
+	CamelURL *url;
+	char *base;
 	
-	g_return_val_if_fail (url != NULL, NULL);
-
-	curl = camel_url_new (url, NULL);
-	base = g_path_get_basename (curl->path);
+	g_return_val_if_fail (uri != NULL, NULL);
+	
+	url = camel_url_new (uri, NULL);
+	base = g_path_get_basename (url->path);
+	camel_url_free (url);
 	
 	new = g_object_new (E_TYPE_ATTACHMENT, NULL);
 	new->editor_gui = NULL;
@@ -411,23 +409,20 @@ e_attachment_new_remote_file (const char *url,
 	new->is_available_local = FALSE;
 	new->percentage = 0;
 	new->file_name = g_build_filename (path, base, NULL);
-
-	g_free(base);
-
+	
+	g_free (base);
+	
 	download_info = g_new (DownloadInfo, 1);
 	download_info->attachment = new;
 	download_info->file_name = g_strdup (new->file_name);
-	download_to_local_path (gnome_vfs_uri_new(url), gnome_vfs_uri_new(new->file_name), download_info);
-
-	camel_url_free (curl);
+	download_to_local_path (gnome_vfs_uri_new (uri), gnome_vfs_uri_new (new->file_name), download_info);
+	
 	return new;
 }
 
+
 void
-e_attachment_build_remote_file (const char *file_name,
-					     EAttachment *attachment,
-					     const char *disposition,
-					     CamelException *ex)
+e_attachment_build_remote_file (const char *file_name, EAttachment *attachment, const char *disposition, CamelException *ex)
 {
 	CamelMimePart *part;
 	CamelDataWrapper *wrapper;
@@ -435,11 +430,11 @@ e_attachment_build_remote_file (const char *file_name,
 	struct stat statbuf;
 	char *mime_type;
 	char *filename;
-	CamelURL *curl;
+	CamelURL *url;
 	
 	g_return_if_fail (file_name != NULL);
 	
-	if (g_stat (file_name, &statbuf) < 0) {
+	if (g_stat (file_name, &statbuf) == -1) {
 		camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
 				      _("Cannot attach file %s: %s"),
 				      file_name, g_strerror (errno));
@@ -456,16 +451,14 @@ e_attachment_build_remote_file (const char *file_name,
 		return;
 	}
 	
-	stream = camel_stream_fs_new_with_name (file_name, O_RDONLY, 0);
-	if (!stream) {
+	if (!(stream = camel_stream_fs_new_with_name (file_name, O_RDONLY, 0))) {
 		camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
 				      _("Cannot attach file %s: %s"),
 				      file_name, g_strerror (errno));
 		return;
 	}
 	
-	mime_type = attachment_guess_mime_type (file_name);
-	if (mime_type) {
+	if ((mime_type = attachment_guess_mime_type (file_name))) {
 		if (!g_ascii_strcasecmp (mime_type, "message/rfc822")) {
 			wrapper = (CamelDataWrapper *) camel_mime_message_new ();
 		} else {
@@ -512,13 +505,13 @@ e_attachment_build_remote_file (const char *file_name,
 	g_free (attachment->file_name);
 	attachment->file_name = filename;
 	
-	curl = camel_url_new ("file:", NULL);
-	camel_url_set_path (curl, file_name);
-	attachment->store_uri = camel_url_to_string (curl, 0);
-	camel_url_free (curl);
+	url = camel_url_new ("file:", NULL);
+	camel_url_set_path (url, file_name);
+	attachment->store_uri = camel_url_to_string (url, 0);
+	camel_url_free (url);
 
 }
-
+
 
 /**
  * e_attachment_new_from_mime_part:
@@ -604,8 +597,6 @@ close_cb (GtkWidget *widget, gpointer data)
 	g_object_unref (attachment->editor_gui);
 	attachment->editor_gui = NULL;
 	
-	g_object_unref (attachment);
-	
 	destroy_dialog_data (dialog_data);
 }
 
@@ -638,7 +629,7 @@ ok_cb (GtkWidget *widget, gpointer data)
 		camel_mime_part_set_content_type (attachment->body, str);
 		camel_data_wrapper_set_mime_type(camel_medium_get_content_object(CAMEL_MEDIUM (attachment->body)), str);
 	}
-
+	
 	if (attachment->is_available_local) {
 		switch (gtk_toggle_button_get_active (dialog_data->disposition_checkbox)) {
 		case 0:
@@ -675,28 +666,22 @@ e_attachment_edit (EAttachment *attachment, GtkWidget *parent)
 	const char *disposition;
 	DialogData *dialog_data;
 	GladeXML *editor_gui;
+	GtkWidget *window;
 	char *type;
 	char *filename;
 	
-	g_return_if_fail (attachment != NULL);
 	g_return_if_fail (E_IS_ATTACHMENT (attachment));
 	
 	if (attachment->editor_gui != NULL) {
-		GtkWidget *window;
-		
-		window = glade_xml_get_widget (attachment->editor_gui,
-					       "dialog");
+		window = glade_xml_get_widget (attachment->editor_gui, "dialog");
 		gdk_window_show (window->window);
 		return;
 	}
 	
-	filename = g_build_filename (EVOLUTION_GLADEDIR,
-				     "e-attachment.glade",
-				     NULL);
-	editor_gui = glade_xml_new (filename,
-				    NULL, NULL);
+	filename = g_build_filename (EVOLUTION_GLADEDIR, "e-attachment.glade", NULL);
+	editor_gui = glade_xml_new (filename, NULL, NULL);
 	g_free (filename);
-
+	
 	if (editor_gui == NULL) {
 		g_warning ("Cannot load `e-attachment.glade'");
 		return;
@@ -704,59 +689,47 @@ e_attachment_edit (EAttachment *attachment, GtkWidget *parent)
 	
 	attachment->editor_gui = editor_gui;
 	
-	gtk_window_set_transient_for
-		(GTK_WINDOW (glade_xml_get_widget (editor_gui, "dialog")),
-		 GTK_WINDOW (gtk_widget_get_toplevel (parent)));
+	gtk_window_set_transient_for (GTK_WINDOW (glade_xml_get_widget (editor_gui, "dialog")),
+				      GTK_WINDOW (gtk_widget_get_toplevel (parent)));
 	
 	dialog_data = g_new (DialogData, 1);
-	g_object_ref (attachment);
 	dialog_data->attachment = attachment;
 	dialog_data->dialog = glade_xml_get_widget (editor_gui, "dialog");
-	dialog_data->file_name_entry = GTK_ENTRY (
-		glade_xml_get_widget (editor_gui, "file_name_entry"));
-	dialog_data->description_entry = GTK_ENTRY (
-		glade_xml_get_widget (editor_gui, "description_entry"));
-	dialog_data->mime_type_entry = GTK_ENTRY (
-		glade_xml_get_widget (editor_gui, "mime_type_entry"));
-	dialog_data->disposition_checkbox = GTK_TOGGLE_BUTTON (
-		glade_xml_get_widget (editor_gui, "disposition_checkbox"));
-
-	if (attachment->is_available_local)	{
-		set_entry (editor_gui, "file_name_entry",
-			   camel_mime_part_get_filename (attachment->body));
-		set_entry (editor_gui, "description_entry",
-			   camel_mime_part_get_description (attachment->body));
+	dialog_data->file_name_entry = GTK_ENTRY (glade_xml_get_widget (editor_gui, "file_name_entry"));
+	dialog_data->description_entry = GTK_ENTRY (glade_xml_get_widget (editor_gui, "description_entry"));
+	dialog_data->mime_type_entry = GTK_ENTRY (glade_xml_get_widget (editor_gui, "mime_type_entry"));
+	dialog_data->disposition_checkbox = GTK_TOGGLE_BUTTON (glade_xml_get_widget (editor_gui, "disposition_checkbox"));
+	
+	if (attachment->is_available_local) {
+		set_entry (editor_gui, "file_name_entry", camel_mime_part_get_filename (attachment->body));
+		set_entry (editor_gui, "description_entry", camel_mime_part_get_description (attachment->body));
 		content_type = camel_mime_part_get_content_type (attachment->body);
 		type = camel_content_type_simple (content_type);
 		set_entry (editor_gui, "mime_type_entry", type);
 		g_free (type);
-	
+		
 		disposition = camel_mime_part_get_disposition (attachment->body);
 		gtk_toggle_button_set_active (dialog_data->disposition_checkbox,
 					      disposition && !g_ascii_strcasecmp (disposition, "inline"));
 	} else {
-		set_entry (editor_gui, "file_name_entry",
-			   attachment->file_name);
-		set_entry (editor_gui, "description_entry",
-			   attachment->description);
-		type = attachment_guess_mime_type (attachment->file_name);
-		if (type) {
+		set_entry (editor_gui, "file_name_entry", attachment->file_name);
+		set_entry (editor_gui, "description_entry", attachment->description);
+		if ((type = attachment_guess_mime_type (attachment->file_name))) {
 			set_entry (editor_gui, "mime_type_entry", type);
 			g_free (type);
 		} else {
 			set_entry (editor_gui, "mime_type_entry", "");
 		}
-	
-		gtk_toggle_button_set_active (dialog_data->disposition_checkbox, attachment->disposition);
 		
+		gtk_toggle_button_set_active (dialog_data->disposition_checkbox, attachment->disposition);
 	}
 	
 	connect_widget (editor_gui, "dialog", "response", (GCallback)response_cb, dialog_data);
-
+	
 	/* make sure that when the parent gets hidden/closed that our windows also close */
 	parent = gtk_widget_get_toplevel (parent);
-	gtk_signal_connect_while_alive (GTK_OBJECT (parent), "destroy", (GCallback)close_cb, dialog_data,
+	gtk_signal_connect_while_alive (GTK_OBJECT (parent), "destroy", (GCallback) close_cb, dialog_data,
 					GTK_OBJECT (dialog_data->dialog));
-	gtk_signal_connect_while_alive (GTK_OBJECT (parent), "hide", (GCallback)close_cb, dialog_data,
+	gtk_signal_connect_while_alive (GTK_OBJECT (parent), "hide", (GCallback) close_cb, dialog_data,
 					GTK_OBJECT (dialog_data->dialog));
 }