changeset 188:f1bacee93ca6 noffle

[svn] * src/client.c,src/client.h,src/fetch.c,src/noffle.c,src/server.c: robustness - instead of retruning simple Passed/Failed statuses from connection functions, return an integer status instead. This allows Noffle to distinguish between a connection failure, an unrecoverable protocol error and a recoverable problem. As a concrete instance, Noffle will no longer abort the connection if a group is removed from the upstream server. Also beef up error detection a bit. * src/content.c: instead of overwriting the existing content file(s) when updating - which leaves a window where Noffle is vulnerable to failure which will leave the content file corrupted (yes, it happened to me), write a new content file and rename it over the old file only when it has been written and closed with no errors reported.
author bears
date Wed, 12 Sep 2001 21:33:44 +0100
parents 166008a80f03
children b1b6963fdd63
files ChangeLog src/client.c src/client.h src/content.c src/fetch.c src/noffle.c src/server.c
diffstat 7 files changed, 246 insertions(+), 148 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Sat Sep 01 16:57:45 2001 +0100
+++ b/ChangeLog	Wed Sep 12 21:33:44 2001 +0100
@@ -1,3 +1,18 @@
+Wed Sep 12 2001 Jim Hague <jim.hague@acm.org>
+
+* src/client.c,src/client.h,src/fetch.c,src/noffle.c,src/server.c:
+  robustness - instead of retruning simple Passed/Failed statuses from
+  connection functions, return an integer status instead. This allows
+  Noffle to distinguish between a connection failure, an unrecoverable
+  protocol error and a recoverable problem. As a concrete instance, Noffle
+  will no longer abort the connection if a group is removed from the
+  upstream server. Also beef up error detection a bit.
+* src/content.c: instead of overwriting the existing content file(s) when
+  updating - which leaves a window where Noffle is vulnerable to failure
+  which will leave the content file corrupted (yes, it happened to me),
+  write a new content file and rename it over the old file only when it
+  has been written and closed with no errors reported.
+	
 Sat Sep 01 2001 Markus Enzenberger <me@markus-enzenberger.de>
 
 * src/client.c,src/protocol.h: perform authentication at connect time,
