[Evolution-hackers] interesting code ...



Or at least ... interesting choice of algorithms.  I sure hope this
isn't called very often?

2 N^2 loops ... AND it accesses data AFTER it is all freed (the uid's
are no longer reffed after you free the objects which hold them, it just
happens that because of other reasons this will usually work).

I gotta say the whole gw code looks mighty odd, whomever wrote it
obviously loved using singly linked lists - about the most inefficient
choice imaginable in this case, particularly given the data in question
is already in an array in all the cases i've seen (these two below are
more than enough for now, thank-you-very-much).

static void
gw_update_all_items ( CamelFolder *folder, GSList *item_list, CamelException *ex) 
{
	CamelGroupwiseFolder *gw_folder = CAMEL_GROUPWISE_FOLDER (folder);
	GPtrArray *summary = camel_folder_get_summary (folder);
	int index = 0;
	GSList *item_ids = NULL, *l = NULL;
	CamelFolderChangeInfo *changes = NULL;

	changes = camel_folder_change_info_new ();
	/*item_ids : List of ids from the summary*/
	while (index < summary->len) {
		CamelMessageInfo *info = g_ptr_array_index (summary, index);
		item_ids = g_slist_append (item_ids, info->uid);
		index ++;
	}
	l = item_ids;
	camel_folder_free_summary (folder, summary);

	changes = camel_folder_change_info_new ();
	/*item_list : List of ids from the server*/
	for (; item_ids != NULL ; item_ids = g_slist_next (item_ids)) {
		GSList *temp = NULL;
		temp = g_slist_find_custom (item_list, (const char *)item_ids->data, (GCompareFunc) strcmp);
		if (!temp) {
			camel_folder_summary_remove_uid (folder->summary, (const char *)item_ids->data);
			camel_data_cache_remove(gw_folder->cache, "cache", (const char *)item_ids->data, ex);
			camel_folder_change_info_remove_uid(changes, (const char *)item_ids->data);
		}
	}
	camel_object_trigger_event (folder, "folder_changed", changes);

	g_slist_free (l);
}

Oh boy, here's another gem of a fragment.  Absolutely brilliant.

Of particular note is the ingenous inner-loop which iterates over a list
which was created from an array .. but then indexes into the array
anyway, ignoring all of the data in the list in the first place.  The
list is never freed either.  I see the list is cunningly being used as a
loop counter!  It could be worse, I guess you could've implemented the
list consumer recursively ...

Note also that 'count' initialised in the first line - is never used.

At least it doesn't try to de-reference previously freed data (but i
wouldn't bet on that!).  Although the code following that pasted here
does a wonderful job of leaking not once, but twice for every
messageinfo it looks up and creates.  Oh wait, this leaks the
messageinfo's too.

 	count = camel_folder_summary_count (destination->summary);
 	qsort (uids->pdata, uids->len, sizeof (void *), uid_compar);

	while (index < uids->len) {
		item_ids = g_list_append (item_ids, g_ptr_array_index (uids, index));
		index ++;
	}

	if (transferred_uids)
		*transferred_uids = NULL;

	if (delete_originals) 
		source_container_id = camel_groupwise_store_container_id_lookup (gw_store, source->name);
	else
		source_container_id = NULL;
	dest_container_id = camel_groupwise_store_container_id_lookup (gw_store, destination->name);

	CAMEL_SERVICE_LOCK (source->parent_store, connect_lock);
	/* check for offline operation */
	if (offline->state == CAMEL_OFFLINE_STORE_NETWORK_UNAVAIL) {
		CamelGroupwiseJournal *journal = (CamelGroupwiseJournal *) ((CamelGroupwiseFolder *) destination)->journal;
		CamelMimeMessage *message;
		GList *l;
		int i;
		
		for (l = item_ids, i = 0; l; l = l->next, i++) {
			CamelMessageInfo *info;

			if (!(info = camel_folder_summary_uid (source->summary, uids->pdata[i])))
				continue;
			
			if (!(message = groupwise_folder_get_message (source, camel_message_info_uid (info), ex)))
				break;
			
			camel_groupwise_journal_transfer (journal, (CamelGroupwiseFolder *)source, message, info, uids->pdata[i], NULL, ex);
			camel_object_unref (message);
			
			if (camel_exception_is_set (ex))

Sorry for being so facetious ... I know it is hard writing to an
undocumented API ... but this really is pretty poor for such basic
stuff.  Sigh.





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]