Mercurial > noffle

changeset 180:09ca6eb5c7ff noffle

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
[svn] * TODO,src/client.c,src/client.h,src/fetch.c,src/fetch.h,src/noffle.c: Improve error checking during fetches. A fetch is now aborted immediately if the connection times out or if an unexpected response arrives. This should fix problems with articles appearing in the wrong group, and possibly other mysterious happenings.
author bears
date Wed, 09 May 2001 12:33:43 +0100
parents f973675760dc
children e196de757ecd
files ChangeLog TODO src/client.c src/client.h src/fetch.c src/fetch.h src/noffle.c
diffstat 7 files changed, 149 insertions(+), 88 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Wed May 09 12:15:31 2001 +0100
+++ b/ChangeLog	Wed May 09 12:33:43 2001 +0100
@@ -1,3 +1,11 @@
+Wed May  9 2001 Jim Hague <jim.hague@acm.org>
+
+ * TODO,src/client.c,src/client.h,src/fetch.c,src/fetch.h,src/noffle.c:
+   Improve error checking during fetches. A fetch is now aborted immediately
+   if the connection times out or if an unexpected response arrives.
+   This should fix problems with articles appearing in the wrong group,
+   and possibly other mysterious happenings.
+	
 Tue May  8 2001 Jim Hague <jim.hague@acm.org>
 
  * src/post.c: If post-locally is set, only do immediate local posting
--- a/TODO	Wed May 09 12:15:31 2001 +0100
+++ b/TODO	Wed May 09 12:33:43 2001 +0100
@@ -14,10 +14,6 @@
 Later
 -----
 
- * Make Noffle handle a connection break-down during a fetch more gracefully.
-   At present it still continues to fetch articles, generating a
-   retrieving failed article for each article left.
-
  * Improve performance of group database. Using GDBM is a bad choice,
    better use a btree from the Berkeley db library in libc.
    This will be a good time for a redesign of the group.h interface
--- a/src/client.c	Wed May 09 12:15:31 2001 +0100
+++ b/src/client.c	Wed May 09 12:33:43 2001 +0100
@@ -1,7 +1,7 @@
 /*
   client.c
 
-  $Id: client.c 269 2001-03-13 11:46:03Z enz $
+  $Id: client.c 279 2001-05-09 11:33:43Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -166,7 +166,7 @@
 
 static int getStat( void );
 
-static Bool
+static int
 performAuth( void )
 {
     int stat;
@@ -176,30 +176,27 @@
     if ( strcmp( user, "" ) == 0 )
     {
         Log_err( "No username for authentication set" );
-        return FALSE;
+        return STAT_AUTH_REQUIRED;
     }    
     putCmd( "AUTHINFO USER %s", user );
     stat = getStat();
     if ( stat == STAT_AUTH_ACCEPTED )
-        return TRUE;
+        return stat;
     else if ( stat != STAT_MORE_AUTH_REQUIRED )
     {
         Log_err( "Username rejected. Server stat: %s", client.lastStat );
-        return FALSE;
+        return stat;
     }    
     if ( strcmp( pass, "" ) == 0 )
     {
         Log_err( "No password for authentication set" );
-        return FALSE;
+        return STAT_AUTH_REQUIRED;
     }
     putCmd( "AUTHINFO PASS %s", pass );
     stat = getStat();
     if ( stat != STAT_AUTH_ACCEPTED )
-    {
         Log_err( "Password rejected. Server status: %s", client.lastStat );
-        return FALSE;
-    }    
-    return TRUE;    
+    return stat;    
 }
 
 static int
@@ -219,7 +216,8 @@
     {
         client.auth = TRUE;
         strcpy( lastCmd, client.lastCmd );
-        if ( performAuth() )
+	result = performAuth();
+        if ( result == STAT_AUTH_ACCEPTED )
         {
             putCmd( lastCmd );
             return getStat();
@@ -533,6 +531,13 @@
     if ( ! putCmd( cmd ) )
         return FALSE;
     stat = getStat();
+    if ( stat == STAT_PROGRAM_FAULT )
+	return FALSE;
+
+    /*
+     * Try LIST instead of LIST ACTIVE in case server doesn't
+     * support LIST ACTIVE.
+     */
     if ( pattern[ 0 ] != '\0' && stat != STAT_GRPS_FOLLOW )
     {
 	*noServerPattern = TRUE;
@@ -589,6 +594,7 @@
     int stat;
     DynStr *response;
     const char *lines;
+    Bool result;
 
     ASSERT( ! Lock_gotLock() );
     Utl_cpyStr( cmd, "LIST NEWSGROUPS" );
@@ -602,6 +608,10 @@
     if ( ! putCmd( cmd ) )
         return FALSE;
     stat = getStat();
+    if ( stat == STAT_PROGRAM_FAULT )
+	return FALSE;
+
+    /* Try without pattern in case server doesn't support patterns. */
     if ( pattern[ 0 ] != '\0' && stat != STAT_GRPS_FOLLOW )
     {
 	*noServerPattern = TRUE;
@@ -623,12 +633,14 @@
 	return FALSE;
     
     lines = DynStr_str( response );
+    result = TRUE;
     while ( ( lines = Utl_getLn( line, lines) ) != NULL )
     {
         if ( sscanf( line, "%s", name ) != 1 )
         {
             Log_err( "Unknown reply to LIST NEWSGROUPS: %s", line );
-            continue;
+	    result = FALSE;
+	    break;
         }
 	if ( *noServerPattern && ! isGetGroup( name ) )
 	    continue;
@@ -641,7 +653,7 @@
     }
     Lock_closeDatabases();
     del_DynStr( response );
-    return TRUE;
+    return result;
 }
 
 Bool
@@ -664,7 +676,7 @@
 	    break;
     }
 
