[PATCH] : Still clean-ups



	Hi all,
clean-ups/improvments/bug fixes, Episode 4 ;-)
Please review, and particularly everything tagged with PLEASE REVIEW :)

in libbalsa/identity.c : small type (it's the only thing I found ;-)

in libbalsa/mailbox.c
-in libbalsa_mailbox_close_append : 
	missing libmutt lock when calling mx_close_mailbox; PLEASE REVIEW,
I'm not really aware of proper locking

-in libbalsa_mailbox_load_messages : 
	use g_list_prepend/g_list_reverse instead of g_list_append, and
small indent fix

-in libbalsa_mailbox_sync_backend_real :
	make use of g_list_next instead of temp var;
	use g_list_prepend instead of append : PLEASE REVIEW, because I did
not call g_list_reverse as I think order is not significant

in translate_message :
	change all g_list_append to g_list_prepend/g_list_reverse (PLEASE
REVIEW all these changes because I'm not sure we really need to reverse all
lists, indeed for certain lists, order doesn't seem to be significant)
	clean up the strings manipulation for "in_reply_to" field


PLEASE REVIEW this piece of code I don't understand :

    for (tmp = cenv->userhdrs; tmp;) {
	if (g_strncasecmp("X-Mutt-Fcc:", (gchar *)tmp->data, 11) == 0) {
	    p = (gchar *)tmp->data + 11;
	    SKIPWS(p);

	    if (p)
		message->fcc_mailbox = g_strdup(p);
	    else
		message->fcc_mailbox = NULL;
->	} else if (g_strncasecmp("X-Mutt-Fcc:", (gchar *)tmp->data, 18)
== 0) {
->	    /* Is X-Mutt-Fcc correct? */
->	    p = (gchar *) tmp->data + 18;
	    SKIPWS(p);

-->	    message->in_reply_to = g_strdup(p);
	} else if (g_strncasecmp ("In-Reply-To:", (gchar *)tmp->data, 12)
== 0){
	    p = (gchar *)tmp->data + 12;
	    while(*p!='\0'&& *p!='<')p++;
	    if(*p!='\0'){
		guint len=0;
		gchar * str=p;

		while(*p!='\0' && *p!='>') {
		    p++;
		    len++;
		}
		message->in_reply_to = g_strndup (str,len);
	    }
	}
	tmp = tmp->next;
    }

I put arrows where is the bug : I think that "X-Mutt-Fcc:" is not the right
string, but dunno the good one. Moreover the next arrow indicate another
bug : we don't want to set this field here because we take care of it in
the next branch of the if test.
In short it would be better if someone knowing what this code was intended
to do to fix that mess.

Thanks if you managed to read till here ;-)
More to come

Bye
Manu
diff -u balsa-1.2.0/libbalsa/identity.c test/balsa-1.2.0/libbalsa/identity.c
--- balsa-1.2.0/libbalsa/identity.c	Tue Jul 24 12:55:37 2001
+++ test/balsa-1.2.0/libbalsa/identity.c	Tue Oct 16 16:33:42 2001
@@ -461,7 +461,7 @@
 /* identity_list_update:
  * Update the list of identities in the config frame, displaying the
  * available identities in the application, and which is default.
- * We need to tempararily switch to selection mode single to avoid row 
+ * We need to temporarily switch to selection mode single to avoid row 
  * autoselection when there is no data attached to it yet (between
  * gtk_clist_append() and gtk_clist_set_row_data()).
  */
diff -u balsa-1.2.0/libbalsa/mailbox.c test/balsa-1.2.0/libbalsa/mailbox.c
--- balsa-1.2.0/libbalsa/mailbox.c	Tue Sep 18 15:28:58 2001
+++ test/balsa-1.2.0/libbalsa/mailbox.c	Tue Oct 16 17:38:35 2001
@@ -385,7 +385,9 @@
     int ret;
     g_return_val_if_fail(handle != NULL, -1);
 
+    libbalsa_lock_mutt();
     ret = mx_close_mailbox(handle->context, NULL);
+    libbalsa_unlock_mutt();
     g_free(handle);
     return ret;
 }
@@ -662,14 +664,15 @@
 
 	mailbox->new_messages--;
 
-	messages=g_list_append(messages, message);
+	messages=g_list_prepend(messages, message);
     }
     UNLOCK_MAILBOX(mailbox);
 