--- a/src/client.c	Sat Sep 01 16:57:45 2001 +0100
+++ b/src/client.c	Wed Sep 12 21:33:44 2001 +0100
@@ -1,7 +1,7 @@
 /*
   client.c
 
-  $Id: client.c 306 2001-09-01 15:57:45Z enz $
+  $Id: client.c 307 2001-09-12 20:33:44Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -209,7 +209,7 @@
     Str lastCmd;
 
     if ( ! getLn( client.lastStat ) )
-        result = STAT_PROGRAM_FAULT;
+        result = STAT_CONNECTION_LOST;
     else if ( sscanf( client.lastStat, "%d", &result ) != 1 )
     {
         Log_err( "Invalid server status: %s", client.lastStat );
@@ -520,7 +520,7 @@
     client.in = client.out = NULL;
 }
 
-static Bool
+static int
 doGetGrps( const char *pattern, Bool *noServerPattern )
 {
     Str cmd;
@@ -536,10 +536,10 @@
 
     *noServerPattern = FALSE;
     if ( ! putCmd( cmd ) )
-        return FALSE;
+        return STAT_CONNECTION_LOST;
     stat = getStat();
-    if ( stat == STAT_PROGRAM_FAULT )
-	return FALSE;
+    if ( IS_FATAL( stat ) )
+        return stat;
 
     /*
      * Try LIST instead of LIST ACTIVE in case server doesn't
@@ -549,37 +549,38 @@
     {
 	*noServerPattern = TRUE;
 	if ( ! putCmd( "LIST" ) )
-	    return FALSE;
+	    return STAT_CONNECTION_LOST;
 	stat = getStat();
     }    
     if ( stat != STAT_GRPS_FOLLOW )    
     {
 	Log_err( "%s failed: %s", cmd, client.lastStat );
-	return FALSE;
+	return stat;
     }
 
     response = collectTxt();
     if ( response == NULL )
-	return FALSE;
+	return STAT_CONNECTION_LOST;
     
     processGrps( DynStr_str( response ), *noServerPattern );
     del_DynStr( response );
-    return TRUE;
+    return STAT_OK;
 }
 
-Bool
+int
 Client_getGrps( void )
 {
     GroupEnum *ge;
     const char *pattern;
-    Bool doneOne, noServerPattern, res;
+    Bool doneOne, noServerPattern;
+    int res;
 
     Log_inf( "Getting groups" );
 
     doneOne = FALSE;
-    res = TRUE;
+    res = STAT_OK;
     ge = new_GetGrEn( client.serv );
-    while ( res && ( pattern = GrEn_next( ge ) ) != NULL )
+    while ( res == STAT_OK && ( pattern = GrEn_next( ge ) ) != NULL )
     {
 	res = doGetGrps( pattern, &noServerPattern );
 	doneOne = TRUE;
@@ -594,7 +595,7 @@
     return res;
 }
 
-static Bool
+static int
 doGetDsc( const char *pattern, Bool *noServerPattern )
 {
     Str name, line, dsc, cmd;
@@ -613,40 +614,40 @@
 
     *noServerPattern = FALSE;
     if ( ! putCmd( cmd ) )
-        return FALSE;
+        return STAT_CONNECTION_LOST;
     stat = getStat();
-    if ( stat == STAT_PROGRAM_FAULT )
-	return FALSE;
+    if ( IS_FATAL( stat ) )
+        return stat;
 
     /* Try without pattern in case server doesn't support patterns. */
     if ( pattern[ 0 ] != '\0' && stat != STAT_GRPS_FOLLOW )
     {
 	*noServerPattern = TRUE;
 	if ( !putCmd( "LIST NEWSGROUPS" ) )
-	     return FALSE;
+	     return STAT_CONNECTION_LOST;
 	stat = getStat();
     }
     if ( stat != STAT_GRPS_FOLLOW )
     {
         Log_err( "%s failed: %s", cmd, client.lastStat );
-        return FALSE;
+        return stat;
     }
 
     response = collectTxt();
     if ( response == NULL )
-	return FALSE;
+	return STAT_CONNECTION_LOST;
     
     if ( ! Lock_openDatabases() )
-	return FALSE;
+	return STAT_NEWSBASE_FATAL;
     
     lines = DynStr_str( response );
-    result = TRUE;
+    result = STAT_OK;
     while ( ( lines = Utl_getLn( line, lines) ) != NULL )
     {
         if ( sscanf( line, "%s", name ) != 1 )
         {
             Log_err( "Unknown reply to LIST NEWSGROUPS: %s", line );
-	    result = FALSE;
+	    result = STAT_PROGRAM_FAULT;
 	    break;
         }
 	if ( *noServerPattern && ! isGetGroup( name ) )
@@ -663,19 +664,20 @@
     return result;
 }
 
-Bool
+int
 Client_getDsc( void )
 {
     GroupEnum *ge;
     const char *pattern;
-    Bool doneOne, noServerPattern, res;
+    Bool doneOne, noServerPattern;
+    int res;
 
     Log_inf( "Querying group descriptions" );
 
     doneOne = FALSE;
-    res = TRUE;
+    res = STAT_OK;
     ge = new_GetGrEn( client.serv );
-    while ( res && ( pattern = GrEn_next( ge ) ) != NULL )
+    while ( res == STAT_OK && ( pattern = GrEn_next( ge ) ) != NULL )
     {
 	res = doGetDsc( pattern, &noServerPattern );
 	doneOne = TRUE;
@@ -683,14 +685,14 @@
 	    break;
     }
 
-    if ( res && ! doneOne )
+    if ( ! doneOne )
 	res = doGetDsc( "", &noServerPattern );
 
     del_GrEn( ge );
     return res;
 }
 
