GNOME.org
 

[PATCH] : cleanups, Episode 6

	Hi all,

Finally the series goes on ;-). In fact with this patch, it's the whole
libbalsa dir that I have read. So now I'll make a pause to give time for
review.
So long 'till next season ;-)
Just one question : in libbalsa_mktemp (in libbalsa/misc.c), I use libmutt
global var without using libmutt lock, is that really safe?


in libbalsa/send.c
-in libbalsa_message_queue :
	correct indentation
-in libbalsa_process_queue :
	make use of LIBBALSA_XXX conversion
	correct useless cast to GList *
	correct indentation
	make use of g_list_next
-in monitor_cb : correct indentation
-in message2HEADER : small corrections
-in 
-in libbalsa_create_msg : seems to me that mutt locking has been forgotten
there ! I have tried to do it correctly, but PLEASE REVIEW ;-)

in libbalsa/source-viewer.c
-in libbalsa_show_file : allocate dynmic buffer instead of big array in
stack (I also had the error handler in case of lack of memory)

That's all folks
Bye
Manu
diff -u balsa-1.2.0/libbalsa/mailbox_mh.c test/balsa-1.2.0/libbalsa/mailbox_mh.c
--- balsa-1.2.0/libbalsa/mailbox_mh.c	Wed Oct 17 08:33:23 2001
+++ test/balsa-1.2.0/libbalsa/mailbox_mh.c	Tue Oct 16 18:52:35 2001
@@ -131,7 +131,7 @@
 					 _("Could not create MH structure at %s (%s)"), path, strerror(errno) );
 		    rmdir (path);
 		    return (-1);
-		}
+		}    
 	} else 
 	    return(-1);
     }
diff -u balsa-1.2.0/libbalsa/send.c test/balsa-1.2.0/libbalsa/send.c
--- balsa-1.2.0/libbalsa/send.c	Mon Oct 15 18:40:23 2001
+++ test/balsa-1.2.0/libbalsa/send.c	Wed Oct 17 13:29:03 2001
@@ -287,7 +287,6 @@
 
 
     g_return_if_fail(message);
-
     mqi = msg_queue_item_new(message);
     set_option(OPTWRITEBCC);
     if (libbalsa_create_msg(message, mqi->message,
@@ -300,34 +299,34 @@
 		|| LIBBALSA_IS_MAILBOX_IMAP(fccbox))) {
 	    libbalsa_lock_mutt();
 	    if (LIBBALSA_IS_MAILBOX_LOCAL(fccbox)) {
-	    mutt_write_fcc(libbalsa_mailbox_local_get_path(fccbox),
-			   mqi->message, NULL, 0, NULL);
+		mutt_write_fcc(libbalsa_mailbox_local_get_path(fccbox),
+			       mqi->message, NULL, 0, NULL);
 	    } else if (LIBBALSA_IS_MAILBOX_IMAP(fccbox)) {
 		server = LIBBALSA_MAILBOX_REMOTE(fccbox)->server;
 		if(!CLIENT_CONTEXT_OPEN(fccbox)) /* Has not been opened */
 		{
-			/* We cannot use LIBBALSA_REMOTE_MAILBOX_SERVER() here because */
-			/* it will lock up when NO IMAP mailbox has been accessed since */
-			/* balsa was started. This should be safe because we have already */
-			/* established that fccbox is in fact an IMAP mailbox */
-			if(server == (LibBalsaServer *)NULL) {
-				libbalsa_unlock_mutt();
-				libbalsa_information(LIBBALSA_INFORMATION_ERROR, "Unable to open sentbox - could not get server information");
-				return;
-			}
-			if (!(server->passwd && *server->passwd) &&
+		    /* We cannot use LIBBALSA_REMOTE_MAILBOX_SERVER() here because */
+		    /* it will lock up when NO IMAP mailbox has been accessed since */
+		    /* balsa was started. This should be safe because we have already */
+		    /* established that fccbox is in fact an IMAP mailbox */
+		    if(server == NULL) {
+			libbalsa_unlock_mutt();
+			libbalsa_information(LIBBALSA_INFORMATION_ERROR, "Unable to open sentbox - could not get server information");
+			return;
+		    }
+		    if (!(server->passwd && *server->passwd) &&
 			!(server->passwd = libbalsa_server_get_password(server, fccbox))) {
-				libbalsa_unlock_mutt();
-				libbalsa_information(LIBBALSA_INFORMATION_ERROR, "Unable to open sentbox - could not get passwords for server");
-				return;
-			}
-			reset_mutt_passwords(server);
+			libbalsa_unlock_mutt();
+			libbalsa_information(LIBBALSA_INFORMATION_ERROR, "Unable to open sentbox - could not get passwords for server");
+			return;
+		    }
+		    reset_mutt_passwords(server);
 		}
 
 		/* Passwords are guaranteed to be set now */
-
+		
 		mutt_write_fcc(LIBBALSA_MAILBOX(fccbox)->url,
-			   mqi->message, NULL, 0, NULL);
+			       mqi->message, NULL, 0, NULL);
 	    }
 	    libbalsa_unlock_mutt();
 	    libbalsa_mailbox_check(fccbox);
