llmessage and llcorehttp: fix the five bugs their tests' follow-up found

indra/llcorehttp/_httpinternal.h

@@ -131,8 +131,12 @@ constexpr int HTTP_RETRY_COUNT_DEFAULT = 5;
 constexpr int HTTP_RETRY_COUNT_MIN = 0;
 constexpr int HTTP_RETRY_COUNT_MAX = 100;
 constexpr HttpTime HTTP_RETRY_BACKOFF_MIN_DEFAULT = 1000000UL; // 1 sec
-constexpr HttpTime HTTP_RETRY_BACKOFF_MAX_DEFAULT = 50000006UL; // 5 sec
+constexpr HttpTime HTTP_RETRY_BACKOFF_MAX_DEFAULT = 5000000UL; // 5 sec
 constexpr HttpTime HTTP_RETRY_BACKOFF_MAX = 20000000UL; // 20 sec
+static_assert(HTTP_RETRY_BACKOFF_MIN_DEFAULT <= HTTP_RETRY_BACKOFF_MAX_DEFAULT,
+              "the default backoff range must not be empty");
+static_assert(HTTP_RETRY_BACKOFF_MAX_DEFAULT <= HTTP_RETRY_BACKOFF_MAX,
+              "the default backoff must lie within the limit options are clamped to");
 
 constexpr int HTTP_REDIRECTS_DEFAULT = 10;
 

indra/llcorehttp/bufferstream.cpp

@@ -185,83 +185,95 @@ std::streamsize BufferArrayStreamBuf::xsputn(const char * src, std::streamsize c
 }
 
 
