Re: [evolution-patches] Fix for bug 72145 (groupwise provider)

[Parthasarathi Susarla <sparthasarathi novell com>]

Hi Michael,
I used GString and rewrote the patch. 
Thanks
-partha
n Fri, 2005-03-04 at 10:57 +0800, Not Zed wrote:
> 
> It is rather redundant that you're using g_strdup() on a value you
> then free ...  but you shouldn't even be doing it anyway.
> 
> Problems:
> 
> You should use a GString if you're going to build strings.  Using
> g_strcat() is very very inefficient for many reasons.  Not to mention
> that you are leaking the previous string every time through the loop.
> It is more efficient also if you allocate a single GString outside of
> the outside loop of this function, and just do g_string_truncate(s, 0)
> before the recp_list loop.
> 
> Since you are using the default camel-folder-summary 'messageinfo'
> virtual methods (and you should continue to do this), you MUST
> allocate the strings the way they are expected.  Use
> camel_pstring_strdup(), these strings are uniquified and ref-counted,
> it saves significant memory (not to mention, it will crash reliably if
> you don't).  This goes for all of the strings in the base MessageInfo
> structure, except "uid" which must be a normal g_malloc'd string.
> 
> 
> On Thu, 2005-03-03 at 17:14 +0530, Parthasarathi Susarla wrote:
> > +                               for (rl = recp_list; rl != NULL; rl
> > = rl->next) {
> > +                                       EGwItemRecipient *recp =
> > (EGwItemRecipient *) rl->data;
> > +                                       if (recp->type ==
> > E_GW_ITEM_RECIPIENT_TO) {
> > +                                               if (i)
> > +                                                       str =
> > g_strconcat (str, ", ", NULL);
> > +                                               str = g_strconcat
> > (str, recp->display_name,"<",
> > +
> > recp->email,">", NULL);
> > +                                       }
> > +                                       i++;
> > +                               }
> > +                               mi->info.to = g_strdup(str);
> > +                               g_free (str);
> >                         }