-Bool
+int
 Client_getNewgrps( const time_t *lastTime )
 {
     Str s;
@@ -707,31 +709,21 @@
     */
     p = s + 2;
     if ( ! putCmd( "NEWGROUPS %s GMT", p ) )
-        return FALSE;
+        return STAT_CONNECTION_LOST;
     stat = getStat();
     if ( stat != STAT_NEW_GRP_FOLLOW )
     {
         Log_err( "NEWGROUPS command failed: %s", client.lastStat );
-
-	/*
-	 * If NEWGROUPS fails, it isn't necessarily fatal. You can do
-	 * a periodic noffle --query groups to refresh your group list.
-	 * So only return failure here if the status indicates the link
-	 * itself failed.
-	 *
-	 * In particular, older versions of NNTPcache have a Y2K bug that
-	 * stops NEWGROUPS working.
-	 */
-        return ( stat != STAT_PROGRAM_FAULT );
+        return stat;
     }
 
     response = collectTxt();
     if ( response == NULL )
-	return FALSE;
+	return STAT_CONNECTION_LOST;
     
     processGrps( DynStr_str( response ), TRUE );
     del_DynStr( response );
-    return TRUE;
+    return STAT_OK;
 }
 
 static const char *
@@ -918,7 +910,7 @@
     }
 }
 
-Bool
+int
 Client_getOver( const char *grp, int rmtFirst, int rmtLast, FetchMode mode )
 {
     size_t nbytes, nlines;
@@ -929,6 +921,7 @@
     const char *lines, *groupLines;
     char *p;
     FilterAction action;
+    int stat;
 
     ASSERT( ! Lock_gotLock() );
     ASSERT( strcmp( grp, "" ) != 0 );
@@ -937,11 +930,12 @@
     if ( Flt_getNewsgroups() )
     {
 	if ( ! putCmd( "XHDR Newsgroups %lu-%lu", rmtFirst, rmtLast ) )
-	    return FALSE;
-	if ( getStat() != STAT_HEAD_FOLLOWS )
+	    return STAT_CONNECTION_LOST;
+	stat = getStat();
+	if ( stat != STAT_HEAD_FOLLOWS )
 	{
 	    Log_err( "XHDR command failed: %s", client.lastStat );
-	    return FALSE;
+	    return stat;
 	}
 
 	Log_dbg( LOG_DBG_FETCH,
@@ -950,7 +944,7 @@
 
 	newsgroups = collectTxt();
 	if ( newsgroups == NULL )
-	    return FALSE;
+	    return STAT_CONNECTION_LOST;
 	
 	groupLines = DynStr_str( newsgroups );
     }
@@ -963,14 +957,15 @@
     if ( ! putCmd( "XOVER %lu-%lu", rmtFirst, rmtLast ) )
     {
 	del_DynStr( newsgroups );
-        return FALSE;
+        return STAT_CONNECTION_LOST;
     }
     
-    if ( getStat() != STAT_OVERS_FOLLOW )
+    stat = getStat();
+    if ( stat != STAT_OVERS_FOLLOW )
     {
 	del_DynStr( newsgroups );
         Log_err( "XOVER command failed: %s", client.lastStat );
-        return FALSE;
+        return stat;
     }
     Log_dbg( LOG_DBG_FETCH,
 	     "Requesting overview for remote %lu-%lu",
@@ -980,14 +975,14 @@
     if ( response == NULL )
     {
 	del_DynStr( newsgroups );
-	return FALSE;
+	return STAT_CONNECTION_LOST;
     }
 
     if ( ! Lock_openDatabases() )
     {
 	del_DynStr( newsgroups );
 	del_DynStr( response );
-	return FALSE;
+	return STAT_NEWSBASE_FATAL;
     }
     
     Cont_read( grp );
@@ -1041,7 +1036,7 @@
     Lock_closeDatabases();
     del_DynStr( response );
     del_DynStr( newsgroups );
-    return TRUE;
+    return STAT_OK;
 }
 
 static void
@@ -1057,9 +1052,10 @@
     Pseudo_retrievingFailed( msgId, reason );
     Db_setStatus( msgId, status | DB_RETRIEVING_FAILED );
     Lock_closeDatabases();
+    return;
 }
 
-static Bool
+static int
 retrieveAndStoreArt( const char *msgId, int artcnt, int artmax )
 {
     Bool err;
@@ -1079,7 +1075,7 @@
 	{
 	    del_DynStr( s );
 	    retrievingFailed( msgId, "Can't open message base" );
-	    return FALSE;
+	    return STAT_NEWSBASE_FATAL;
 	}
 	
         err = ! Db_storeArt( msgId, txt );
@@ -1104,61 +1100,74 @@
 	del_DynStr( s );
     }
     else
+    {
         retrievingFailed( msgId, "Connection broke down" );
-    return ! err;
+	return STAT_CONNECTION_LOST;
+    }
+    return err ? STAT_NEWSBASE_FATAL : STAT_OK;
 }
 
