# HG changeset patch
# User bears
# Date 1007901117 0
# Node ID ffb1848a39db3d70311e351e0e86db0542b78152
# Parent  bf290632d29e27a39a07a681cadbb5d0d6c5527f
[svn] * src/util.c: Improve (correct) error detection when updating
  timestamp file.
* src/content.h, src/content.c: Return Boolean success/fail from
  Cont_write. Also ensure cont.first isn't polluted in the event
  of a failed update.
* src/client.c,src/control.c,src/fetch.c,src/noffle.c,src/post.c,
  src/pseudo.c: If Cont_write fails, don't do actions that need it to
  have worked. Typically, don't update first and last article numbers
  in group database.
* src/server.c: If groupinfo.lastupdate is unreadable or corrupt,
  spot this and report it and give an explicit error when processing
  NNTP NEWGROUPS command.

diff -r bf290632d29e -r ffb1848a39db ChangeLog
--- a/ChangeLog	Sun Dec 09 11:32:31 2001 +0000
+++ b/ChangeLog	Sun Dec 09 12:31:57 2001 +0000
@@ -1,3 +1,16 @@
+Sun Dec 9 2001 Jim Hague <jim.hague@acm.org>
+
+* src/util.c: Improve (correct) error detection when updating
+  timestamp file.
+* src/content.h, src/content.c: Return Boolean success/fail from
+  Cont_write. Also ensure cont.first isn't polluted in the event
+  of a failed update.
+* src/client.c,src/control.c,src/fetch.c,src/noffle.c,src/post.c,
+  src/pseudo.c,
+* src/server.c: If groupinfo.lastupdate is unreadable or corrupt,
+  spot this and report it and give an explicit error when processing
+  NNTP NEWGROUPS command.
+
 Sun Dec 9 2001 Jim Hague <jim.hague@acm.org>
 
 * src/post.c: Always replace message ID in posted message if existing
diff -r bf290632d29e -r ffb1848a39db src/client.c
--- a/src/client.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/client.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   client.c
 
-  $Id: client.c 334 2001-11-22 15:35:54Z mirkol $
+  $Id: client.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -1034,8 +1034,8 @@
         Log_inf( "Added %s %lu-%lu", client.grp, oldLast + 1, Cont_last() );
 	Log_inf( "%u articles marked for download in %s",
 		 cntMarked, client.grp  );
-	Cont_write();
-	Grp_setFirstLast( grp, Cont_first(), Cont_last() );
+	if ( Cont_write() )
+            Grp_setFirstLast( grp, Cont_first(), Cont_last() );
 	Grp_setLastPostTime( grp );
     }
     Lock_closeDatabases();
diff -r bf290632d29e -r ffb1848a39db src/content.c
--- a/src/content.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/content.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   content.c
 
-  $Id: content.c 338 2001-11-29 22:16:06Z bears $
+  $Id: content.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -183,7 +183,7 @@
     }
 }
 
-void
+Bool
 Cont_write( void )
 {
     Bool anythingWritten;
@@ -192,10 +192,11 @@
     const Over *ov, *ov_next;
     Str tmpfname;
     Bool writeErr;
+    int first;
 
     /* If nowt has changed, do nowt. */
     if ( ! cont.dirty )
-	return;
+	return TRUE;
     
     /* Save the overview to temporary file in same dir. */
     /* old tmpfnames will be expired at noffle.c:expireContents() */
@@ -204,11 +205,11 @@
     if ( ! ( f = fopen( tmpfname, "w" ) ) )
     {
         Log_err( "Could not open %s for writing", tmpfname );
-        return;
+        return FALSE;
     }
     Log_dbg( LOG_DBG_NEWSBASE, "Writing %s (%lu)", tmpfname, cont.size );
     anythingWritten = FALSE;
-    cont.first = -1;
+    first = -1;
     writeErr = FALSE;
     
     for ( i = 0; i < cont.size; ++i )
@@ -232,7 +233,7 @@
 		 || ( ov_next != NULL &&
 		      Ov_numb( ov_next ) - Ov_numb( ov ) == 1 ) )
             {
-		anythingWritten = TRUE;
+                anythingWritten = TRUE;
                 if ( ! Ov_write( ov, f ) )
                 {
                     Log_err( "Writing of overview line to %s failed: %s",
@@ -242,8 +243,8 @@
                 }
                 else
 		{
-		    if  ( cont.first < 0 )
-			cont.first = cont.vecFirst + i;
+		    if  ( first < 0 )
+			first = cont.vecFirst + i;
 		}
             }
         }
