GNOME.org
 

Re: Patch: add weak references to current_folder, old_folder and last_folder in imap store

El mar, 06-05-2008 a las 14:50 +0200, Philip Van Hoof escribió:
> Prepend the private symbols with an underscore, use a -priv.h file, and
> then this is approved.

	New version of the patch, with the recommendations. Changelog entry
would be this time:
* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.[ch]:
        * Now references to current_folder, old_folder and last_folder
          are weak references. This way, when they're finalized we
          properly remove the store reference to them.
        * Removed some small leaks and compilation warnings.
* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store-priv.h:
	* Added. Includes internal declarations of weak reference handlers
	  for folders in the imap store.
* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c,
  libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c:
        * Use weak references to folders in accesses to store.

> 
> 
> On Tue, 2008-05-06 at 13:57 +0200, José Dapena Paz wrote:
> > 	Hi,
> > 
> > 	This patch adds weak references to current_folder, old_folder and
> > last_folder in imap store, to make them be removed when the objects are
> > finalized. This should prevent some crashes and memory corruptions
> > caused by accessing these folders after they're freed.
> > 
> > 	The patch does not include Changelog entry. It would be:
> > 
> > * libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.[ch]:
> > 	* Now references to current_folder, old_folder and last_folder
> > 	  are weak references. This way, when they're finalized we
> > 	  properly remove the store reference to them.
> > 	* Removed some small leaks and compilation warnings.
> > * libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c,
> >   libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c:
> > 	* Use weak references to folders in accesses to store.
> > _______________________________________________
> > tinymail-devel-list mailing list
> > tinymail-devel-list gnome org
> > http://mail.gnome.org/mailman/listinfo/tinymail-devel-list
-- 
José Dapena Paz <jdapena igalia com>
Igalia

Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(revision 3640)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(working copy)
@@ -70,6 +70,7 @@
 #include "camel-imap-message-cache.h"
 #include "camel-imap-store-summary.h"
 #include "camel-imap-store.h"
+#include "camel-imap-store-priv.h"
 #include "camel-imap-summary.h"
 #include "camel-imap-utils.h"
 
@@ -321,6 +322,7 @@
 			camel_imap_folder_selected (folder, response2, &nex, TRUE);
 			camel_imap_response_free (imap_store, response2);
 		}
+		camel_exception_clear (&nex);
 		camel_imap_store_start_idle (imap_store);
 	}
 
@@ -395,6 +397,18 @@
 
 	let_idle_die (imap_store, TRUE);
 
+	if (imap_store->current_folder) {
+		camel_object_unhook_event (imap_store->current_folder, "finalize",
+					   _camel_imap_store_current_folder_finalize, imap_store);
+		imap_store->current_folder = NULL;
+	}
+
+	if (imap_store->old_folder) {
+		camel_object_unhook_event (imap_store->old_folder, "finalize",
+					   _camel_imap_store_old_folder_finalize, imap_store);
+		imap_store->old_folder = NULL;
+	}
+
 	/* This frees current_folder, folders, authtypes, streams, and namespace. */
 	camel_service_disconnect((CamelService *)imap_store, TRUE, NULL);
 
@@ -901,7 +915,7 @@
 	CamelImapResponse *response;
 	CamelStream *tcp_stream;
 	CamelSockOptData sockopt;
-	gboolean force_imap4 = FALSE, ssl_ena = FALSE;
+	gboolean force_imap4 = FALSE;
 	gboolean clean_quit = TRUE;
 	char *buf;
 	gboolean not_ssl = TRUE;
@@ -2121,6 +2135,12 @@
 	/* if (store->current_folder && CAMEL_IS_OBJECT (store->current_folder))
 		camel_object_unref (store->current_folder); */
 	store->old_folder = store->current_folder;