-Bool
+int
 Client_retrieveArt( const char *msgId )
 {
-    Bool res = FALSE;
+    int res;
     
     ASSERT( Lock_gotLock() );
     if ( ! Db_contains( msgId ) )
     {
         Log_err( "Article '%s' not prepared in database. Skipping.", msgId );
-        return TRUE;
+        return STAT_PROGRAM_FAULT;
     }
     if ( ! ( Db_status( msgId ) & DB_NOT_DOWNLOADED ) )
     {
         Log_inf( "Article '%s' already retrieved. Skipping.", msgId );
-        return TRUE;
+        return STAT_OK;
     }
 
     Lock_closeDatabases();
     if ( ! putCmd( "ARTICLE %s", msgId ) )
+    {
         retrievingFailed( msgId, "Connection broke down" );
-    else if ( getStat() != STAT_ART_FOLLOWS )
+	res = STAT_CONNECTION_LOST;
+    }
+    else if ( ( res = getStat() ) != STAT_ART_FOLLOWS )
         retrievingFailed( msgId, client.lastStat );
     else
         res = retrieveAndStoreArt( msgId, 0, 0 );
-    Lock_openDatabases();
+    if ( ! Lock_openDatabases() )
+	res = STAT_NEWSBASE_FATAL;
     return res;
 }
 
-Bool
+int
 Client_retrieveArtList( const char *list, int *artcnt, int artmax )
 {
     Str msgId;
     DynStr *s;
     const char *p;
-    Bool res;
+    int res, msgStat;
     
     ASSERT( Lock_gotLock() );
     Log_inf( "Retrieving article list" );
     s = new_DynStr( (int)strlen( list ) );
     p = list;
+    res = STAT_OK;
     while ( ( p = Utl_getLn( msgId, p ) ) )
         if ( ! Db_contains( msgId ) )
-            Log_err( "[%d/%d] Skipping retrieving of %s (not prepared in database)",
+	{
+            Log_err( "[%d/%d] Skipping retrieving of %s "
+		     "(not prepared in database)",
                      ++(*artcnt), artmax, msgId );
+	    res = STAT_PROGRAM_FAULT;
+	}
         else if ( ! ( Db_status( msgId ) & DB_NOT_DOWNLOADED ) )
-            Log_inf( "[%d/%d] Skipping %s (already retrieved)", ++(*artcnt), artmax, msgId );
+            Log_inf( "[%d/%d] Skipping %s (already retrieved)",
+		     ++(*artcnt), artmax, msgId );
         else if ( ! putCmdNoFlush( "ARTICLE %s", msgId ) )
         {
             retrievingFailed( msgId, "Connection broke down" );
             del_DynStr( s );
-            return FALSE;
+            return STAT_CONNECTION_LOST;
         }
         else
             DynStr_appLn( s, msgId );
@@ -1167,55 +1176,58 @@
     fflush( client.out );
     Log_dbg( LOG_DBG_PROTOCOL, "[S FLUSH]" );
     
+    /*
+     * We got something. Try to process all messages and return the
+     * 'worst' error encountered (note we may have already hit a
+     * STAT_PROGRAM_FAULT).
+     */
     p = DynStr_str( s );
-    res = TRUE;
-    while ( res && ( p = Utl_getLn( msgId, p ) ) )
+    while ( ! IS_FATAL( res ) && ( p = Utl_getLn( msgId, p ) ) )
     {
-	switch( getStat() )
-	{
-	case STAT_ART_FOLLOWS:
-	    res = retrieveAndStoreArt( msgId, ++(*artcnt), artmax );
-	    break;
-
-	case STAT_PROGRAM_FAULT:
-	    res = FALSE;
-	    /* Fall through */
-
-	default:
+	msgStat = getStat();
+	if ( msgStat == STAT_ART_FOLLOWS )
+	    msgStat = retrieveAndStoreArt( msgId, ++(*artcnt), artmax );
+	else
             retrievingFailed( msgId, client.lastStat );
-	    break;
-	}
+	    
+	if ( res == STAT_OK || ( ! IS_FATAL( res ) && IS_FATAL( msgStat ) ) )
+	    res = msgStat;
     }
     del_DynStr( s );
-    Lock_openDatabases();
+    if ( ! Lock_openDatabases() && ! IS_FATAL( res ) )
+	res = STAT_NEWSBASE_FATAL;
     return res;
 }
 
-Bool
+int
 Client_changeToGrp( const char* name )
 {
     unsigned int stat;
-    int estimatedNumb, first, last;
-    Bool res;
+    int estimatedNumb, first, last, res;
 
     ASSERT( Lock_gotLock() );
     if ( ! Grp_exists( name ) )
-        return FALSE;
+        return STAT_NEWSBASE_FATAL;
     Lock_closeDatabases();
-    res = putCmd( "GROUP %s", name );
-    res = res && ( getStat() == STAT_GRP_SELECTED );
-    if ( ! Lock_openDatabases() || ! res )
-        return FALSE;
+    stat = STAT_OK;
+    if ( ! putCmd( "GROUP %s", name ) )
+	res = STAT_CONNECTION_LOST;
+    if ( stat == STAT_OK )
+	stat = getStat();
+    if ( ! Lock_openDatabases() )
+	return STAT_NEWSBASE_FATAL;
+    if ( stat != STAT_GRP_SELECTED )
+	return stat;
     if ( sscanf( client.lastStat, "%u %d %d %d",
                  &stat, &estimatedNumb, &first, &last ) != 4 )
     {
         Log_err( "Bad server response to GROUP: %s", client.lastStat );
-        return FALSE;
+        return STAT_PROGRAM_FAULT;
     }
     Utl_cpyStr( client.grp, name );
     client.rmtFirst = first;
     client.rmtLast = last;
-    return TRUE;
+    return STAT_OK;
 }
 
 void
@@ -1226,7 +1238,7 @@
     *last = client.rmtLast;
 }
 