-    if ( ! doneOne )
+    if ( res && ! doneOne )
 	res = doGetDsc( "", &noServerPattern );
 
     del_GrEn( ge );
@@ -909,6 +921,9 @@
 		 rmtFirst, rmtLast );
 
 	newsgroups = collectTxt();
+	if ( newsgroups == NULL )
+	    return FALSE;
+	
 	groupLines = DynStr_str( newsgroups );
     }
     else
@@ -1063,19 +1078,21 @@
     return ! err;
 }
 
-void
+Bool
 Client_retrieveArt( const char *msgId )
 {
+    Bool res = FALSE;
+    
     ASSERT( Lock_gotLock() );
     if ( ! Db_contains( msgId ) )
     {
         Log_err( "Article '%s' not prepared in database. Skipping.", msgId );
-        return;
+        return TRUE;
     }
     if ( ! ( Db_status( msgId ) & DB_NOT_DOWNLOADED ) )
     {
         Log_inf( "Article '%s' already retrieved. Skipping.", msgId );
-        return;
+        return TRUE;
     }
 
     Lock_closeDatabases();
@@ -1084,16 +1101,18 @@
     else if ( getStat() != STAT_ART_FOLLOWS )
         retrievingFailed( msgId, client.lastStat );
     else
-        retrieveAndStoreArt( msgId, 0, 0 );
+        res = retrieveAndStoreArt( msgId, 0, 0 );
     Lock_openDatabases();
+    return res;
 }
 
-void
+Bool
 Client_retrieveArtList( const char *list, int *artcnt, int artmax )
 {
     Str msgId;
     DynStr *s;
     const char *p;
+    Bool res;
     
     ASSERT( Lock_gotLock() );
     Log_inf( "Retrieving article list" );
@@ -1109,7 +1128,7 @@
         {
             retrievingFailed( msgId, "Connection broke down" );
             del_DynStr( s );
-            return;
+            return FALSE;
         }
         else
             DynStr_appLn( s, msgId );
@@ -1119,15 +1138,20 @@
     Log_dbg( "[S FLUSH]" );
     
     p = DynStr_str( s );
-    while ( ( p = Utl_getLn( msgId, p ) ) )
+    res = TRUE;
+    while ( res && ( p = Utl_getLn( msgId, p ) ) )
     {
         if ( getStat() != STAT_ART_FOLLOWS )
+	{
             retrievingFailed( msgId, client.lastStat );
-        else if ( ! retrieveAndStoreArt( msgId, ++(*artcnt), artmax ) )
-            break;
+	    res = FALSE;
+	}
+        else
+	    res = retrieveAndStoreArt( msgId, ++(*artcnt), artmax );
     }
     del_DynStr( s );
     Lock_openDatabases();
+    return res;
 }
 
 Bool
