Think I found a bunch of the makehistory problem

[Russ Allbery]

At least with tradindexed.

Test suites rock.  Don't ever write anything without a test suite if you
can possibly help it.

The tradindexed test suite wasn't testing group repacks (redoing the group
index because an article comes in with a lower number than the current
base number of the index file), and after I fiddled with it to test that,
it started failing.  Inspection with my new tradindexed dumping code
revealed that the highest numbered article had its index entry in the
wrong slot; it was in the slot for article 463 instead of 464 (but had all
the right data).

After a lot more debugging, I found an off-by-one error in the repacking
code that only hits if the new base of the index file is still larger than
one (which would explain why it didn't show up in ad hoc testing).  The
result is that all the existing index entries are shifted up one less than
they should be, and hence all show up as corresponding to the wrong
article numbers.

Repacks only happen when one gets articles out of order (and furthermore
out of order by more than the 128-article slop factor that tradindexed
always adds), so this bug primarily affects makehistory and would be
rarely seen during normal operation.

Anyway, here's the patch.  As explanation, observe that ge->base is the
delta between article numbers and positions in the index file.  The
article numbers are 1-based, whereas positions are 0-based, which means
that the minimum value of ge->base should be 1.  delta is the amount that
we're reducing ge->base by.  Which means that setting delta equal to
ge->base will do the wrong thing.  (As it turns out, setting ge->base to 0
will cause tradindexed to reset it to 1 the next time it stores a record,
since the low water mark will be less than 128 -- if it weren't, we
wouldn't be in this code pass at all.)

The -1 in pwrite was a change added to correct for having gotten the delta
setting wrong earlier.  It makes things work properly if we're shifting
down to a base of 1, but is off by one in all other cases.

This patch has been applied to STABLE as well.

Index: ov3.c
===================================================================
RCS file: /dist1/cvs/isc/inn/inn/storage/ov3/ov3.c,v
retrieving revision 1.41
diff -u -r1.41 ov3.c
--- ov3.c	2002/02/28 21:27:59	1.41
+++ ov3.c	2002/03/09 03:10:10
@@ -1009,7 +1009,7 @@
     syslog(L_NOTICE, "tradindexed: repacking group %s, offset %d", group, delta);
     GROUPlock(gloc, INN_LOCK_WRITE);
 
-    if (delta > ge->base) delta = ge->base;
+    if (delta >= ge->base) delta = ge->base - 1;
 
     strcpy(bakgroup, group);
     strcat(bakgroup, "-BAK");
@@ -1062,7 +1062,7 @@
     }
 
     if (pwrite(fd, &gh->indexmem[ge->low - ge->base] , nbytes,
-	       sizeof(INDEXENTRY)*(ge->low - ge->base + delta - 1)) != nbytes) {
+	       sizeof(INDEXENTRY)*(ge->low - ge->base + delta)) != nbytes) {
 	syslog(L_ERROR, "tradindexed: packgroup cant write to %s: %m", newidx);
 	close(fd);
 	OV3closegroup(gh, FALSE);


-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>