@@ -498,7 +497,7 @@
 	       Bcc: header.  The BCC copy of the message recipient list
 	       is taken from the Bcc recipients.  recipient list and the
 	       Bcc: header is preserved in the message. */
-	    bcc_recip = g_list_first((GList *) queu->bcc_list);
+	    bcc_recip = g_list_first(queu->bcc_list);
 	    if (!bcc_recip)
 		bcc_message = NULL;
 	    else
@@ -570,9 +569,9 @@
 	       copy of the message gets the To and Cc recipient list.
 	       The bcc copy gets the Bcc recipients.  */
 
-	    recip = g_list_first((GList *) queu->to_list);
+	    recip = g_list_first(queu->to_list);
 	    while (recip) {
-        	addy = recip->data;
+        	addy = LIBBALSA_ADDRESS(recip->data);
 		phrase = libbalsa_address_get_phrase(addy);
 		mailbox = libbalsa_address_get_mailbox(addy, 0);
 		recipient = smtp_add_recipient (message, mailbox);
@@ -584,12 +583,12 @@
 		if (bcc_message)
 		    smtp_set_header (bcc_message, "To", phrase, mailbox);
 #endif
-	    	recip = recip->next;
+	    	recip = g_list_next(recip);
 	    }
 
-	    recip = g_list_first((GList *) queu->cc_list);
+	    recip = g_list_first(queu->cc_list);
 	    while (recip) {
-        	addy = recip->data;
+        	addy = LIBBALSA_ADDRESS(recip->data);
 		phrase = libbalsa_address_get_phrase(addy);
 		mailbox = libbalsa_address_get_mailbox(addy, 0);
 		recipient = smtp_add_recipient (message, mailbox);
@@ -599,11 +598,11 @@
 		if (bcc_message)
 		    smtp_set_header (bcc_message, "Cc", phrase, mailbox);
 #endif
-	    	recip = recip->next;
+	    	recip = g_list_next(recip);
 	    }
 
 	    while (bcc_recip) {
-        	addy = bcc_recip->data;
+        	addy = LIBBALSA_ADDRESS(bcc_recip->data);
 		phrase = libbalsa_address_get_phrase(addy);
 		mailbox = libbalsa_address_get_mailbox(addy, 0);
 		recipient = smtp_add_recipient (bcc_message, mailbox);
@@ -611,7 +610,7 @@
 #ifdef LIBESMTP_ADDS_HEADERS
 		smtp_set_header (message, "Bcc", phrase, mailbox);
 #endif
-	    	bcc_recip = bcc_recip->next;
+	    	bcc_recip = g_list_next(bcc_recip);
 	    }
 
 	    /* Estimate the size of the message.  This need not be exact
@@ -643,7 +642,7 @@
 	    new_message->sent = 0;
 	    new_message->acc = 0;
 	}
-	lista = lista->next;
+	lista = g_list_next(lista);
     }
 
    /* At this point the session is ready to be sent.  As I've written the
@@ -711,7 +710,7 @@
 {
     SendThreadMessage *threadmsg;
     MessageQueueItem *mqi;
-    char buf[1024];
+    gchar * buf=NULL;
     va_list ap;
     const char *mailbox;
     smtp_message_t message;
@@ -731,22 +730,22 @@
         mailbox = va_arg (ap, const char *);
         message = va_arg (ap, smtp_message_t);
 	status = smtp_reverse_path_status (message);
-	snprintf (buf, sizeof buf, _("From: %d <%s>"), status->code, mailbox);
+	buf=g_new(gchar,1024);
+	snprintf(buf,sizeof buf,_("From: %d <%s>"), status->code, mailbox);
 	MSGSENDTHREAD(threadmsg, MSGSENDTHREADPROGRESS, buf, NULL, NULL, 0);
 
-	snprintf (buf, sizeof buf, _("From %s: %d %s"),
-	          mailbox, status->code, status->text);
+	snprintf(buf,sizeof buf,_("From %s: %d %s"),mailbox, status->code, status->text);
 	libbalsa_information(LIBBALSA_INFORMATION_MESSAGE, buf);
         break;
     case SMTP_EV_RCPTSTATUS:
         mailbox = va_arg (ap, const char *);
         recipient = va_arg (ap, smtp_recipient_t);
 	status = smtp_recipient_status (recipient);
-	snprintf (buf, sizeof buf, _("To: %d <%s>"), status->code, mailbox);
+	buf=g_new(gchar,1024);
+	snprintf(buf,sizeof buf,_("To: %d <%s>"), status->code, mailbox);
 	MSGSENDTHREAD(threadmsg, MSGSENDTHREADPROGRESS, buf, NULL, NULL, 0);
 
-	snprintf (buf, sizeof buf, _("To %s: %d %s"),
-	          mailbox, status->code, status->text);
+	snprintf (buf,sizeof buf,_("To %s: %d %s"),mailbox, status->code, status->text);
 	libbalsa_information(LIBBALSA_INFORMATION_MESSAGE, buf);
         break;
     case SMTP_EV_MESSAGEDATA:
@@ -769,8 +768,8 @@
     case SMTP_EV_MESSAGESENT:
         message = va_arg (ap, smtp_message_t);
         status = smtp_message_transfer_status (message);
-	snprintf (buf, sizeof buf, _("%d %s"),
-		  status->code, status->text);
+	buf=g_new(gchar,1024);
+	snprintf(buf,sizeof buf,_("%d %s"),status->code, status->text);
 	MSGSENDTHREAD(threadmsg, MSGSENDTHREADPROGRESS, buf, NULL, NULL, 0);
 	libbalsa_information(LIBBALSA_INFORMATION_MESSAGE, buf);
         /* Reset 'mqi->sent' for the next message (i.e. bcc copy) */