+	if (store->old_folder)
+		camel_object_hook_event (store->old_folder, "finalize",
+					 _camel_imap_store_old_folder_finalize, store);
+	if (store->current_folder)
+		camel_object_unhook_event (store->current_folder, "finalize",
+					   _camel_imap_store_current_folder_finalize, store);
 	store->current_folder = NULL;
 
 	if (store->authtypes) {
@@ -2629,6 +2649,8 @@
 
 	if (imap_store->current_folder) {
 		/* camel_object_unref (imap_store->current_folder); */
+		camel_object_unhook_event (imap_store->current_folder, "finalize",
+					   _camel_imap_store_current_folder_finalize, imap_store);
 		imap_store->current_folder = NULL;
 	}
 
@@ -2806,6 +2828,8 @@
 		CamelException local_ex;
 
 		imap_store->current_folder = new_folder;
+		camel_object_hook_event (imap_store, "finalize",
+					 _camel_imap_store_current_folder_finalize, imap_store);
 		/* camel_object_ref (new_folder); */
 		camel_exception_init (&local_ex);
 		//camel_imap_folder_selected (new_folder, response, &local_ex, TRUE);
@@ -2813,6 +2837,8 @@
 		if (camel_exception_is_set (&local_ex)) {
 			camel_exception_xfer (ex, &local_ex);
 			/* camel_object_unref (imap_store->current_folder); */
+			camel_object_unhook_event (imap_store, "finalize",
+						   _camel_imap_store_current_folder_finalize, imap_store);
 			imap_store->current_folder = NULL;
 			camel_object_unref (new_folder);
 			new_folder = NULL;
@@ -2881,6 +2907,8 @@
 		camel_object_unref (imap_store->current_folder);*/
 
 	/* no need to actually create a CamelFolder for INBOX */
+	camel_object_unhook_event (imap_store->current_folder, "finalize",
+				   _camel_imap_store_current_folder_finalize, imap_store);
 	imap_store->current_folder = NULL;
 
 	response = camel_imap_command(imap_store, NULL, ex, "DELETE %F", folder_name);
@@ -3046,7 +3074,7 @@
 
 		g_mkdir (newpath, S_IRWXU);
 
-		while (file = g_dir_read_name (dir)) {
+		while ((file = g_dir_read_name (dir)) != NULL) {
 			gchar *old_fullname;
 			gchar *new_fullname;
 			old_fullname = g_strdup_printf ("%s/%s", oldpath, file);
@@ -3105,6 +3133,8 @@
 	/*if (imap_store->current_folder)
 		camel_object_unref (imap_store->current_folder); */
 	/* no need to actually create a CamelFolder for INBOX */
+	camel_object_unhook_event (imap_store->current_folder, "finalize",
+				   _camel_imap_store_current_folder_finalize, imap_store);
 	imap_store->current_folder = NULL;
 
 	/* Undefined progress */
@@ -4454,3 +4484,26 @@
 
 	return nread;
 }
+
+void 
+_camel_imap_store_current_folder_finalize (CamelObject *stream, gpointer event_data, gpointer user_data)
+{
+	CamelImapStore *store = (CamelImapStore *) user_data;
+
+	store->current_folder = NULL;
+}
+void 
+_camel_imap_store_old_folder_finalize (CamelObject *stream, gpointer event_data, gpointer user_data)
+{
+	CamelImapStore *store = (CamelImapStore *) user_data;
+
+	store->old_folder = NULL;
+}
+
+void 
+_camel_imap_store_last_folder_finalize (CamelObject *stream, gpointer event_data, gpointer user_data)
+{
+	CamelImapStore *store = (CamelImapStore *) user_data;
+
+	store->last_folder = NULL;
+}
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(revision 3640)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(working copy)
@@ -89,6 +89,7 @@
 #include "camel-imap-private.h"
 #include "camel-imap-search.h"
 #include "camel-imap-store.h"
+#include "camel-imap-store-priv.h"
 #include "camel-imap-summary.h"
 #include "camel-imap-utils.h"
 #include "camel-imap-wrapper.h"
@@ -798,8 +799,11 @@
 		imap_folder->idle_lock = NULL;
 	}
 
-	if (store->current_folder == (CamelFolder*) object)
+	if (store->current_folder == (CamelFolder*) object) {
+		camel_object_unhook_event (store->current_folder, "finalize",
+					   _camel_imap_store_current_folder_finalize, store);
 		store->current_folder = NULL;
+	}
 
 	if (imap_folder->search)
 		camel_object_unref (CAMEL_OBJECT (imap_folder->search));
@@ -3826,6 +3830,7 @@
 		if (tbreak)
 			break;
 	}
+	camel_exception_clear (&ex);
 	if (resp)
 		g_free (resp);
 errh:
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c	(revision 3640)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-command.c	(working copy)
@@ -45,6 +45,7 @@
 #include "camel-imap-folder.h"
 #include "camel-imap-store-summary.h"
 #include "camel-imap-store.h"
+#include "camel-imap-store-priv.h"
 #include "camel-imap-utils.h"
 #include "camel-imap-summary.h"
 
@@ -135,15 +136,27 @@
 		if (store->current_folder && CAMEL_IS_OBJECT (store->current_folder))
 			camel_object_unref(store->current_folder); */
 
-		if (!folder)
+		if (!folder) {
 			store->last_folder = store->current_folder;
-		else {
+			if (store->last_folder)
+				camel_object_hook_event (store->last_folder, "finalize",
+							 _camel_imap_store_last_folder_finalize, store);
+		} else {
 			modseq = camel_imap_folder_get_highestmodseq (CAMEL_IMAP_FOLDER (folder));
+			if (store->last_folder)
+				camel_object_unhook_event (store->last_folder, "finalize",
+							   _camel_imap_store_last_folder_finalize, store);
 			store->last_folder = NULL;
 		}
-
+		
+		if (store->current_folder)
+			camel_object_unhook_event (store->current_folder, "finalize",
+						   _camel_imap_store_current_folder_finalize, store);
 		store->current_folder = folder;
-
+		if (store->current_folder)
+			camel_object_hook_event (store->current_folder, "finalize",
+						 _camel_imap_store_current_folder_finalize, store);
+		
 		if (modseq && (store->capabilities & IMAP_CAPABILITY_QRESYNC))
 		{
 			CamelImapSummary *imap_summary = CAMEL_IMAP_SUMMARY (folder->summary);
@@ -285,11 +298,19 @@
 		gboolean wasnull = FALSE;
 		if (!store->current_folder) {
 			store->current_folder = store->last_folder;
+			if (store->current_folder)
+				camel_object_hook_event (store->current_folder, "finalize",
+							 _camel_imap_store_current_folder_finalize, store);
 			wasnull = TRUE;
 		}
 		camel_imap_store_stop_idle (store);
-		if (wasnull)
-			store->current_folder = NULL;
+		if (wasnull) {
+			if (store->current_folder) {
+				camel_object_unhook_event (store->current_folder, "finalize",
+							   _camel_imap_store_current_folder_finalize, store);
+				store->current_folder = NULL;
+			}
+		}
 	}
 
 	/* Check for current folder */
@@ -1283,9 +1304,9 @@
 
 		start = *p ? p + 1 : p;
 	}
-
+	
 	g_ptr_array_free (args, TRUE);
-
+	
 	return out;
 }
 