@@ -255,8 +256,14 @@
 	writeErr = TRUE;
     }
 
+    if ( writeErr )
+    {
+        /* Write error - leave everything as at present */
+        return FALSE;
+    }
+    
     /*
-      If empty, remove the overview file and set set first to one
+      If empty, remove the overview file and set first to one
       beyond last to flag said emptiness.
      */
     if ( ! anythingWritten )
@@ -264,21 +271,33 @@
 	if ( unlink( tmpfname ) < 0 )
 	    Log_err( "Unlink of %s failed: %s", tmpfname, strerror( errno ) );
 	if ( unlink( cont.file ) < 0 )
+        {
 	    Log_err( "Unlink of %s failed: %s", cont.file, strerror( errno ) );
+            return FALSE;
+        }
 	else
 	{
 	    cont.dirty = FALSE;
 	    cont.first = cont.last + 1;
 	}
     }
-    else if ( ! writeErr )
+    else
     {
 	if ( rename( tmpfname, cont.file ) < 0 )
+        {
 	    Log_err( "Rename of content file %s to %s failed: %s",
 		     tmpfname, cont.file, strerror( errno ) );
+            return FALSE;
+        }
 	else
+        {
+            ASSERT( first != -1 );
 	    cont.dirty = FALSE;
+            cont.first = first;
+        }
     }
+
+    return TRUE;
 }
 
 const Over *
diff -r bf290632d29e -r ffb1848a39db src/content.h
--- a/src/content.h	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/content.h	Sun Dec 09 12:31:57 2001 +0000
@@ -8,7 +8,7 @@
   filename SPOOLDIR/overview/GROUPNAME. One entire overview file is read
   and cached in memory, at a time.
 
-  $Id: content.h 55 2000-05-06 16:56:50Z bears $ 
+  $Id: content.h 342 2001-12-09 12:31:57Z bears $ 
 */
 
 #ifndef CONT_H
@@ -22,7 +22,8 @@
 
 /*
   Try to read overviews from overview file for group <grp>.
-  Fill with fake articles, if something goes wrong.
+  Ignore any badly-formatted lines and create a new empty
+  overview file if no lines can be read.
 */
 void
 Cont_read( const char *grp );
@@ -35,8 +36,8 @@
 void
 Cont_app( Over *ov );
 
-/* Write content */
-void
+/* Write content. Return FALSE on error. */
+Bool
 Cont_write( void );
 
 Bool
diff -r bf290632d29e -r ffb1848a39db src/control.c
--- a/src/control.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/control.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   control.c
 
-  $Id: control.c 316 2001-10-31 11:44:53Z bears $
+  $Id: control.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -27,6 +27,7 @@
     Str server;
     Bool seen = FALSE;
     int res = CANCEL_OK;
+    Bool removeFromDb = TRUE;
 
     /* See if in outgoing and zap if so. */
     if ( Out_find( msgId, server ) )
