OVSTATICSEARCH bug

[Jeffrey M. Vinocur]

Hi all --

Below is a patch that I've been using for a while.  

A bunch of my users were in agreement that they were seeing odd behavior
that correlated with XOVER malfunctioning (never seeing articles appear in
their newsreaders, mainly).  Some were able to capture traces of the
incriminating sessions and it was pretty clear that garbled overview data 
was responsible.

After some debugging that I no longer can recall the details of, I managed 
to find the bit of code that seemed to be responsible.  Everything has 
been working fine without it for many months, but I'm not sure I fully 
understand all of the implications of this change.

What with all of the wonderful progress we've seen lately I thought I 
ought to bring this up so somebody (Russ?) more familiar with tradindexed 
can tell me it's okay to commit.


Index: storage/tradindexed/tdx-data.c
===================================================================
--- storage/tradindexed/tdx-data.c      (revision 8379)
+++ storage/tradindexed/tdx-data.c      (working copy)
@@ -462,6 +462,7 @@
         unmap_index(data);
         map_index(data);
         data->high = high;
+       unmap_data(data);
     }
     if (start > data->high)
         return NULL;
@@ -512,17 +513,13 @@
     if (search->current > search->limit || search->current > max)
         return false;

-    /* Make sure that the offset into the data file is sensible, and try
-       remapping the data file if the portion the offset is pointing to isn't
-       currently mapped.  Otherwise, warn about possible corruption and return
-       a miss. */
+    /* There is a small chance that remapping the data file could make this
+       offset accessible, but changing the memory location in the middle of
+       a search conflicts with our OVSTATICSEARCH.  And it seems not to
+       be an issue in limited testing, although write caching that leads to
+       on-disk IDX and DAT being out of sync could trigger a problem here. */
     if (entry->offset + entry->length > search->data->datalen) {
-        unmap_data(search->data);
-        if (!map_data(search->data))
-            return false;
-    }
-    if (entry->offset + entry->length > search->data->datalen) {
-        warn("Invalid entry for article %lu in %s.IDX: offset %lu length %lu",
+        warn("Invalid or inaccessible entry for article %lu in %s.IDX: 
offset %lu length %lu",
              search->current + search->data->base, search->data->path,
              (unsigned long) entry->offset, (unsigned long) 
entry->length);
         return false;
Index: doc/man/libstorage.3
===================================================================
--- doc/man/libstorage.3        (revision 8379)
+++ doc/man/libstorage.3        (working copy)
@@ -327,6 +327,9 @@
 OVSPACE        probe overview space usage
 OVSTATALL      stat all articles when
        OVexpiregroup is called
+OVSTATICSEARCH if results of OVsearch are stored
+       in a static buffer and must be copied
+       before the next call to OVsearch
 .fi
 .in -0.5i
 .sp 1
@@ -352,6 +355,11 @@
 .I low
 and
 .IR high .
+The setting of OVSTATICSEARCH determines how search result data must be
+handled.  (Note that with some storage methods, each call to OVopensearch
+may cause internal storage to be remapped.  Therefore multiple
+simultaneous searches may require data to be copied in between OVsearch
+calls even if OVSTATICSEARCH is false.)
 .PP
 .I OVsearch
 retrieves information; article number, overview data, or arrival time.

-- 
Jeffrey M. Vinocur
jeff at litech.org