Index: camel-groupwise-folder.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/camel/providers/groupwise/camel-groupwise-folder.c,v
retrieving revision 1.36
diff -u -p -r1.36 camel-groupwise-folder.c
--- camel-groupwise-folder.c	10 Mar 2005 06:59:14 -0000	1.36
+++ camel-groupwise-folder.c	10 Mar 2005 07:39:51 -0000
@@ -713,6 +721,8 @@ gw_update_summary ( CamelFolder *folder,
 	guint32 item_status, status_flags = 0;
 	CamelFolderChangeInfo *changes = NULL ;
 	int scount ;
+	gboolean exists = FALSE ;
+	char *uid = NULL;
 	
 //	CAMEL_SERVICE_ASSERT_LOCKED (gw_store, connect_lock);
 
@@ -732,18 +742,10 @@ gw_update_summary ( CamelFolder *folder,
 
 		id = e_gw_item_get_id (item) ;
 		mi = (CamelGroupwiseMessageInfo *)camel_folder_summary_uid (folder->summary, id) ;
+		if (mi) 
+			exists = TRUE ;
 
-		if (mi) {
-			/*Message exists*/
-			item_status = e_gw_item_get_item_status (item);
-			/*if (item_status & E_GW_ITEM_STAT_DELETED)
-				status_flags |= CAMEL_MESSAGE_DELETED;*/
-			if (item_status & E_GW_ITEM_STAT_REPLIED)
-				status_flags |= CAMEL_MESSAGE_ANSWERED;
-			mi->info.flags |= status_flags;
-			camel_folder_change_info_change_uid (changes, e_gw_item_get_id (item)) ;
-		} else {
-			/*New message*/
+		if (!exists) {
 			mi = camel_message_info_new (folder->summary) ; 
 			if (mi->info.content == NULL) {
 				mi->info.content = camel_folder_summary_content_info_new (folder->summary);
@@ -751,65 +753,77 @@ gw_update_summary ( CamelFolder *folder,
 			}
 
 			type = e_gw_item_get_item_type (item) ;
-
-
-			if (type == E_GW_ITEM_TYPE_CONTACT) {
+			if ((type == E_GW_ITEM_TYPE_CONTACT) || (type == E_GW_ITEM_TYPE_UNKNOWN)) {
+				exists = FALSE;
 				continue ;
+			}
 
-			} else if (type == E_GW_ITEM_TYPE_UNKNOWN) 
-				continue ;
+		}
 
-			status_flags = 0;
-			item_status = e_gw_item_get_item_status (item);
-			if (item_status & E_GW_ITEM_STAT_READ)
-				status_flags |= CAMEL_MESSAGE_SEEN;
-			/*if (item_status & E_GW_ITEM_STAT_DELETED)
-				status_flags |= CAMEL_MESSAGE_DELETED;*/
-			if (item_status & E_GW_ITEM_STAT_REPLIED)
-				status_flags |= CAMEL_MESSAGE_ANSWERED;
-			mi->info.flags |= status_flags;
-
-			attach_list = e_gw_item_get_attach_id_list (item) ;
-			if (attach_list)  
-				mi->info.flags |= CAMEL_MESSAGE_ATTACHMENTS;
-
-			org = e_gw_item_get_organizer (item) ; 
-			if (org) 
-				mi->info.from = g_strconcat(org->display_name,"<",org->email,">",NULL) ;
-			recp_list = e_gw_item_get_recipient_list (item);
-			if (recp_list) {
-				GSList *rl;
-				char *str = "";
-				int i = 0;
-				for (rl = recp_list; rl != NULL; rl = rl->next) {
-					EGwItemRecipient *recp = (EGwItemRecipient *) rl->data;
-					if (recp->type == E_GW_ITEM_RECIPIENT_TO) {
-						if (i)
-							str = g_strconcat (str, ", ", NULL);
-						str = g_strconcat (str, recp->display_name,"<",
-								   recp->email,">", NULL);
-					}
-					i++;
+		item_status = e_gw_item_get_item_status (item);
+		if (item_status & E_GW_ITEM_STAT_READ)
+			status_flags |= CAMEL_MESSAGE_SEEN;
+		/*if (item_status & E_GW_ITEM_STAT_DELETED)
+		  status_flags |= CAMEL_MESSAGE_DELETED;*/
+		if (item_status & E_GW_ITEM_STAT_REPLIED)
+			status_flags |= CAMEL_MESSAGE_ANSWERED;
+		mi->info.flags |= status_flags;
+
+		attach_list = e_gw_item_get_attach_id_list (item) ;
+		if (attach_list)  
+			mi->info.flags |= CAMEL_MESSAGE_ATTACHMENTS;
+
+		org = e_gw_item_get_organizer (item) ; 
+		if (org) 
+			mi->info.from = g_strconcat(org->display_name,"<",org->email,">",NULL) ;
+		recp_list = e_gw_item_get_recipient_list (item);
+		if (recp_list) {
+			GSList *rl;
+			GString *str = g_string_new (NULL);
+			int i = 0;
+			for (rl = recp_list; rl != NULL; rl = rl->next) {
+				EGwItemRecipient *recp = (EGwItemRecipient *) rl->data;
+				if (recp->type == E_GW_ITEM_RECIPIENT_TO) {
+					if (i)
+						str = g_string_append (str, ", ");
+					str = g_string_append (str, recp->display_name);
+					str = g_string_append (str, "<");
+					str = g_string_append (str, recp->email);
+					str = g_string_append (str, ">");
 				}
-				mi->info.to = g_strdup(str);
-				g_free (str);
+				i++;
 			}
-			
-			temp_date = e_gw_item_get_creation_date(item) ;
-			if (temp_date) {
-				time_t time = e_gw_connection_get_date_from_string (temp_date) ;
-				time_t actual_time = camel_header_decode_date (ctime(&time), NULL) ;
-				mi->info.date_sent = mi->info.date_received = actual_time ;
-			}
-			mi->info.uid = g_strdup (e_gw_item_get_id (item)) ;
-			mi->info.subject = g_strdup (e_gw_item_get_subject(item)) ;
+			mi->info.to = camel_pstring_strdup (str->str);
+			g_string_free (str, TRUE);
+		}
 
+		temp_date = e_gw_item_get_creation_date(item) ;
+		if (temp_date) {
+			time_t time = e_gw_connection_get_date_from_string (temp_date) ;
+			time_t actual_time = camel_header_decode_date (ctime(&time), NULL) ;
+			mi->info.date_sent = mi->info.date_received = actual_time ;
+		}
 
+		uid = e_gw_item_get_id (item);
+		if (uid) {
+			int len = strlen(uid);
+			mi->info.uid = g_malloc0 (len+1);
+			mi->info.uid = (char *)memcpy(mi->info.uid, uid, len);
+			uid = NULL;
+		}
+		
+		mi->info.subject = camel_pstring_strdup(e_gw_item_get_subject(item));
+		
+		if (exists) 
+			camel_folder_change_info_change_uid (changes, e_gw_item_get_id (item)) ;
+		else {
 			camel_folder_summary_add (folder->summary,(CamelMessageInfo *)mi) ;
 			camel_folder_change_info_add_uid (changes, mi->info.uid) ;
-			g_ptr_array_add (msg, mi) ;
-			g_free(date) ;
 		}
+
+		g_ptr_array_add (msg, mi) ;
+		g_free(date) ;
+		exists = FALSE ;
 		
 	}
 	camel_object_trigger_event (folder, "folder_changed", changes) ;