Re: [PATCH]: POP3 hangs solved (I hope) : cleaner version

[Emmanuel <e allaud wanadoo fr>]

On 2001.10.05 20:31 Emmanuel wrote:
> 	Hi all,
> finally here is the patch corresponding to my observations, it's simple
> and
> (AFAI have tested it) it fixes hangs when doing pop retrieving.
> The cause of the hang was because we always closed a file descriptor that
> got to be set by the pop3 connection, even in cases where the connection
> had not been established. So on connection errors (host not found or
> anything that will prevent the file descriptor to be properly set) we
> blindly used write and close on a bad descriptor.
> If it doesn't fix all hangs on pop3 retrieving, at least it is more
> robust
> now.
> 
> All changes are in libbalsa/pop3.c :
> - one error message typo
> - I have moved a test on a pointer before using strchr on it (strchr on
> NULL leads to a segfault)
> - I have made the test to see if we send the "quit" command to the pop
> server to be more selective (doesn't send the command when the connection
> have not been set due to connection errors)
> - I have added a test to prevent us to close the file descriptor of the
> pop
> connection if it has not been set due to connection errors).
> 
> Patch attached.

Here is just a new version that seems much cleaner in regards of error
handling.

Bye
Manu
--- balsa-1.2.0/libbalsa/pop3.c	Wed Aug 29 13:56:36 2001
+++ balsa-1.2.0-curr/libbalsa/pop3.c	Fri Oct  5 21:04:04 2001
@@ -53,7 +53,7 @@
     static gchar* errmsgs[] = {
 	"",
 	N_("connection error"),
-	N_("POP command exection failed"),
+	N_("POP command execution failed"),
 	N_("Could not write the message"),
 	N_("Could not run the delivery program (procmail)"),
 	N_("Could not open mailbox for spooling"),
@@ -142,10 +142,12 @@
     char *finish;
     size_t len;
     
+    if (!buff) return FALSE;
+
     start = strchr(buff, '<');
     finish = strchr(buff, '>');
     
-    if( buff && stamp && start && finish ) {
+    if( stamp && start && finish ) {
         len = strlen(start) - strlen(finish) + 1;
         strncpy( stamp, start, len );
         return TRUE;
@@ -591,20 +593,26 @@
 
     DM("POP3 - begin connection");
     status = pop_connect(&s, pop_host, pop_port);
+    if (status!=POP_OK)
+	goto connect_failed;
     DM("POP3 Connected; hello");
 
-    if(status == POP_OK)
-	status = pop_auth(s, pop_user, pop_pass, use_apop);
-
+    status = pop_auth(s, pop_user, pop_pass, use_apop);
     DM("POP3 authentication %s successful", (status ==POP_OK ? "" : "NOT"));
-    if(status == POP_OK)
-	status = pop_get_stats(s, &first_msg, &msgs, &bytes,  
-			       delete_on_server ? NULL : uid, last_uid);
+    if (status!=POP_OK)
+	goto clean_up_connect;
+
+    status = pop_get_stats(s, &first_msg, &msgs, &bytes,  
+			   delete_on_server ? NULL : uid, last_uid);
+    if (status!=POP_OK)
+	goto clean_up_connect;
+
     DM("POP3 status after get stats: %d", status);
-    if(status == POP_OK)
-	status = proccb(s, first_msg, msgs, bytes, delete_on_server, 
-			data, prog_cb);
-    
+    status = proccb(s, first_msg, msgs, bytes, delete_on_server, 
+		    data, prog_cb);
+
+ clean_up_connect:
+    /* We send the quit command, but not in case a connection error occured */
     if(status != POP_CONN_ERR) {
 	DM("POP3 C: quit");
 	/* exit gracefully */
@@ -613,8 +621,10 @@
 	if(status == POP_OK)
 	    strcpy(last_uid, uid);/* FIXME: overflow error on hideous reply? */
     }
+    /* We close the fd anyway, it is valid here*/
     close (s);
 
+ connect_failed:
     return status;
 }