-Bool
+int
 Client_postArt( const char *msgId, const char *artTxt, Str errStr )
 {
     int stat;
@@ -1234,27 +1246,27 @@
     errStr[0] = '\0';
     
     if ( ! putCmd( "POST" ) )
-        return FALSE;
+        return STAT_CONNECTION_LOST;
     stat = getStat();
-    if ( stat == STAT_PROGRAM_FAULT )
-	return FALSE;
+    if ( IS_FATAL( stat ) )
+	return stat;
     else if ( stat != STAT_SEND_ART )
     {
         Log_err( "Posting of %s not allowed: %s", msgId, client.lastStat );
         strcpy( errStr, client.lastStat );
-        return TRUE;
+        return stat;
     }
     putTxtBuf( artTxt );
     putEndOfTxt();
     stat = getStat();
-    if ( stat == STAT_PROGRAM_FAULT )
-	return FALSE;
+    if ( IS_FATAL( stat ) )
+	return stat;
     else if ( stat != STAT_POST_OK )
     {
         Log_err( "Posting of %s failed: %s", msgId, client.lastStat );
         strcpy( errStr, client.lastStat );
-        return TRUE;
+        return stat;
     }
     Log_inf( "Posted %s (Status: %s)", msgId, client.lastStat );
-    return TRUE;
+    return STAT_OK;
 }
--- a/src/client.h	Sat Sep 01 16:57:45 2001 +0100
+++ b/src/client.h	Wed Sep 12 21:33:44 2001 +0100
@@ -3,7 +3,7 @@
 
   Noffle acting as client to other NNTP-servers
 
-  $Id: client.h 279 2001-05-09 11:33:43Z bears $
+  $Id: client.h 307 2001-09-12 20:33:44Z bears $
 */
 
 #ifndef CLIENT_H
@@ -28,6 +28,16 @@
 #include "database.h"
 #include "fetchlist.h"
 
