Various fixes resulting from QA code review.

Some documentation fixes.
More clear handling of what is and isn't a valid indexes.
Added functions for the 'really long lived circular buffer use case'
Improved unit tests.

3 files changed, 72 insertions, 35 deletions

diff --git a/src/corelib/io/qdebug.h b/src/corelib/io/qdebug.h
index 6c05756..9b0fbe5 100644
--- a/src/corelib/io/qdebug.h
+++ b/src/corelib/io/qdebug.h
@@ -235,7 +235,7 @@ inline QDebug operator<<(QDebug debug, const QSet<T> &set)
 
 #if defined(FORCE_UREF)
 template <class T>
-inline QDebug &operator<<(QDebug debug, const QContiguousCache<T> &contiguousCache)
+inline QDebug &operator<<(QDebug debug, const QContiguousCache<T> &cache)
 #else
 template <class T>
 inline QDebug operator<<(QDebug debug, const QContiguousCache<T> &cache)
diff --git a/src/corelib/tools/qcontiguouscache.cpp b/src/corelib/tools/qcontiguouscache.cpp
index 95fa9e7..6671982 100644
--- a/src/corelib/tools/qcontiguouscache.cpp
+++ b/src/corelib/tools/qcontiguouscache.cpp
@@ -62,9 +62,10 @@ void QContiguousCacheData::dump() const
     display in a user interface view.  Unlike QCache, it adds a restriction
     that elements within the cache are contiguous.  This has the advantage
     of matching how user interface views most commonly request data, as
-    a set of rows localized around the current scrolled position.  It also
-    allows the cache to consume less memory and processor cycles than QCache
-    for this use-case.
+    a set of rows localized around the current scrolled position.  This
+    restriction allows the cache to consume less memory and processor
+    cycles than QCache.  The QContiguousCache class also can provide
+    an upper bound on memory usage via setCapacity().
 
     The simplest way of using a contiguous cache is to use the append()
     and prepend().
@@ -83,8 +84,8 @@ MyRecord record(int row) const
 }
 \endcode
 
-    If the cache is full then the item at the opposite end of the cache from where
-    the new item is appended or prepended will be removed.
+    If the cache is full then the item at the opposite end of the cache from
+    where the new item is appended or prepended will be removed.
 
     This usage can be further optimized by using the insert() function
     in the case where the requested row is a long way from the currently cached
@@ -93,6 +94,19 @@ MyRecord record(int row) const
     the contiguous nature of the cache.  Hence it is important to take some care then
     when using insert() in order to avoid unwanted clearing of the cache.
 
+    The range of valid indexes for the QContiguousCache class are from
+    0 to INT_MAX.  Calling prepend() such that the first index would become less
+    than 0 or append() such that the last index would become greater
+    than INT_MAX can result in the indexes of the cache being invalid.
+    When the cache indexes are invalid it is important to call
+    normalizeIndexes() before calling any of containsIndex(), firstIndex(),
+    lastIndex(), at() or the [] operator.  Calling these
+    functions when the cache has invalid indexes will result in undefined
+    behavior.  The indexes can be checked by using areIndexesValid()
+
+    In most cases the indexes will not exceed 0 to INT_MAX, and
+    normalizeIndexes() will not need to be be used.
+
     See the \l{Contiguous Cache Example}{Contiguous Cache} example.
 */
 
@@ -288,6 +302,10 @@ MyRecord record(int row) const
     worthwhile taking effort to insert items in an order that starts adjacent
     to the current index range for the cache.
 
+    The range of valid indexes for the QContiguousCache class are from
+    0 to INT_MAX. Inserting outside of this range has undefined behavior.
+
+
     \sa prepend(), append(), isFull(), firstIndex(), lastIndex()
 */
 
@@ -301,24 +319,14 @@ MyRecord record(int row) const
 /*! \fn int QContiguousCache::firstIndex() const
 
     Returns the first valid index in the cache.  The index will be invalid if the
-    cache is empty.  However the following code is valid even when the cache is empty:
-
-    \code
-    for (int i = cache.firstIndex(); i <= cache.lastIndex(); ++i)
-        qDebug() << "Item" << i << "of the cache is" << cache.at(i);
-    \endcode
+    cache is empty.
 
     \sa capacity(), size(), lastIndex()
 */
 
 /*! \fn int QContiguousCache::lastIndex() const
 
-    Returns the last valid index in the cache.  If the cache is empty will return -1.
-
-    \code
-    for (int i = cache.firstIndex(); i <= cache.lastIndex(); ++i)
-        qDebug() << "Item" << i << "of the cache is" << cache.at(i);
-    \endcode
+    Returns the last valid index in the cache.  The index will be invalid if the cache is empty.
 
     \sa capacity(), size(), firstIndex()
 */
@@ -386,6 +394,37 @@ MyRecord record(int row) const
     \sa takeFirst(), removeLast()
 */
 