@@ -786,6 +785,7 @@
 		      NULL, NULL, 0);
         break;
     }
+    g_free(buf);
     va_end (ap);
 }
 #endif
@@ -835,7 +835,7 @@
 		mqi = new_message;
 		total_messages_left++;
 	    }
-	    lista = lista->next;
+	    lista = g_list_next(lista);
 	}
 #ifdef BALSA_USE_THREADS
     }
@@ -898,24 +898,24 @@
 static void
 monitor_cb (const char *buf, int buflen, int writing, void *arg)
 {
-  FILE *fp = arg;
-
-  if (writing == SMTP_CB_HEADERS)
-    {
-      fputs ("H: ", fp);
-      fwrite (buf, 1, buflen, fp);
-      return;
-    }
-
- fputs (writing ? "C: " : "S: ", fp);
- fwrite (buf, 1, buflen, fp);
- if (buf[buflen - 1] != '\n')
-   putc ('\n', fp);
+    FILE *fp = arg;
+    
+    if (writing == SMTP_CB_HEADERS)
+	{
+	    fputs ("H: ", fp);
+	    fwrite (buf, 1, buflen, fp);
+	    return;
+	}
+    
+    fputs (writing ? "C: " : "S: ", fp);
+    fwrite (buf, 1, buflen, fp);
+    if (buf[buflen - 1] != '\n')
+	putc ('\n', fp);
 }
 #endif
 
 /* balsa_send_message_real:
-   does the acutal message sending. 
+   does the actual message sending. 
    This function may be called as a thread and should therefore do
    proper gdk_threads_{enter/leave} stuff around GTK,libbalsa or
    libmutt calls. Also, structure info should be freed before exiting.
@@ -1057,7 +1057,8 @@
 
 
 static void
-message2HEADER(LibBalsaMessage * message, HEADER * hdr) {
+message2HEADER(LibBalsaMessage * message, HEADER * hdr)
+{
     gchar *tmp;
 
     libbalsa_lock_mutt();
@@ -1073,7 +1074,7 @@
 	LIST *delptr = 0;
 
 	while (sptr) {
-	    dptr->data = g_strdup(sptr->data);
+	    dptr->data = g_strdup((gchar*)sptr->data);
 	    sptr = sptr->next;
 	    delptr = dptr;
 	    dptr->next = mutt_new_list();
@@ -1142,7 +1143,8 @@
 libbalsa_message_postpone(LibBalsaMessage * message,
 			  LibBalsaMailbox * draftbox,
 			  LibBalsaMessage * reply_message,
-			  gchar * fcc, gint encoding) {
+			  gchar * fcc, gint encoding)
+{
     HEADER *msg;
     BODY *last, *newbdy;
     gchar *tmp;
@@ -1179,7 +1181,7 @@
 		
 		type = g_strdup (body->mime_type);
 		if ((subtype = strchr (type, '/'))) {
-		    *subtype++ = 0;
+		    *subtype++ = '\0';
 		    libbalsa_lock_mutt();
 		    newbdy->type = mutt_check_mime_type (type);
 		    newbdy->subtype = g_strdup(subtype);
@@ -1212,7 +1214,9 @@
     }
 
     mutt_prepare_envelope(msg->env);
+    libbalsa_unlock_mutt();
     encode_descriptions(msg->content);
+    libbalsa_lock_mutt();
 
     if ((reply_message != NULL) && (reply_message->mailbox != NULL))
 	/* Just saves the message ID, mailbox type and mailbox name. We could
@@ -1237,7 +1241,7 @@
 	    /* it will lock up when NO IMAP mailbox has been accessed since */
 	    /* balsa was started. This should be safe because we have already */
 	    /* established that draftbox is in fact an IMAP mailbox */