@@ -1166,24 +1190,33 @@
 }
 
 Bool
-Client_postArt( const char *msgId, const char *artTxt,
-                    Str errStr )
+Client_postArt( const char *msgId, const char *artTxt, Str errStr )
 {
+    int stat;
+    
+    errStr[0] = '\0';
+    
     if ( ! putCmd( "POST" ) )
         return FALSE;
-    if ( getStat() != STAT_SEND_ART )
+    stat = getStat();
+    if ( stat == STAT_PROGRAM_FAULT )
+	return FALSE;
+    else if ( stat != STAT_SEND_ART )
     {
         Log_err( "Posting of %s not allowed: %s", msgId, client.lastStat );
         strcpy( errStr, client.lastStat );
-        return FALSE;
+        return TRUE;
     }
     putTxtBuf( artTxt );
     putEndOfTxt();
-    if ( getStat() != STAT_POST_OK )
+    stat = getStat();
+    if ( stat == STAT_PROGRAM_FAULT )
+	return FALSE;
+    else if ( stat != STAT_POST_OK )
     {
         Log_err( "Posting of %s failed: %s", msgId, client.lastStat );
         strcpy( errStr, client.lastStat );
-        return FALSE;
+        return TRUE;
     }
     Log_inf( "Posted %s (Status: %s)", msgId, client.lastStat );
     return TRUE;
--- a/src/client.h	Wed May 09 12:15:31 2001 +0100
+++ b/src/client.h	Wed May 09 12:33:43 2001 +0100
@@ -3,7 +3,7 @@
 
   Noffle acting as client to other NNTP-servers
 
-  $Id: client.h 183 2000-07-25 12:14:54Z bears $
+  $Id: client.h 279 2001-05-09 11:33:43Z bears $
 */
 
 #ifndef CLIENT_H
@@ -62,14 +62,14 @@
 /*
   Retrieve full article text and store it into database.
 */
-void
+Bool
 Client_retrieveArt( const char *msgId );
 
 /*
   Same, but for a list of msgId's (new line after each msgId).
   All ARTICLE commands are sent and then all answers read.
 */
-void
+Bool
 Client_retrieveArtList( const char *list, int *artcnt, int artmax );
 
 /*
--- a/src/fetch.c	Wed May 09 12:15:31 2001 +0100
+++ b/src/fetch.c	Wed May 09 12:33:43 2001 +0100
@@ -1,7 +1,7 @@
 /*
   fetch.c
 
-  $Id: fetch.c 236 2000-12-05 19:50:09Z enz $
+  $Id: fetch.c 279 2001-05-09 11:33:43Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -59,7 +59,7 @@
     return TRUE;
 }
 
-void
+Bool
 Fetch_getNewGrps( void )
 {
     time_t t;
@@ -71,14 +71,14 @@
     if ( ! Utl_getStamp( &t, file ) )
     {
         Log_err( "Cannot read %s. Please run noffle --query groups", file );
-        return;
+        return FALSE;
     }
     Log_inf( "Updating groupinfo" );
-    Client_getNewgrps( &t );
+    return Client_getNewgrps( &t );
 }
 
 /* Databases open on entry, closed on exit. */
-static void
+static Bool
 fetchNewArts( const char *name, FetchMode mode )
 {
     int next, first, last, refetch;
@@ -88,7 +88,7 @@
         Log_err( "Could not change to group %s", name );
 	if ( Lock_gotLock() )
 	    Lock_closeDatabases();
-        return;
+        return FALSE;
     }
     Client_rmtFirstLast( &first, &last );
     Cont_read( name );
@@ -101,7 +101,7 @@
         Cont_write();
         Grp_setFirstLast( name, Cont_first(), Cont_last() );
 	Lock_closeDatabases();
-        return;
+        return TRUE;
     }
     if ( first == 0 && last == 0 )
     {
@@ -109,7 +109,7 @@
         Cont_write();
         Grp_setFirstLast( name, Cont_first(), Cont_last() );
 	Lock_closeDatabases();
-        return;
+        return TRUE;
     }
     if ( next > last + 1 )
     {
@@ -136,21 +136,21 @@
     Log_inf( "Getting remote overviews %lu-%lu for group %s",
              first, last, name );
     Lock_closeDatabases();
-    Client_getOver( name, first, last, mode );
+    return Client_getOver( name, first, last, mode );
 }
 