-std::streampos BufferArrayStreamBuf::seekoff(std::streamoff off,
-                                             std::ios_base::seekdir way,
-                                             std::ios_base::openmode which)
+namespace
 {
-    std::streampos ret(-1);
-
-    if (! mBufferArray)
+    // Where a seek of off from way lands in size bytes, from cur for a seek
+    // from the current position. Before the start is -1, and past the end
+    // is the end.
+    std::streamoff seek_target(std::streamoff off, std::ios_base::seekdir way, size_t cur, size_t size)
     {
-        return ret;
-    }
-
-    if (std::ios_base::in == which)
-    {
-        size_t pos(0);
-
+        std::streamoff base(0);
         switch (way)
         {
         case std::ios_base::beg:
-            pos = off;
             break;
 
         case std::ios_base::cur:
-            pos = mReadCurPos += off;
+            base = std::streamoff(cur);
             break;
 
         case std::ios_base::end:
-            pos = mBufferArray->mLen - off;
+            base = std::streamoff(size);
             break;
 
         default:
-            return ret;
+            return -1;
         }
 
-        if (pos >= mBufferArray->size())
+        const std::streamoff target(base + off);
+        if (target < 0)
         {
-            pos = (std::max)(size_t(0), mBufferArray->size() - 1);
+            return -1;
         }
-        size_t ba_offset(0);
-        int block(mBufferArray->findBlock(pos, &ba_offset));
-        if (block < 0)
-            return ret;
-        const char * start(NULL), * end(NULL);
-        if (! mBufferArray->getBlockStartEnd(block, &start, &end))
-            return ret;
-        mReadCurBlock = block;
-        mReadBegin = start;
-        mReadCur = start + ba_offset;
-        mReadEnd = end;
-        ret = mReadCurPos = pos;
+        return (std::min)(target, std::streamoff(size));
     }
-    else if (std::ios_base::out == which)
-    {
-        size_t pos(0);
+}
 
-        switch (way)
-        {
-        case std::ios_base::beg:
-            pos = off;
-            break;
 
-        case std::ios_base::cur:
-            pos = mWriteCurPos += off;
-            break;
+std::streampos BufferArrayStreamBuf::seekoff(std::streamoff off,
+                                             std::ios_base::seekdir way,
+                                             std::ios_base::openmode which)
+{
+    std::streampos ret(-1);
 
-        case std::ios_base::end:
-            pos = mBufferArray->mLen - off;
-            break;
+    if (! mBufferArray)
+    {
+        return ret;
+    }
 
-        default:
+    if (std::ios_base::in == which)
+    {
+        const size_t size(mBufferArray->size());
+        const std::streamoff target(seek_target(off, way, mReadCurPos, size));
+        if (target < 0)
+        {
             return ret;
         }
+        const size_t pos(target);
 
-        if (pos > mBufferArray->size())
+        if (0 == size)
+        {
+            // Nothing to read: where a new stream buffer starts
+            mReadCurBlock = -1;
+            mReadBegin = mReadCur = mReadEnd = NULL;
+        }
+        else
+        {
+            // The end lies just past the last byte, in that byte's block,
+            // so that a read there finds nothing and a putback finds the byte.
+            const bool at_end(pos == size);
+            size_t ba_offset(0);
+            int block(mBufferArray->findBlock(at_end ? pos - 1 : pos, &ba_offset));
+            if (block < 0)
+                return ret;
+            const char * start(NULL), * end(NULL);
+            if (! mBufferArray->getBlockStartEnd(block, &start, &end))
+                return ret;
+            mReadCurBlock = block;
+            mReadBegin = start;
+            mReadCur = start + ba_offset + (at_end ? 1 : 0);
+            mReadEnd = end;
+        }
+        ret = mReadCurPos = pos;
+    }
+    else if (std::ios_base::out == which)
+    {
+        const std::streamoff target(seek_target(off, way, mWriteCurPos, mBufferArray->size()));
+        if (target < 0)
         {
-            pos = mBufferArray->size();
+            return ret;
         }
-        ret = mWriteCurPos = pos;
+        ret = mWriteCurPos = size_t(target);
     }
 
     return ret;

indra/llcorehttp/tests/bufferstream_test.cpp

@@ -114,6 +114,21 @@ TEST_CASE("BufferStream Tests: BufferArrayStream holds a reference on an empty B
 }
 
 
+TEST_CASE("BufferStream Tests: BufferArrayStreamBuf over an empty BufferArray seeks to its start and reads nothing", "[BufferStream_Tests]")
+{
+    // create a new ref counted BufferArray with implicit reference
+    BufferArray * ba = new BufferArray;
+    BufferArrayStreamBuf bsb(ba);
+    ba->release();
+    ba = NULL;
+
+    CHECK(bsb.seekoff(0, std::ios_base::beg, std::ios_base::in) == std::streampos(0));
+    CHECK(bsb.seekoff(0, std::ios_base::end, std::ios_base::in) == std::streampos(0));
+    CHECK(tst_traits_t::eof() == bsb.underflow());
+    CHECK(tst_traits_t::eof() == bsb.pbackfail(tst_traits_t::eof()));
+}
+
+
 TEST_CASE("BufferStream Tests: BufferArrayStreamBuf reads and appends and seeks a BufferArray", "[BufferStream_Tests]")
 {
     // create a new ref counted BufferArray with implicit reference
@@ -152,11 +167,26 @@ TEST_CASE("BufferStream Tests: BufferArrayStreamBuf reads and appends and seeks
     CHECK(bsb->seekoff(4, std::ios_base::cur, std::ios_base::in) == std::streampos(4));
     UNSCOPED_INFO("showmanyc() picks up seekoff");
     CHECK(bsb->showmanyc() == (c_len + 5 - 4));
-    CHECK(bsb->seekoff(0, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 4));
-    // The seek to the end leaves the read position on the last character, the '!',
-    // so pbackfail() refuses the '!' and puts back the 'a' before it
-    CHECK(tst_traits_t::eof() == bsb->pbackfail('!'));
-    CHECK(bsb->pbackfail('a') == 'a');
+    UNSCOPED_INFO("the end is past the last character");
+    CHECK(bsb->seekoff(0, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 5));
+    CHECK(tst_traits_t::eof() == bsb->underflow());
+    CHECK(bsb->showmanyc() == 0);
+    // so the last character, the '!', is the one that puts back there
+    CHECK(tst_traits_t::eof() == bsb->pbackfail('a'));
+    CHECK(bsb->pbackfail('!') == '!');
+    UNSCOPED_INFO("an offset from the end counts back from it");
+    CHECK(bsb->seekoff(-2, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 3));
+    CHECK(bsb->underflow() == 'a');
+    UNSCOPED_INFO("past the end is the end");
+    CHECK(bsb->seekoff(1, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 5));
+    UNSCOPED_INFO("before the start fails and leaves the position");
+    CHECK(bsb->seekoff(-1, std::ios_base::beg, std::ios_base::in) == std::streampos(-1));
+    CHECK(bsb->seekoff(0, std::ios_base::cur, std::ios_base::in) == std::streampos(c_len + 5));
+    UNSCOPED_INFO("a write from the end overwrites the last character");
+    CHECK(bsb->seekoff(-1, std::ios_base::end, std::ios_base::out) == std::streampos(c_len + 4));
+    CHECK(bsb->xsputn("?", 1) == 1);
+    CHECK(bsb->seekoff(-1, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 4));
+    CHECK(bsb->underflow() == '?');
 
     // the stream buffer is not ref counted; deleting it drops the BufferArray's last reference
     delete bsb;

indra/llmessage/llbuffer.cpp

@@ -363,7 +363,7 @@ LLBufferArray::segment_iterator_t LLBufferArray::splitAfter(U8* address)
     // We have the location and the segment.
     U8* base = (*it).data();
     S32 size = (*it).size();
-    if(address == (base + size))
+    if(address == (base + size - 1))
     {
         // No need to split, since this is the last byte of the
         // segment. We do not want to have zero length segments, since

indra/llmessage/llbufferstream.cpp

@@ -212,15 +212,17 @@ int LLBufferStreamBuf::sync()
     address = mBuffer->seek(mChannels.out(), address, -1);
     if(address)
     {
-        LLBufferArray::segment_iterator_t it;
-        it = mBuffer->splitAfter(address);
         LLBufferArray::segment_iterator_t end = mBuffer->endSegment();
+        LLBufferArray::segment_iterator_t it = mBuffer->getSegment(address);
         if(it != end)
         {
-            ++it;
-            if(it != end)
+            // Give back what the put area did not use. A write that
+            // filled its segment left nothing over, and the segment
+            // after that one is not the put area's to erase.
+            if(address != ((*it).data() + (*it).size() - 1))
             {
-                mBuffer->eraseSegment(it);
+                it = mBuffer->splitAfter(address);
+                mBuffer->eraseSegment(++it);
             }
             return_value = 0;
         }