@@ -60,14 +61,21 @@
 	{
 	    Cont_read( grp );
 	    Cont_delete( no );
-	    Cont_write();
+            /*
+             * If we don't manage to remove the overview, leave the message
+             * in the main database to avoid confusion. Yes, the message
+             * will not be cancelled properly for this group but we don't
+             * signal this to the calling routine.
+             */
+	    if ( Cont_write() )
+                Log_dbg( LOG_DBG_CONTROL,
+                         "Removed '%s' from group '%s'.",
+                         msgId, grp );
+            else
+                removeFromDb = FALSE;
 
 	    if ( ! Grp_local( grp ) && ! seen )
 		res = CANCEL_NEEDS_MSG;
-
-	    Log_dbg( LOG_DBG_CONTROL,
-		     "Removed '%s' from group '%s'.",
-		     msgId, grp );
 	}
 	else
 	{
@@ -75,7 +83,8 @@
 	}
     }
     del_Itl( refs );
-    Db_delete( msgId );
+    if ( removeFromDb )
+        Db_delete( msgId );
     Log_inf( "Message '%s' cancelled.", msgId );
     return res;
 }
diff -r bf290632d29e -r ffb1848a39db src/fetch.c
--- a/src/fetch.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/fetch.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   fetch.c
 
-  $Id: fetch.c 316 2001-10-31 11:44:53Z bears $
+  $Id: fetch.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -109,16 +109,16 @@
     if ( next == last + 1 )
     {
         Log_inf( "No new articles in %s", name );
-        Cont_write();
-        Grp_setFirstLast( name, Cont_first(), Cont_last() );
+        if ( Cont_write() )
+            Grp_setFirstLast( name, Cont_first(), Cont_last() );
 	Lock_closeDatabases();
         return STAT_OK;
     }
     if ( first == 0 && last == 0 )
     {
         Log_inf( "No articles in %s", name );
-        Cont_write();
-        Grp_setFirstLast( name, Cont_first(), Cont_last() );
+        if ( Cont_write() )
+            Grp_setFirstLast( name, Cont_first(), Cont_last() );
 	Lock_closeDatabases();
         return STAT_OK;
     }
diff -r bf290632d29e -r ffb1848a39db src/noffle.c
--- a/src/noffle.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/noffle.c	Sun Dec 09 12:31:57 2001 +0000
@@ -10,7 +10,7 @@
   received for some seconds (to allow multiple clients connect at the same
   time).
 
-  $Id: noffle.c 337 2001-11-22 22:48:07Z bears $
+  $Id: noffle.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -293,8 +293,8 @@
 		Fetchlist_remove( grp );
 		Grp_setRmtNext( grp, GRP_RMT_NEXT_NOT_SUBSCRIBED );
             }
-            Cont_write();
-            Grp_setFirstLast( grp, Cont_first(), Cont_last() );
+            if ( Cont_write() )
+                Grp_setFirstLast( grp, Cont_first(), Cont_last() );
             Log_inf( "%ld overviews deleted from group %s, %ld left (%ld-%ld)",
                      cntDel, grp, cntLeft, Grp_first( grp ), Grp_last( grp ) );
         }
@@ -380,9 +380,11 @@
 	    if ( toDelete )
 		Db_delete( msgId );
 	}
-	Cont_write();
-	Grp_delete( name );
-	printf( "Group '%s' deleted.\n", name );
+        if ( Cont_write() )
+        {
+            Grp_delete( name );
+            printf( "Group '%s' deleted.\n", name );
+        }
     }
 }
 
diff -r bf290632d29e -r ffb1848a39db src/post.c
--- a/src/post.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/post.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   post.c
 
-  $Id: post.c 341 2001-12-09 11:32:31Z bears $
+  $Id: post.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -97,10 +97,14 @@
 	Db_setXref( msgId, t );
     }
     
-    Cont_write();
-    Grp_setFirstLast( Cont_grp(), Cont_first(), Cont_last() );
-    Grp_setLastPostTime( Cont_grp() );
-    return TRUE;
+    if ( Cont_write() )
+    {
+        Grp_setFirstLast( Cont_grp(), Cont_first(), Cont_last() );
+        Grp_setLastPostTime( Cont_grp() );
+        return TRUE;
+    }
+    else
+        return FALSE;
 }
 
 static Bool