+/*
+  Status values returned by client routines. These will be NNTP status
+  values or the two below, which are not valid NNTP result codes.
+ */
+#define STAT_OK                 0
+#define STAT_CONNECTION_LOST    (-1)
+#define	STAT_NEWSBASE_FATAL	(-2)
+
+#define IS_FATAL(stat)      	(stat < 0)
+
 /* Format of server name: <host>[:<port>] */
 Bool
 Client_connect( const char *serv );
@@ -35,20 +45,20 @@
 void
 Client_disconnect( void );
 
-Bool
+int
 Client_getGrps( void );
 
-Bool
+int
 Client_getDsc( void );
 
-Bool
+int
 Client_getNewgrps( const time_t *lastTime );
 
 /*
   Change to group <name> at server if it is also in current local grouplist.
   Returns TRUE at success.
 */
-Bool
+int
 Client_changeToGrp( const Str name );
 
 /*
@@ -56,20 +66,20 @@
   to the current content. For articles that are to be fetched due to FULL
   or THREAD mode, store IDs in request database.
 */
-Bool
+int
 Client_getOver( const char *grp, int rmtFirst, int rmtLast, FetchMode mode );
 
 /*
   Retrieve full article text and store it into database.
 */
-Bool
+int
 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.
 */
-Bool
+int
 Client_retrieveArtList( const char *list, int *artcnt, int artmax );
 
 /*
@@ -79,7 +89,7 @@
 void
 Client_rmtFirstLast( int *first, int *last );
 
-Bool
+int
 Client_postArt( const char *msgId, const char *artTxt, Str errStr );
 
 #endif
--- a/src/content.c	Sat Sep 01 16:57:45 2001 +0100
+++ b/src/content.c	Wed Sep 12 21:33:44 2001 +0100
@@ -1,7 +1,7 @@
 /*
   content.c
 
-  $Id: content.c 300 2001-08-05 08:24:22Z bears $
+  $Id: content.c 307 2001-09-12 20:33:44Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -10,6 +10,7 @@
 
 #include <stdio.h>
 #include <dirent.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -188,20 +189,26 @@
     int i;
     FILE *f;
     const Over *ov, *ov_next;
+    Str tmpfname;
+    Bool writeErr;
 
     /* If nowt has changed, do nowt. */
     if ( ! cont.dirty )
 	return;
     
-    /* Save the overview */
-    if ( ! ( f = fopen( cont.file, "w" ) ) )
+    /* Save the overview to temporary file in same dir. */
+    snprintf( tmpfname, MAXCHAR, "%s/overview/%s.%d",
+	      Cfg_spoolDir(), cont.name, (int) getpid() ); 
+    if ( ! ( f = fopen( tmpfname, "w" ) ) )
     {
-        Log_err( "Could not open %s for writing", cont.file );
+        Log_err( "Could not open %s for writing", tmpfname );
         return;
     }
-    Log_dbg( LOG_DBG_NEWSBASE, "Writing %s (%lu)", cont.file, cont.size );
+    Log_dbg( LOG_DBG_NEWSBASE, "Writing %s (%lu)", tmpfname, cont.size );
     anythingWritten = FALSE;
     cont.first = -1;
+    writeErr = FALSE;
+    
     for ( i = 0; i < cont.size; ++i )
     {
 	ov = cont.elem[ i ];
@@ -223,21 +230,28 @@
 		 || ( ov_next != NULL &&
 		      Ov_numb( ov_next ) - Ov_numb( ov ) == 1 ) )
             {
+		anythingWritten = TRUE;
                 if ( ! Ov_write( ov, f ) )
                 {
-                    Log_err( "Writing of overview line failed" );
+                    Log_err( "Writing of overview line to %s failed: %s",
+			     tmpfname, strerror( errno ) );
+		    writeErr = TRUE;
                     break;
                 }
                 else
 		{
-                    anythingWritten = TRUE;
 		    if  ( cont.first < 0 )
 			cont.first = cont.vecFirst + i;
 		}
             }
         }
     }