@@ -1294,10 +1315,10 @@
 {
 	va_list ap;
 	char *result;
-
+	
 	va_start (ap, fmt);
 	result = imap_command_strdup_vprintf (store, fmt, ap);
 	va_end (ap);
-
+	
 	return result;
 }
Index: libtinymail-camel/camel-lite/camel/providers/imap/Makefile.am
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/Makefile.am	(revision 3640)
+++ libtinymail-camel/camel-lite/camel/providers/imap/Makefile.am	(working copy)
@@ -33,6 +33,7 @@
 	camel-imap-message-cache.h		\
 	camel-imap-search.h			\
 	camel-imap-store.h			\
+	camel-imap-store-priv.h			\
 	camel-imap-store-summary.h		\
 	camel-imap-summary.h			\
 	camel-imap-types.h			\
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store-priv.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store-priv.h	(revision 0)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store-priv.h	(revision 0)
@@ -0,0 +1,36 @@
+#ifndef CAMEL_IMAP_STORE_PRIV_H
+#define CAMEL_IMAP_STORE_PRIV_H 1
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */
+/* camel-imap-store-priv.h : class for an imap store */
+
+/*
+ * Authors: Jeffrey Stedfast <fejj ximian com>
+ *
+ * Copyright (C) 2000 Ximian, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU Lesser General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
+ * USA
+ */
+
+#include <camel/camel-object.h>
+
+G_BEGIN_DECLS
+
+void _camel_imap_store_current_folder_finalize (CamelObject *stream, gpointer event_data, gpointer user_data);
+void _camel_imap_store_old_folder_finalize (CamelObject *stream, gpointer event_data, gpointer user_data);
+void _camel_imap_store_last_folder_finalize (CamelObject *stream, gpointer event_data, gpointer user_data);
+
+G_END_DECLS
+
+#endif /* CAMEL_IMAP_STORE_PRIV_H */
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]