+/*! \fn void QContiguousCache::normalizeIndexes()
+
+    Moves the first index and last index of the cache
+    such that they point to valid indexes.  The function does not modify
+    the contents of the cache or the ordering of elements within the cache.
+
+    It is provided so that index overflows can be corrected when using the
+    cache as a circular buffer.
+
+    \code
+    QContiguousCache<int> cache(10);
+    cache.insert(INT_MAX, 1); // cache contains one value and has valid indexes, INT_MAX to INT_MAX
+    cache.append(2); // cache contains two values but does not have valid indexes.
+    cache.normalizeIndexes(); // cache has two values, 1 and 2.  New first index will be in the range of 0 to capacity().
+    \endcode
+
+    \sa areIndexesValid(), append(), prepend()
+*/
+
+/*! \fn bool QContiguousCache::areIndexesValid() const
+
+    Returns whether the indexes for items stored in the cache are valid.
+    Indexes can become invalid if items are appended after the index position
+    INT_MAX or prepended before the index position 0.  This is only expected
+    to occur in very long lived circular buffer style usage of the
+    contiguous cache.  Indexes can be made valid again by calling
+    normalizeIndexs().
+
+    \sa normalizeIndexes(), append(), prepend()
+*/
+
 /*! \fn void QContiguousCache::dump() const
 
     \internal
diff --git a/src/corelib/tools/qcontiguouscache.h b/src/corelib/tools/qcontiguouscache.h
index 5250a79..5cd1582 100644
--- a/src/corelib/tools/qcontiguouscache.h
+++ b/src/corelib/tools/qcontiguouscache.h
@@ -43,6 +43,7 @@
 #define QCONTIGUOUSCACHE_H
 
 #include <QtCore/qatomic.h>
+#include <limits.h>
 
 QT_BEGIN_HEADER
 
@@ -50,7 +51,7 @@ QT_BEGIN_NAMESPACE
 
 QT_MODULE(Core)
 
-struct QContiguousCacheData
+struct Q_CORE_EXPORT QContiguousCacheData
 {
     QBasicAtomicInt ref;
     int alloc;
@@ -75,8 +76,6 @@ struct QContiguousCacheTypedData
     T array[1];
 };
 
-class QContiguousCacheDevice;
-
 template<typename T>
 class QContiguousCache {
     typedef QContiguousCacheTypedData<T> Data;
@@ -128,13 +127,17 @@ public:
     void removeLast();
     T takeLast();
 
+    inline bool areIndexesValid() const
+    { return d->offset >= 0 && d->offset < INT_MAX - d->count && (d->offset % d->alloc) == d->start; }
+
+    inline void normalizeIndexes() { d->offset = d->start; }
     // debug
     void dump() const { p->dump(); }
 private:
     void detach_helper();
 
-    QContiguousCacheData *malloc(int alloc);
-    void free(Data *d);
+    QContiguousCacheData *malloc(int aalloc);
+    void free(Data *x);
     int sizeOfTypedData() {
         // this is more or less the same as sizeof(Data), except that it doesn't
         // count the padding at the end
@@ -189,10 +192,6 @@ void QContiguousCache<T>::setCapacity(int asize)
     x.d->count = qMin(d->count, asize);
     x.d->offset = d->offset + d->count - x.d->count;
     x.d->start = x.d->offset % x.d->alloc;
-    /*  deep copy -
-        slow way now, get unit test working, then
-        improve performance if need be. (e.g. memcpy)
-    */
     T *dest = x.d->array + (x.d->start + x.d->count-1) % x.d->alloc;
     T *src = d->array + (d->start + d->count-1) % d->alloc;
     int count = x.d->count;
@@ -249,11 +248,11 @@ inline QContiguousCacheData *QContiguousCache<T>::malloc(int aalloc)
 }
 
 template <typename T>
-QContiguousCache<T>::QContiguousCache(int asize)
+QContiguousCache<T>::QContiguousCache(int capacity)
 {
-    p = malloc(asize);
+    p = malloc(capacity);
     d->ref = 1;
-    d->alloc = asize;
+    d->alloc = capacity;
     d->count = d->start = d->offset = 0;
     d->sharable = true;
 }
@@ -302,7 +301,6 @@ void QContiguousCache<T>::free(Data *x)
     }
     qFree(x);
 }
-
 template <typename T>
 void QContiguousCache<T>::append(const T &value)
 {
@@ -349,6 +347,7 @@ void QContiguousCache<T>::prepend(const T &value)
 template<typename T>
 void QContiguousCache<T>::insert(int pos, const T &value)
 {
+    Q_ASSERT_X(pos >= 0 && pos < INT_MAX, "QContiguousCache<T>::insert", "index out of range");
     detach();
     if (containsIndex(pos)) {
         if(QTypeInfo<T>::isComplex)
@@ -362,8 +361,8 @@ void QContiguousCache<T>::insert(int pos, const T &value)
     else {
         // we don't leave gaps.
         clear();
-        d->offset = d->start = pos;
-        d->start %= d->alloc;
+        d->offset = pos;
+        d->start = pos % d->alloc;
         d->count = 1;
         if (QTypeInfo<T>::isComplex)
             new (d->array + d->start) T(value);
@@ -378,9 +377,8 @@ inline const T &QContiguousCache<T>::at(int pos) const
 template <typename T>
 inline const T &QContiguousCache<T>::operator[](int pos) const
 { Q_ASSERT_X(pos >= d->offset && pos - d->offset < d->count, "QContiguousCache<T>::at", "index out of range"); return d->array[pos % d->alloc]; }
-template <typename T>
 
-// can use the non-inline one to modify the index range.
+template <typename T>
 inline T &QContiguousCache<T>::operator[](int pos)
 {
     detach();