-void
+Bool
 Fetch_getNewArts( const char *name, FetchMode mode )
 {
     if ( ! Lock_openDatabases() )
     {
         Log_err( "Could not open message base" );
-        return;
+        return FALSE;
     }
-    fetchNewArts( name, mode );
+    return fetchNewArts( name, mode );
 }
 
-void
+Bool
 Fetch_updateGrps( void )
 {
     FetchMode mode;
@@ -161,7 +161,7 @@
     if ( ! Lock_openDatabases() )
     {
         Log_err( "Could not open message base" );
-        return;
+        return FALSE;
     }
     Fetchlist_read();
     size = Fetchlist_size();
@@ -170,31 +170,35 @@
         Fetchlist_element( &name, &mode, i );
         if ( strcmp( Grp_server( name ), fetch.serv ) == 0 )
 	{
-            fetchNewArts( name, mode );
+            if ( ! fetchNewArts( name, mode ) )
+		return FALSE;
 	    if ( ! Lock_openDatabases() )
 	    {
 		Log_err( "Could not open message base" );
-		return;
+		return FALSE;
 	    }
 	}
     }
     Lock_closeDatabases();
+    return TRUE;
 }
 
-static void
+static Bool
 fetchMessageList( const char *list, int *artcnt, int artmax )
 {
     const char *p;
     Str msgId;
 
     ASSERT( Lock_gotLock() );
-    Client_retrieveArtList( list, artcnt, artmax );
+    if ( ! Client_retrieveArtList( list, artcnt, artmax ) )
+	return FALSE;
     p = list;
     while ( ( p = Utl_getLn( msgId, p ) ) )
         Req_remove( fetch.serv, msgId );
+    return TRUE;
 }
 
-void
+Bool
 Fetch_getReq_( void )
 {
     Str msgId;
@@ -202,6 +206,7 @@
     DynStr *fetchList;
     const char *p;
     int count = 0, artcnt = 0, artmax = 0;
+    Bool res;
 
     ASSERT( fetch.ready );
     Log_dbg( "Retrieving articles marked for download" );
@@ -212,7 +217,7 @@
 	if ( list != NULL )
 	    del_DynStr( list );
         Log_err( "Out of memory in Fetch_get_Req_");
-	return;
+	return FALSE;
     }
 
     /*
@@ -223,7 +228,7 @@
     if ( ! Lock_openDatabases() )
     {
         Log_err( "Could not open message base" );
-        return;
+        return FALSE;
     }
 
     if ( Req_first( fetch.serv, msgId ) )
@@ -239,20 +244,22 @@
 
     /* Retrieve in groups of up to size MAX_ARTICLE_CMDS_QUEUED. */
     p = DynStr_str( list );
-    while ( ( p = Utl_getLn( msgId, p ) ) != NULL )
+    res = TRUE;
+    while ( res && ( p = Utl_getLn( msgId, p ) ) != NULL )
     {
 	DynStr_appLn( fetchList, msgId );
 	if ( ++count % MAX_ARTICLE_CMDS_QUEUED == 0 )
 	{
-	    fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
+	    res = fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
 	    DynStr_clear( fetchList );
 	}
     }
-    fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
+    res = res && fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
 
     del_DynStr( fetchList );
     del_DynStr( list );
     Lock_closeDatabases();
+    return res;
 }
 
 static void
@@ -296,13 +303,15 @@
     }
 }
 
