Re: [evolution-patches] Exchange connector: Fix for 73494 and 72100

["Ahmed Sarfraaz" <asarfraaz novell com>]

Few comments below

Sushma Rai wrote:

>Hi,
>
>Please review this revised patch, incorporated 
>Michael's comment and also, did modification to
>forming internal uri during folder creation.
>
>Thanks,
>Sushma.
>
>>>Index: storage/exchange-folder-size.c
>>>===================================================================
>>> static void
>>>@@ -73,10 +75,12 @@ finalize (GObject *object)
>>> {
>>> 	ExchangeFolderSize *fsize = EXCHANGE_FOLDER_SIZE (object);
>>> 
>>>+	g_hash_table_foreach_remove (fsize->priv->table,
free_fsize_table, NULL);
>>> 	g_hash_table_destroy (fsize->priv->table);
>>> 	g_hash_table_destroy (fsize->priv->row_refs);
>
While you are at this, free the row_refs as well.

>>>+void
>>>+exchange_folder_size_remove (ExchangeFolderSize *fsize, 
>>>+				const char *folder_name)
>>>+{
>>>

>>>+	cached_info = g_hash_table_lookup (folder_size_table,
folder_name);
>>>+	if (cached_info)  {
>>>+		row = g_hash_table_lookup (priv->row_refs, folder_name);
>>>+		path = gtk_tree_row_reference_get_path (row);
>>>+		g_hash_table_remove (folder_size_table,
cached_info->folder_name);
>>>
Minor : Guess, you could use "folder_name" instead of
"cached_info->folder_name", for consistency.

>>>+		if (gtk_tree_model_get_iter (GTK_TREE_MODEL
(fsize->priv->model), &iter, path)) {
>>>+			gtk_list_store_remove (fsize->priv->model,
&iter);
>>>+		}
>>>+		gtk_tree_path_free (path);
>>>
Free and remove the row_reference as well.
Free the cached_info value after removing it from the cache.

>>> 
>>>Index: storage/exchange-hierarchy-webdav.c
>>>===================================================================
>>>RCS file:
/cvs/gnome/evolution-exchange/storage/exchange-hierarchy-webdav.c,v
>>>retrieving revision 1.27
>>>diff -u -p -r1.27 exchange-hierarchy-webdav.c
>>>--- storage/exchange-hierarchy-webdav.c	9 Mar 2005 07:50:40
-0000	1.27
>>>+++ storage/exchange-hierarchy-webdav.c	17 Mar 2005 08:31:01
-0000
>>>@@ -273,6 +273,8 @@ e_folder_webdav_new (ExchangeHierarchy *
>>> 						physical_uri,
internal_uri);
>>> 	} else {
>>> 		http_uri = e2k_uri_concat
(e_folder_exchange_get_internal_uri (parent), name);
>>>+		/* appending "/" here, so that hash table lookup in
rescan() succeeds */
>>>+		http_uri = e2k_uri_concat (http_uri, "/");
>>>
A better solution could be to append the forward slash to name [ if it
already doesnt have it ] and call e2k_uri_concat just once. We are also
leaking the string [ http_uri ] returned in the first call here.

>>> 	
>>>Index: lib/e2k-uri.c
>>>===================================================================
>>>RCS file: /cvs/gnome/evolution-exchange/lib/e2k-uri.c,v
>>>retrieving revision 1.2
>>>diff -u -p -r1.2 e2k-uri.c
>>>--- lib/e2k-uri.c	22 Jun 2004 15:21:59 -0000	1.2
>>>+++ lib/e2k-uri.c	17 Mar 2005 08:31:13 -0000
>>>@@ -387,10 +387,16 @@ e2k_uri_concat (const char *uri_prefix, 
>>> 	const char *p;
>>> 
>>> 	p = strrchr (uri_prefix, '/');
>>>-	if (p && !p[1])
>>>+	if ((p && !p[1]) || *tail == '/') {
>>>+		/* uri_prefix ends with "/" or tail is a "/", i.e.,
>>>+		 * trying to append a "/" to the uri 
>>>+		 */
>>> 		return g_strdup_printf ("%s%s", uri_prefix, tail);
>>>-	else
>>>+	}
>>>+	else {
>>>+		/* uri_prefix doesn't contain a trailing "/" */
>>> 		return g_strdup_printf ("%s/%s", uri_prefix, tail);
>>>+	}
>>> }
>>> 
>>> /**
>>>
With the above modification in e_folder_webdav_new, this change will no
longer be needed.

-- Sarfraaz