Re: [evolution-patches] Fix for the bug #61865 and 'delta' related issues[GW Calendar]

[Sivaiah Nallagatla <snallagatla novell com>]

Here are comments on groupwise related part

while handling New items 

+ if (!comp)
+ g_message ("Invalid component returned");
+ else if (!e_cal_backend_cache_put_component (cache, comp)) 
+ g_message ("Could not add the component");
+ e_cal_backend_notify_object_created (E_CAL_BACKEND (cbgw),
e_cal_component_get_as_string (comp));

we seem to be going ahead and using comp even when comp is NULL, and
also the string returned from e_cal_component_get_as_string is leaked
here and at other places in get_deltas function

while handling modified items , we are not freeing the item_list 

while handling deleted items 
cache_keys = e_cal_backend_cache_get_keys (cache);
+	for (l = item_list; l; l = g_slist_next (l)) {
+		/* this works assuming rid is null*/
+		cache_keys = g_slist_delete_link (cache_keys, 
+				g_slist_find_custom (cache_keys, l->data, (GCompareFunc) strcmp));
+	}

is there any possibility of g_slist_find_custom reurning NULL. Alos the
removed node is  leaked here

item_list and cahce_key lists also should be freed 

g_object_ref (priv->cnc);
g_object_ref (priv->cache);
priv->timeout_id = g_timeout_add (CACHE_REFRESH_INTERVAL, (GSourceFunc)
get_deltas, (gpointer) cbgw);

there seems to be no corresponding unrefs for these 

+			/* FIXME  check if list is null and status may have
+			 * failed. */
+			comp = g_object_ref ( (ECalComponent *) list->data );
we seem to be not using this comp anywhere

in e_gw_connection_get_quick_messages

+			id = g_strdup (soup_soap_parameter_get_string_value (param_id));
+			if (id)
+				*item_list = g_slist_append (*item_list, id);

no need to dup the id here , it is being leaked now


Thanks,
Siva
On Tue, 2004-09-14 at 21:06 +0530, Harish Krishnaswamy wrote: 
> hi,
> 
>  The adjoining patch addresses the bug #61865 and all other bugs related
> to the issue of not getting changes from the server. It uses the
> getQuickMessagesRequest and computes the deltas (for deletes) and
> discontinues the use of getDeltasRequest, which was not working as
> expected. One utility method has been added to e-file-cache and e-cal-
> backend-cache,  as part of the fix.
> 
> The calendar cache is being wiped clean and recreated once per eds
> session as a work-around for getDeltas issue. With the current fix in,
> the cache is built only once when the account is set up. All subsequent
> sessions merely obtain the deltas, this should hopefully solve a big
> chunk of performance problems (#63374) by speeding up the calendar load-
> time.
> 
> The patch also includes a small change to address an as yet unreported
> problem, one that would have been uncovered by this fix - When a
> calendar item that originated from a GW client (other than evo) and
> processed by us, a duplicate item was being created as the same item
> retrieved from the itip interface and the SOAP interface carried
> different iCalIds. This was unobservable to most users, since the client
> did not receive the changes  properly while it was running- the
> duplicate item was not visible to the user and when s/he restarted the
> client - the cache was wiped clean.
> The same has been addressed by forcing a read from the server before
> adding the items retrieved from itip, onto the cache.
> 
> 
> harish
>