-void
+Bool
 Fetch_postArts( void )
 {
     DynStr *s;
     Str msgId, errStr, sender;
     const char *txt;
+    Bool res;
 
+    res = TRUE;
     s = new_DynStr( 10000 );
     if ( Out_first( fetch.serv, msgId, s ) )
     {
@@ -310,24 +319,35 @@
         do
         {
             txt = DynStr_str( s );
-            Out_remove( fetch.serv, msgId );
-            if ( ! Client_postArt( msgId, txt, errStr ) )
-            {
-                Utl_cpyStr( sender, Cfg_mailTo() );
-                if ( strcmp( sender, "" ) == 0
-                     && ! Prt_searchHeader( txt, "SENDER", sender )
-                     && ! Prt_searchHeader( txt, "X-NOFFLE-X-SENDER",
-                                            sender ) /* see server.c */
-                     && ! Prt_searchHeader( txt, "FROM", sender ) )
-                    Log_err( "Article %s has no From/Sender/X-Sender field",
-                             msgId );
-                else
-                    returnArticleToSender( sender, errStr, txt );
-            }
+	    if ( ! Client_postArt( msgId, txt, errStr ) )
+	    {
+		res = FALSE;
+		break;
+	    }
+	    
+	    /*
+	     * OK, no server communication SNAFU during post. Now, do we
+	     * get an error response? If so, try to return article to sender.
+	     */
+	    Out_remove( fetch.serv, msgId );
+	    if ( errStr[0] != '\0' )
+	    {
+		Utl_cpyStr( sender, Cfg_mailTo() );
+		if ( strcmp( sender, "" ) == 0
+		     && ! Prt_searchHeader( txt, "SENDER", sender )
+		     && ! Prt_searchHeader( txt, "X-NOFFLE-X-SENDER",
+					    sender ) /* see server.c */
+		     && ! Prt_searchHeader( txt, "FROM", sender ) )
+		    Log_err( "Article %s has no From/Sender/X-Sender field",
+			     msgId );
+		else
+		    returnArticleToSender( sender, errStr, txt );
+	    }
         }
         while ( Out_next( msgId, s ) );
     }
     del_DynStr( s );
+    return res;
 }
 
 Bool
--- a/src/fetch.h	Wed May 09 12:15:31 2001 +0100
+++ b/src/fetch.h	Wed May 09 12:33:43 2001 +0100
@@ -3,7 +3,7 @@
 
   Do the daily business by using client.c
 
-  $Id: fetch.h 51 2000-05-05 23:49:38Z uh1763 $
+  $Id: fetch.h 279 2001-05-09 11:33:43Z bears $
 */
 
 #ifndef FETCH_H
@@ -23,20 +23,20 @@
 void
 Fetch_close( void );
 
-void
+Bool
 Fetch_getNewGrps( void );
 
-void
+Bool
 Fetch_updateGrps( void );
 
-void
+Bool
 Fetch_getReq_( void );
 
-void
+Bool
 Fetch_postArts( void );
 
 /* Get new articles in group "grp", using fetch mode "mode". */
-void
+Bool
 Fetch_getNewArts( const char *grp, FetchMode mode );
 
 #endif
--- a/src/noffle.c	Wed May 09 12:15:31 2001 +0100
+++ b/src/noffle.c	Wed May 09 12:33:43 2001 +0100
@@ -10,7 +10,7 @@
   received for some seconds (to allow multiple clients connect at the same
   time).
 
-  $Id: noffle.c 261 2001-02-26 22:41:26Z mnalis $
+  $Id: noffle.c 279 2001-05-09 11:33:43Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -162,17 +162,19 @@
     while ( Cfg_nextServ( serv ) )
         if ( Fetch_init( serv ) )
         {
-            Fetch_postArts();
+	    Bool connOK;
+	    
+            connOK = Fetch_postArts();
 
-            Fetch_getNewGrps();
+            connOK = connOK && Fetch_getNewGrps();
 
             /* Get overviews of new articles and store IDs of new articles
                that are to be fetched becase of FULL or THREAD mode in the
                request database. */
-            Fetch_updateGrps();         
+            connOK = connOK && Fetch_updateGrps();         
 
             /* get requested articles */
-            Fetch_getReq_();
+            connOK = connOK && Fetch_getReq_();
 
             Fetch_close();
         }
@@ -215,9 +217,11 @@
     while ( Cfg_nextServ( serv ) )
         if ( Fetch_init( serv ) )
         {
+	    Bool connOK = TRUE;
+	    
             if ( noffle.queryGrps )
-                Client_getGrps();
-            if ( noffle.queryDsc )
+                connOK = Client_getGrps();
+            if ( connOK && noffle.queryDsc )
                 Client_getDsc();
             Fetch_close();
         }