-    fclose( f );
+    if ( fclose( f ) != 0 )
+    {
+	Log_err( "Close of content file %s failed: %s",
+		 tmpfname, strerror( errno ) );
+	writeErr = TRUE;
+    }
 
     /*
       If empty, remove the overview file and set set first to one
@@ -245,11 +259,22 @@
      */
     if ( ! anythingWritten )
     {
-	unlink( cont.file );
-	cont.first = cont.last + 1;
+	if ( unlink( cont.file ) < 0 )
+	    Log_err( "Unlink of %s failed: %s", cont.file, strerror( errno ) );
+	else
+	{
+	    cont.dirty = FALSE;
+	    cont.first = cont.last + 1;
+	}
     }
-
-    cont.dirty = FALSE;
+    else if ( ! writeErr )
+    {
+	if ( rename( tmpfname, cont.file ) < 0 )
+	    Log_err( "Rename of content file %s to %s failed: %s",
+		     tmpfname, cont.file, strerror( errno ) );
+	else
+	    cont.dirty = FALSE;
+    }
 }
 
 const Over *
--- a/src/fetch.c	Sat Sep 01 16:57:45 2001 +0100
+++ b/src/fetch.c	Wed Sep 12 21:33:44 2001 +0100
@@ -1,7 +1,7 @@
 /*
   fetch.c
 
-  $Id: fetch.c 300 2001-08-05 08:24:22Z bears $
+  $Id: fetch.c 307 2001-09-12 20:33:44Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -74,21 +74,32 @@
         return FALSE;
     }
     Log_inf( "Updating groupinfo" );
-    return Client_getNewgrps( &t );
+
+    /*
+     * If NEWGROUPS fails, it isn't necessarily fatal. You can do
+     * a periodic noffle --query groups to refresh your group list.
+     * So only return failure here if the status indicates the link
+     * itself failed.
+     *
+     * In particular, older versions of NNTPcache have a Y2K bug that
+     * stops NEWGROUPS working.
+     */
+    return ( ! IS_FATAL( Client_getNewgrps( &t ) ) );
 }
 
 /* Databases open on entry, closed on exit. */
-static Bool
+static int
 fetchNewArts( const char *name, FetchMode mode )
 {
-    int next, first, last, refetch;
+    int next, first, last, refetch, stat;
 
-    if ( ! Client_changeToGrp( name ) )
+    stat = Client_changeToGrp( name );
+    if ( stat != STAT_OK )
     {
         Log_err( "Could not change to group %s", name );
 	if ( Lock_gotLock() )
 	    Lock_closeDatabases();
-        return TRUE;
+        return stat;
     }
     Client_rmtFirstLast( &first, &last );
     Cont_read( name );
@@ -101,7 +112,7 @@
         Cont_write();
         Grp_setFirstLast( name, Cont_first(), Cont_last() );
 	Lock_closeDatabases();
-        return TRUE;
+        return STAT_OK;
     }
     if ( first == 0 && last == 0 )
     {
@@ -109,13 +120,14 @@
         Cont_write();
         Grp_setFirstLast( name, Cont_first(), Cont_last() );
 	Lock_closeDatabases();
-        return TRUE;
+        return STAT_OK;
     }
     if ( next > last + 1 )
     {
     	refetch = last - Cfg_maxFetch() + 1;
     	if ( refetch < 0 ) refetch = 1;
-        Log_err( "Article number inconsistent (%s rmt=%lu-%lu, next=%lu). Refetching from %lu",
+        Log_err( "Article number inconsistent (%s rmt=%lu-%lu, next=%lu). "
+		 "Refetching from %lu",
                  name, first, last, next, refetch );
         Pseudo_cntInconsistent( name, first, last, next, refetch );
         first = refetch;
@@ -125,6 +137,16 @@
         Log_inf( "Missing articles (%s first=%lu next=%lu)",
                  name, first, next );
         Pseudo_missArts( name, first, next );
+
+	/*
+	 * If we are missing articles but there are none to fetch,
+	 * we must ensure we don't repeatedly generate missing
+	 * article warning on every fetch until there is something
+	 * to fetch. To guard against this, update the group remote
+	 * next now.
+	 */
+	Grp_setRmtNext( name, first );
+	next = first;
     }
     else
         first = next;
@@ -170,7 +192,7 @@
         Fetchlist_element( &name, &mode, i );
         if ( strcmp( Grp_server( name ), fetch.serv ) == 0 )
 	{
-            if ( ! fetchNewArts( name, mode ) )
+            if ( IS_FATAL( fetchNewArts( name, mode ) ) )
 		return FALSE;
 	    if ( ! Lock_openDatabases() )
 	    {
@@ -183,19 +205,21 @@
     return TRUE;
 }
 
-static Bool
+static int
 fetchMessageList( const char *list, int *artcnt, int artmax )
 {
     const char *p;
     Str msgId;
+    int stat;
 
     ASSERT( Lock_gotLock() );
-    if ( ! Client_retrieveArtList( list, artcnt, artmax ) )
-	return FALSE;
+    stat = Client_retrieveArtList( list, artcnt, artmax );
+    if ( stat != STAT_OK )
+	return stat;
     p = list;
     while ( ( p = Utl_getLn( msgId, p ) ) )
         Req_remove( fetch.serv, msgId );
-    return TRUE;
+    return STAT_OK;
 }
 
 Bool
@@ -207,6 +231,7 @@
     const char *p;
     int count = 0, artcnt = 0, artmax = 0;
     Bool res;
+    int stat;
 
     ASSERT( fetch.ready );
     Log_dbg( LOG_DBG_FETCH, "Retrieving articles marked for download" );
@@ -250,11 +275,14 @@
 	DynStr_appLn( fetchList, msgId );
 	if ( ++count % MAX_ARTICLE_CMDS_QUEUED == 0 )
 	{
-	    res = fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
+	    stat = fetchMessageList( DynStr_str( fetchList ), &artcnt,
+				     artmax );
+	    res = ! IS_FATAL( stat );
 	    DynStr_clear( fetchList );
 	}
     }
-    res = res && fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
+    stat = fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax );
+    res = res && ! IS_FATAL( stat );
 
     del_DynStr( fetchList );
     del_DynStr( list );