@@ -179,7 +183,6 @@
     s = new_DynStr( 10000 );
     article.text = s;
 
-    memset( &article.over, 0, sizeof( article.over ) );
     replyToFound = pathFound = orgFound = FALSE;
     
     /* Grab header lines first, getting overview info as we go. */
@@ -539,6 +542,19 @@
     return postExternal() && err;
 }
 
+
+static void
+clearArticleInfo( void )
+{
+    article.text = NULL;
+    article.newsgroups = NULL;
+    article.control = NULL;
+    article.approved = FALSE;
+    article.posted = FALSE;
+    article.flags = 0;
+    memset( &article.over, 0, sizeof( article.over ) );
+}
+
 /* Register an article for posting. */
 Bool
 Post_open( const char * text, unsigned flags )
@@ -549,8 +565,9 @@
 	return FALSE;
     }
 
+    clearArticleInfo();
+    
     article.flags = flags;
-    
     if ( ! getArticleText( text ) )
 	return FALSE;
 
@@ -601,8 +618,6 @@
 	del_Itl( article.control );
 	article.control = NULL;
     }
-    article.approved = FALSE;
-    article.posted = FALSE;
 }
 
 
diff -r bf290632d29e -r ffb1848a39db src/pseudo.c
--- a/src/pseudo.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/pseudo.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   pseudo.c
   
-  $Id: pseudo.c 316 2001-10-31 11:44:53Z bears $
+  $Id: pseudo.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -206,8 +206,8 @@
         Cont_app( ov );
         if ( Db_prepareEntry( ov, Cont_grp(), Cont_last() ) )
             Db_storeArt( Ov_msgId( ov ), DynStr_str( artTxt ) );
-        Cont_write();
-        Grp_setFirstLast( Cont_grp(), Cont_first(), Cont_last() );
+        if ( Cont_write() )
+            Grp_setFirstLast( Cont_grp(), Cont_first(), Cont_last() );
     }
     del_DynStr( body );
     del_DynStr( artTxt );
diff -r bf290632d29e -r ffb1848a39db src/server.c
--- a/src/server.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/server.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   server.c
 
-  $Id: server.c 316 2001-10-31 11:44:53Z bears $
+  $Id: server.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -938,10 +938,17 @@
     }
     snprintf( file, MAXCHAR, "%s/groupinfo.lastupdate", Cfg_spoolDir() );
     t = getTimeInSeconds( year, mon, day, hour, min, sec );
+
+    if ( ! Utl_getStamp( &lastUpdate, file ) )
+    {
+        /* Can't get stamp. Put out error message. */
+        putStat( STAT_PROGRAM_FAULT, "Server error reading %s", file );        
+        return TRUE;
+    }
+    
     putStat( STAT_NEW_GRP_FOLLOW, "New groups since %s", arg );
 
-    if ( ! Utl_getStamp( &lastUpdate, file )
-         || t == (time_t)-1 || t <= lastUpdate )
+    if ( t == (time_t)-1 || t <= lastUpdate )
     {
         if ( Grp_firstGrp( &g ) )
             do
diff -r bf290632d29e -r ffb1848a39db src/util.c
--- a/src/util.c	Sun Dec 09 11:32:31 2001 +0000
+++ b/src/util.c	Sun Dec 09 12:31:57 2001 +0000
@@ -1,7 +1,7 @@
 /*
   util.c
 
-  $Id: util.c 325 2001-11-14 10:56:42Z bears $
+  $Id: util.c 342 2001-12-09 12:31:57Z bears $
 */
 
 #if HAVE_CONFIG_H
@@ -255,8 +255,7 @@
         return;
     }
     fprintf( f, "%lu\n", t );
-    fclose( f );
-    if ( ferror( f ) )
+    if ( fclose( f ) != 0 )
     {
          Log_err( "Error stamping into file %s: %s",
 		 tmpfname, strerror( errno ) );