-	    if(server == (LibBalsaServer *)NULL) {
+	    if(server == NULL) {
 		libbalsa_information(LIBBALSA_INFORMATION_ERROR, "Unable to open draftbox - could not get server information");
 		g_free(tmp);
 		mutt_free_header(&msg);
@@ -1280,7 +1284,8 @@
  * get the type and subtype for later use.  Returns the type, and the
  * subtype in a gchar array, free using g_strfreev()
  * */
-gchar **libbalsa_lookup_mime_type(const gchar * path) {
+gchar **libbalsa_lookup_mime_type(const gchar * path)
+{
     gchar **tmp;
     const gchar *mime_type;
 
@@ -1295,9 +1300,11 @@
    copies message to msg.
    PS: seems to be broken when queu == 1 - further execution of
    mutt_free_header(mgs) leads to crash.
-*/ static gboolean
+*/
+static gboolean
 libbalsa_create_msg(LibBalsaMessage * message, HEADER * msg, gchar **tmpfile,
-		    gint encoding, int queu) {
+		    gint encoding, int queu)
+{
     BODY *last, *newbdy;
     FILE *tempfp;
     HEADER *msg_tmp;
@@ -1310,31 +1317,31 @@
 
 
     message2HEADER(message, msg);
-
     /* If the message has references set, add them to he envelope */
+    libbalsa_lock_mutt();
     if (message->references != NULL) {
 	list = message->references;
 	msg->env->references = mutt_new_list();
 	references = msg->env->references;
-	references->data = g_strdup(list->data);
-	list = list->next;
+	references->data = g_strdup((gchar*)list->data);
+	list = g_list_next(list);
 
-	while (list != NULL) {
+	while (list) {
 	    references->next = mutt_new_list();
 	    references = references->next;
-	    references->data = g_strdup(list->data);
+	    references->data = g_strdup((gchar*)list->data);
 	    references->next = NULL;
-	    list = list->next;
+	    list = g_list_next(list);
 	}
 	/* There's no specific header for In-Reply-To, just
-	 * add it to the user headers */ in_reply_to = mutt_new_list();
+	 * add it to the user headers */ 
+	in_reply_to = mutt_new_list();
 	in_reply_to->next = msg->env->userhdrs;
 	in_reply_to->data =
 	    g_strconcat("In-Reply-To: ", message->in_reply_to, NULL);
 	msg->env->userhdrs = in_reply_to;
     }
-
-
+    libbalsa_unlock_mutt();
     if (message->mailbox)
 	libbalsa_message_body_ref(message);
 
@@ -1344,6 +1351,7 @@
     while (last && last->next)
 	last = last->next;
 
+    libbalsa_lock_mutt();
     while (body) {
 	FILE *tempfp = NULL;
 	newbdy = NULL;
@@ -1358,12 +1366,14 @@
 
 		/* Do this here because we don't want
 		 * to use libmutt's mime types */
+		libbalsa_unlock_mutt();
 		mime_type =
 		    libbalsa_lookup_mime_type((const gchar *) body->
 					      filename);
 		/* use BASE64 encoding for non-text mime types */
 		if(strcasecmp(mime_type[0],"text") != 0)
 		    newbdy->encoding = ENCBASE64;
+		libbalsa_lock_mutt();
 		newbdy->type = mutt_check_mime_type(mime_type[0]);
 		newbdy->type = mutt_check_mime_type(mime_type[0]);
 		g_free(newbdy->subtype);
@@ -1371,7 +1381,9 @@
 		g_strfreev(mime_type);
 	    }
 	} else if (body->buffer) {
+	    libbalsa_unlock_mutt();
 	    newbdy = add_mutt_body_plain(body->charset, encoding);
+	    libbalsa_lock_mutt();
 #ifdef BALSA_MDN_REPLY
 	    if (body->mime_type) {
 		/* change the type and subtype within the mutt body */
@@ -1379,11 +1391,9 @@
 		
 		type = g_strdup (body->mime_type);
 		if ((subtype = strchr (type, '/'))) {
-		    *subtype++ = 0;
-		    libbalsa_lock_mutt();
+		    *subtype++ = '\0';
 		    newbdy->type = mutt_check_mime_type (type);
 		    newbdy->subtype = g_strdup(subtype);
-		    libbalsa_unlock_mutt();
 		}
 		g_free (type);
 	    }
@@ -1408,14 +1418,14 @@
 
 	body = body->next;
     }
-
+    
     if (msg->content) {
 	if (msg->content->next)
 	    msg->content = mutt_make_multipart(msg->content);
     }
 
     mutt_prepare_envelope(msg->env);
-
+    libbalsa_unlock_mutt();
     encode_descriptions(msg->content);
 
 /* We create the message in MIME format here, we use the same format 
@@ -1425,10 +1435,12 @@
 	if ((tempfp = safe_fopen(*tmpfile, "w")) == NULL)
 	    return (-1);
 
+	libbalsa_lock_mutt();
 	mutt_write_rfc822_header(tempfp, msg->env, msg->content, -1);
 	fputc('\n', tempfp);	/* tie off the header. */
 
 	if ((mutt_write_mime_body(msg->content, tempfp) == -1)) {
+	    libbalsa_unlock_mutt();
 	    fclose(tempfp);
 	    unlink(*tmpfile);
 	    g_free(*tmpfile);
@@ -1439,6 +1451,7 @@
 
 	if (fclose(tempfp) != 0) {
 	    mutt_perror(*tmpfile);
+	    libbalsa_unlock_mutt();
 	    unlink(*tmpfile);
 	    g_free(*tmpfile);
 	    *tmpfile=NULL;
@@ -1447,7 +1460,7 @@
 
     } else {
 	/* the message is in the queue */
-
+	libbalsa_lock_mutt();
 	msg_tmp =
 	    CLIENT_CONTEXT(message->mailbox)->hdrs[message->msgno];
 	mutt_parse_mime_message(CLIENT_CONTEXT(message->mailbox),
@@ -1456,15 +1469,18 @@
 	    mx_open_message(CLIENT_CONTEXT(message->mailbox),
 			    msg_tmp->msgno);
 
+	libbalsa_unlock_mutt();
 	*tmpfile=libbalsa_mktemp();
 	if ((tempfp = safe_fopen(*tmpfile, "w")) == NULL)
 	    return FALSE;
 
+	libbalsa_lock_mutt();
 	_mutt_copy_message(tempfp, mensaje->fp, msg_tmp,
 			   msg_tmp->content, 0, CH_NOSTATUS);
 
 	if (fclose(tempfp) != 0) {
 	    mutt_perror(*tmpfile);
+	    libbalsa_unlock_mutt();
 	    unlink(*tmpfile);
 	    g_free(*tmpfile);
 	    *tmpfile=NULL;
@@ -1475,6 +1491,7 @@
 
     }
 
+    libbalsa_unlock_mutt();
     if (message->mailbox)
 	libbalsa_message_body_unref(message);
 
diff -u balsa-1.2.0/libbalsa/source-viewer.c test/balsa-1.2.0/libbalsa/source-viewer.c
--- balsa-1.2.0/libbalsa/source-viewer.c	Tue Jul 31 10:53:05 2001
+++ test/balsa-1.2.0/libbalsa/source-viewer.c	Wed Oct 17 12:20:46 2001
@@ -54,7 +54,8 @@
 libbalsa_show_file(FILE* f, long length)
 {
     GtkWidget* window, *interior;
-    GtkEditable* text;  char buf[1024];
+    GtkEditable* text;
+    gchar * buf;
     int linelen, pos = 0;
 
     window = gnome_app_new("balsa", _("Message Source"));
@@ -73,16 +74,23 @@
 
     if(length<0) length = 5*1024*1024; /* random limit for the file size
 					* not likely to be used */
-    while(length>0 && fgets(buf, sizeof(buf), f)) {
-	linelen = strlen(buf);
-	gtk_editable_insert_text(text, buf,
-				 length>linelen ? linelen : length, 
-				 &pos);
-	length -= linelen;
+    buf=g_new(gchar,1024);
+    if (buf) {
+	while(length>0 && fgets(buf, sizeof(buf), f)) {
+	    linelen = strlen(buf);
+	    gtk_editable_insert_text(text, buf,
+				     length>linelen ? linelen : length, 
+				     &pos);
+	    length -= linelen;
+	}
+	g_free(buf);
+	gtk_window_set_default_size(GTK_WINDOW(window), 500, 400);
+	gtk_widget_show_all(window);
+    }
+    else {
+	libbalsa_information(LIBBALSA_INFORMATION_ERROR,_("Unable to open source viewer : not enough memory"));
+	gtk_widget_destroy(GTK_WIDGET(window));
     }
-
-    gtk_window_set_default_size(GTK_WINDOW(window), 500, 400);
-    gtk_widget_show_all(window);
 }
 
 /* libbalsa_show_message_source:
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]