@@ -319,7 +347,7 @@
         do
         {
             txt = DynStr_str( s );
-	    if ( ! Client_postArt( msgId, txt, errStr ) )
+	    if ( Client_postArt( msgId, txt, errStr ) != STAT_OK )
 	    {
 		res = FALSE;
 		break;
--- a/src/noffle.c	Sat Sep 01 16:57:45 2001 +0100
+++ b/src/noffle.c	Wed Sep 12 21:33:44 2001 +0100
@@ -10,7 +10,7 @@
   received for some seconds (to allow multiple clients connect at the same
   time).
 
-  $Id: noffle.c 300 2001-08-05 08:24:22Z bears $
+  $Id: noffle.c 307 2001-09-12 20:33:44Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -217,11 +217,11 @@
     while ( Cfg_nextServ( serv ) )
         if ( Fetch_init( serv ) )
         {
-	    Bool connOK = TRUE;
+	    int stat = STAT_OK;
 	    
             if ( noffle.queryGrps )
-                connOK = Client_getGrps();
-            if ( connOK && noffle.queryDsc )
+                stat = Client_getGrps();
+            if ( stat == STAT_OK && noffle.queryDsc )
                 Client_getDsc();
             Fetch_close();
         }
--- a/src/server.c	Sat Sep 01 16:57:45 2001 +0100
+++ b/src/server.c	Wed Sep 12 21:33:44 2001 +0100
@@ -1,7 +1,7 @@
 /*
   server.c
 
-  $Id: server.c 300 2001-08-05 08:24:22Z bears $
+  $Id: server.c 307 2001-09-12 20:33:44Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -394,6 +394,7 @@
 retrieveArt( const char *msgId )
 {
     Str s;
+    int stat;
 
     findServer( msgId, s );    
     if ( strcmp( s, "(unknown)" ) == 0 
@@ -405,8 +406,15 @@
         Online_set( FALSE );
         return FALSE;
     }
-    Client_retrieveArt( msgId );
+    stat = Client_retrieveArt( msgId );
     Client_disconnect();
+    if ( IS_FATAL( stat ) )
+    {
+        Log_inf( "Server connection failed or newsbase problem. "
+		 "Leaving online mode." );        
+        Online_set( FALSE );
+        return FALSE;
+    }
     return TRUE;
 }