-    if(messages!=NULL){
-      gtk_signal_emit(GTK_OBJECT(mailbox),
+    if (messages!=NULL) {
+	messages=g_list_reverse(messages);
+	gtk_signal_emit(GTK_OBJECT(mailbox),
 			libbalsa_mailbox_signals[MESSAGES_NEW], messages);
-      g_list_free(messages);
+	g_list_free(messages);
     }
 
     libbalsa_mailbox_set_unread_messages_flag(mailbox,
@@ -690,13 +693,13 @@
 
     list = g_list_first(mailbox->message_list);
 
-    if(list){
-      gtk_signal_emit(GTK_OBJECT(mailbox),
-		      libbalsa_mailbox_signals[MESSAGES_DELETE], list);
+    if (list) {
+	gtk_signal_emit(GTK_OBJECT(mailbox),
+			libbalsa_mailbox_signals[MESSAGES_DELETE], list);
     }
 
     while (list) {
-	message = list->data;
+	message = LIBBALSA_MESSAGE(list->data);
 	list = list->next;
 
 	message->mailbox = NULL;
@@ -758,13 +761,12 @@
     message_list = mailbox->message_list;
     while (message_list) {
 	current_message = LIBBALSA_MESSAGE(message_list->data);
-	tmp_message_list = message_list->next;
 	if (current_message->flags & LIBBALSA_MESSAGE_FLAG_DELETED) {
-	    p=g_list_append(p, current_message);
+	    p=g_list_prepend(p, current_message);
 	}
-	message_list = tmp_message_list;
+	message_list = g_list_next(message_list);
     }
-    if(p){
+    if (p) {
 	gtk_signal_emit(GTK_OBJECT(mailbox),
 			libbalsa_mailbox_signals[MESSAGES_DELETE],
 			p);
@@ -774,7 +776,7 @@
     message_list = mailbox->message_list;
     while (message_list) {
 	current_message = LIBBALSA_MESSAGE(message_list->data);
-	tmp_message_list = message_list->next;
+	tmp_message_list = g_list_next(message_list);
 	if (current_message->flags & LIBBALSA_MESSAGE_FLAG_DELETED) {
 	    current_message->mailbox = NULL;
 	    gtk_object_unref(GTK_OBJECT(current_message));
@@ -820,7 +822,7 @@
 
 
 /* internal c-client translation:
- * mutt lists can cantain null adresses for address strings like
+ * mutt lists can contain null adresses for address strings like
  * "To: Dear Friends,". We do remove them.
  */
 static LibBalsaMessage *
@@ -849,43 +851,50 @@
 
     for (addy = cenv->to; addy; addy = addy->next) {
 	addr = libbalsa_address_new_from_libmutt(addy);
-	if(addr) message->to_list = g_list_append(message->to_list, addr);
+	if(addr) message->to_list = g_list_prepend(message->to_list, addr);
     }
+    message->to_list=g_list_reverse(message->to_list);
 
     for (addy = cenv->cc; addy; addy = addy->next) {
 	addr = libbalsa_address_new_from_libmutt(addy);
-	if(addr) message->cc_list = g_list_append(message->cc_list, addr);
+	if(addr) message->cc_list = g_list_prepend(message->cc_list, addr);
     }
+    message->cc_list=g_list_reverse(message->cc_list);
 
     for (addy = cenv->bcc; addy; addy = addy->next) {
 	addr = libbalsa_address_new_from_libmutt(addy);
-	if(addr) message->bcc_list = g_list_append(message->bcc_list, addr);
+	if(addr) message->bcc_list = g_list_prepend(message->bcc_list, addr);
     }
+    message->bcc_list=g_list_reverse(message->bcc_list);
 
     /* Get fcc from message */
     for (tmp = cenv->userhdrs; tmp;) {
-	if (g_strncasecmp("X-Mutt-Fcc:", tmp->data, 11) == 0) {
-	    p = tmp->data + 11;
+	if (g_strncasecmp("X-Mutt-Fcc:", (gchar *)tmp->data, 11) == 0) {
+	    p = (gchar *)tmp->data + 11;
 	    SKIPWS(p);
 
 	    if (p)
 		message->fcc_mailbox = g_strdup(p);
 	    else
 		message->fcc_mailbox = NULL;
-	} else if (g_strncasecmp("X-Mutt-Fcc:", tmp->data, 18) == 0) {
+	} else if (g_strncasecmp("X-Mutt-Fcc:", (gchar *)tmp->data, 18) == 0) {
 	    /* Is X-Mutt-Fcc correct? */
-	    p = tmp->data + 18;
+	    p = (gchar *) tmp->data + 18;
 	    SKIPWS(p);
 
 	    message->in_reply_to = g_strdup(p);
-	} else if (g_strncasecmp ("In-Reply-To:", tmp->data, 12) == 0){
-	    p = tmp->data + 12;
+	} else if (g_strncasecmp ("In-Reply-To:", (gchar *)tmp->data, 12) == 0){
+	    p = (gchar *)tmp->data + 12;
 	    while(*p!='\0'&& *p!='<')p++;
 	    if(*p!='\0'){
-		message->in_reply_to = g_strdup (p);
-		p=message->in_reply_to;
-		while(*p!='\0' && *p!='>')p++;
-		if(*p=='>')*(p+1)='\0';
+		guint len=0;
+		gchar * str=p;
+
+		while(*p!='\0' && *p!='>') {
+		    p++;
+		    len++;
+		}
+		message->in_reply_to = g_strndup (str,len);
 	    }
 	}
 	tmp = tmp->next;
@@ -897,9 +906,13 @@
     message->message_id = g_strdup(cenv->message_id);
 
     for (tmp = cenv->references; tmp != NULL; tmp = tmp->next) {
-	message->references = g_list_append(message->references,
-					    g_strdup(tmp->data));
+	message->references = g_list_prepend(message->references,
+					    g_strdup((gchar *)tmp->data));
     }
+    /* FIXME : this reverse seems useless, I think list order is not significant for references
+     * see references_for_threading : there we used prepend without reversing the result
+     */
+    message->references = g_list_reverse(message->references);
 
     /* more! */
 
@@ -908,7 +921,7 @@
 	while(p!=NULL){
 	    message->references_for_threading = 
 		g_list_prepend(message->references_for_threading, 
-			       g_strdup(p->data));
+			       g_strdup((gchar *)p->data));
             p=p->next;
 	}
     }


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