Do some work on BufferArray to make it a bit less naive about

chunking data.  Remove the stateful use of a seek pointer so
that shared read is possible (though maybe not interesting).

4 files changed, 132 insertions, 150 deletions

diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 791ee5f860..196011f953 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -340,7 +340,6 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 			long data_size(0);
 			if (mReqBody)
 			{
-				mReqBody->seek(0);
 				data_size = mReqBody->size();
 			}
 			curl_easy_setopt(mCurlHandle, CURLOPT_POSTFIELDS, static_cast<void *>(NULL));
@@ -356,7 +355,6 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 			long data_size(0);
 			if (mReqBody)
 			{
-				mReqBody->seek(0);
 				data_size = mReqBody->size();
 			}
 			curl_easy_setopt(mCurlHandle, CURLOPT_INFILESIZE, data_size);
@@ -423,10 +421,8 @@ size_t HttpOpRequest::writeCallback(void * data, size_t size, size_t nmemb, void
 		op->mReplyBody = new BufferArray();
 	}
 	const size_t req_size(size * nmemb);
-	char * lump(op->mReplyBody->appendBufferAlloc(req_size));
-	memcpy(lump, data, req_size);
-
-	return req_size;
+	const size_t write_size(op->mReplyBody->append(static_cast<char *>(data), req_size));
+	return write_size;
 }
 
 		
@@ -450,9 +446,9 @@ size_t HttpOpRequest::readCallback(void * data, size_t size, size_t nmemb, void
 	}
 
 	const size_t do_size((std::min)(req_size, body_size - op->mCurlBodyPos));
-	op->mReqBody->read(static_cast<char *>(data), do_size);
-	op->mCurlBodyPos += do_size;
-	return do_size;
+	const size_t read_size(op->mReqBody->read(op->mCurlBodyPos, static_cast<char *>(data), do_size));
+	op->mCurlBodyPos += read_size;
+	return read_size;
 }
 
 		
diff --git a/indra/llcorehttp/bufferarray.cpp b/indra/llcorehttp/bufferarray.cpp
index 4c20350b13..c13b1a3540 100644
--- a/indra/llcorehttp/bufferarray.cpp
+++ b/indra/llcorehttp/bufferarray.cpp
@@ -27,6 +27,18 @@
 #include "bufferarray.h"
 
 
+// BufferArray is a list of chunks, each a BufferArray::Block, of contiguous
+// data presented as a single array.  Chunks are at least BufferArray::BLOCK_ALLOC_SIZE
+// in length and can be larger.  Any chunk may be partially filled or even
+// empty.
+//
+// The BufferArray itself is sharable as a RefCounted entity.  As shared
+// reads don't work with the concept of a current position/seek value,
+// none is kept with the object.  Instead, the read and write operations
+// all take position arguments.  Single write/shared read isn't supported
+// directly and any such attempts have to be serialized outside of this
+// implementation.
+
 namespace LLCore
 {
 
@@ -58,7 +70,8 @@ public:
 	static Block * alloc(size_t len);
 
 public:
-	size_t mLen;
+	size_t mUsed;
+	size_t mAlloced;
 
 	// *NOTE:  Must be last member of the object.  We'll
 	// overallocate as requested via operator new and index
@@ -74,7 +87,6 @@ public:
 
 BufferArray::BufferArray()
 	: LLCoreInt::RefCounted(true),
-	  mPos(0),
 	  mLen(0)
 {}
 
@@ -94,19 +106,45 @@ BufferArray::~BufferArray()
 
 size_t BufferArray::append(const char * src, size_t len)
 {
-	if (len)
+	const size_t ret(len);
+	
+	// First, try to copy into the last block
+	if (len && ! mBlocks.empty())
+	{
+		Block & last(*mBlocks.back());
+		if (last.mUsed < last.mAlloced)
+		{
+			// Some will fit...
+			const size_t copy_len((std::min)(len, (last.mAlloced - last.mUsed)));
+
+			memcpy(&last.mData[last.mUsed], src, copy_len);
+			last.mUsed += copy_len;
+			llassert_always(last.mUsed <= last.mAlloced);
+			mLen += copy_len;
+			src += copy_len;
+			len -= copy_len;
+		}
+	}
+
+	// Then get new blocks as needed
+	while (len)
 	{
+		const size_t copy_len((std::min)(len, BLOCK_ALLOC_SIZE));
+		
 		if (mBlocks.size() >= mBlocks.capacity())
 		{
 			mBlocks.reserve(mBlocks.size() + 5);
 		}
-		Block * block = Block::alloc(len);
-		memcpy(block->mData, src, len);
+		Block * block = Block::alloc(BLOCK_ALLOC_SIZE);
+		memcpy(block->mData, src, copy_len);
+		block->mUsed = copy_len;
+		llassert_always(block->mUsed <= block->mAlloced);
 		mBlocks.push_back(block);
-		mLen += len;
-		mPos = mLen;
+		mLen += copy_len;
+		src += copy_len;
+		len -= copy_len;
 	}
-	return len;
+	return ret;
 }
 
 
@@ -117,41 +155,33 @@ char * BufferArray::appendBufferAlloc(size_t len)
 	{
 		mBlocks.reserve(mBlocks.size() + 5);
 	}
-	Block * block = Block::alloc(len);
+	Block * block = Block::alloc((std::max)(BLOCK_ALLOC_SIZE, len));
+	block->mUsed = len;
 	mBlocks.push_back(block);
 	mLen += len;
-	mPos = mLen;
 	return block->mData;
 }
 
 
-size_t BufferArray::seek(size_t pos)
+size_t BufferArray::read(size_t pos, char * dst, size_t len)
 {
-	if (pos > mLen)
-		pos = mLen;
-	mPos = pos;
-	return mPos;
-}
-
-	
-size_t BufferArray::read(char * dst, size_t len)
-{
-	size_t result(0), offset(0);
-	size_t len_limit(mLen - mPos);
+	if (pos >= mLen)
+		return 0;
+	size_t len_limit(mLen - pos);
 	len = std::min(len, len_limit);
-
-	if (mPos >= mLen || 0 == len)
+	if (0 == len)
 		return 0;
 	
+	size_t result(0), offset(0);
 	const int block_limit(mBlocks.size());
-	int block_start(findBlock(mPos, &offset));
+	int block_start(findBlock(pos, &offset));
 	if (block_start < 0)
 		return 0;
 
 	do
 	{
 		Block & block(*mBlocks[block_start]);
-		size_t block_limit(block.mLen - offset);
+		size_t block_limit(block.mUsed - offset);
 		size_t block_len(std::min(block_limit, len));
 		
 		memcpy(dst, &block.mData[offset], block_len);
@@ -163,19 +193,18 @@ size_t BufferArray::read(char * dst, size_t len)
 	}
 	while (len && block_start < block_limit);
 
-	mPos += result;
 	return result;
 }
 
 
-size_t BufferArray::write(const char * src, size_t len)
+size_t BufferArray::write(size_t pos, const char * src, size_t len)
 {
-	size_t result(0), offset(0);
-	if (mPos > mLen || 0 == len)
+	if (pos > mLen || 0 == len)
 		return 0;
 	
+	size_t result(0), offset(0);
 	const int block_limit(mBlocks.size());
-	int block_start(findBlock(mPos, &offset));
+	int block_start(findBlock(pos, &offset));
 
 	if (block_start >= 0)
 	{
@@ -184,20 +213,39 @@ size_t BufferArray::write(const char * src, size_t len)
 		do
 		{
 			Block & block(*mBlocks[block_start]);
-			size_t block_limit(block.mLen - offset);
+			size_t block_limit(block.mUsed - offset);
 			size_t block_len(std::min(block_limit, len));
 		
 			memcpy(&block.mData[offset], src, block_len);
 			result += block_len;
-			len -= block_len;
 			src += block_len;
+			len -= block_len;
 			offset = 0;
 			++block_start;
 		}
 		while (len && block_start < block_limit);
 	}
-	mPos += result;
 
+	// Something left, see if it will fit in the free
+	// space of the last block.
+	if (len && ! mBlocks.empty())
+	{
+		Block & last(*mBlocks.back());
+		if (last.mUsed < last.mAlloced)
+		{
+			// Some will fit...
+			const size_t copy_len((std::min)(len, (last.mAlloced - last.mUsed)));
+
+			memcpy(&last.mData[last.mUsed], src, copy_len);
+			last.mUsed += copy_len;
+			result += copy_len;
+			llassert_always(last.mUsed <= last.mAlloced);
+			mLen += copy_len;
+			src += copy_len;
+			len -= copy_len;
+		}
+	}
+	
 	if (len)
 	{
 		// Some or all of the remaining write data will
@@ -218,12 +266,12 @@ int BufferArray::findBlock(size_t pos, size_t * ret_offset)
 	const int block_limit(mBlocks.size());
 	for (int i(0); i < block_limit; ++i)
 	{
-		if (pos < mBlocks[i]->mLen)
+		if (pos < mBlocks[i]->mUsed)
 		{
 			*ret_offset = pos;
 			return i;
 		}
-		pos -= mBlocks[i]->mLen;
+		pos -= mBlocks[i]->mUsed;
 	}
 
 	// Shouldn't get here but...
@@ -237,7 +285,8 @@ int BufferArray::findBlock(size_t pos, size_t * ret_offset)
 
 
 BufferArray::Block::Block(size_t len)
-	: mLen(len)
+	: mUsed(0),
+	  mAlloced(len)
 {
 	memset(mData, 0, len);
 }
@@ -245,7 +294,8 @@ BufferArray::Block::Block(size_t len)
 
 BufferArray::Block::~Block()
 {
-	mLen = 0;
+	mUsed = 0;
+	mAlloced = 0;
 }
 
 
diff --git a/indra/llcorehttp/bufferarray.h b/indra/llcorehttp/bufferarray.h
index b26ad1b297..9ccd85d4f8 100644
--- a/indra/llcorehttp/bufferarray.h
+++ b/indra/llcorehttp/bufferarray.h
@@ -73,6 +73,9 @@ private:
 	void operator=(const BufferArray &);		// Not defined
 
 public:
+	// Internal magic number, may be used by unit tests.
+	static const size_t BLOCK_ALLOC_SIZE = 1504;
+	
 	/// Appends the indicated data to the BufferArray
 	/// modifying current position and total size.  New
 	/// position is one beyond the final byte of the buffer.
@@ -96,24 +99,16 @@ public:
 			return mLen;
 		}
 
-	/// Set the current position for subsequent read and
-	/// write operations.  'pos' values before the beginning
-	/// or greater than the size of the buffer are coerced
-	/// to a value within the buffer.
-	///
-	/// @return			Actual current position after seek.
-	size_t seek(size_t pos);
-
-	/// Copies data from the current position in the instance
+	/// Copies data from the given position in the instance
 	/// to the caller's buffer.  Will return a short count of
 	/// bytes copied if the 'len' extends beyond the data.
-	size_t read(char * dst, size_t len);
+	size_t read(size_t pos, char * dst, size_t len);
 
 	/// Copies data from the caller's buffer to the instance
 	/// at the current position.  May overwrite existing data,
 	/// append data when current position is equal to the
 	/// size of the instance or do a mix of both.
-	size_t write(const char * src, size_t len);
+	size_t write(size_t pos, const char * src, size_t len);
 	
 protected:
 	int findBlock(size_t pos, size_t * ret_offset);
@@ -123,46 +118,10 @@ protected:
 	typedef std::vector<Block *> container_t;
 
 	container_t			mBlocks;
-	size_t				mPos;
 	size_t				mLen;
 };  // end class BufferArray
 
 
-#if 0
-
-// Conceptual for now.  Another possibility is going with
-// something like Boost::asio's buffers interface.  They're
-// trying to achieve the same thing above and below....
-
-class BufferStream : public std::streambuf
-{
-public:
-	BufferStream(BufferArray * buffer);
-	virtual ~BufferStream();
-
-private:
-	BufferStream(const BufferStream &);			// Not defined
-	void operator=(const BufferStream &);		// Not defined
-
-public:
-	// Types
-	typedef std::streambuf::pos_type pos_type;
-	typedef std::streambuf::off_type off_type;
-
-	virtual int underflow();
-
-	virtual int overflow(int c);
-
-	virtual int sync();
-
-	virtual pos_type seekoff(off_type off, std::ios::seekdir way, std::ios::openmode which);
-
-protected:
-	BufferArray *		mBufferArray;
-};  // end class BufferStream
-
-#endif // 0
-
 }  // end namespace LLCore
 
 #endif	// _LLCORE_BUFFER_ARRAY_H_
diff --git a/indra/llcorehttp/tests/test_bufferarray.hpp b/indra/llcorehttp/tests/test_bufferarray.hpp
index ecbb5ef250..2ad9391d1c 100644
--- a/indra/llcorehttp/tests/test_bufferarray.hpp
+++ b/indra/llcorehttp/tests/test_bufferarray.hpp
@@ -67,7 +67,7 @@ void BufferArrayTestObjectType::test<1>()
 
 	// Try to read
 	char buffer[20];
-	size_t read_len(ba->read(buffer, sizeof(buffer)));
+	size_t read_len(ba->read(0, buffer, sizeof(buffer)));
 	ensure("Read returns empty", 0 == read_len);
 	
 	// release the implicit reference, causing the object to be released
@@ -92,16 +92,13 @@ void BufferArrayTestObjectType::test<2>()
 	char str1[] = "abcdefghij";
  	char buffer[256];
 	
-	size_t len = ba->write(str1, strlen(str1));
+	size_t len = ba->write(0, str1, strlen(str1));
 	ensure("Wrote length correct", strlen(str1) == len);
 	ensure("Recorded size correct", strlen(str1) == ba->size());
 
 	// read some data back
-	len = ba->seek(2);
-	ensure("Seek worked", 2 == len);
-
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, 2);
+	len = ba->read(2, buffer, 2);
 	ensure("Read length correct", 2 == len);
 	ensure("Read content correct", 'c' == buffer[0] && 'd' == buffer[1]);
 	ensure("Read didn't overwrite", 'X' == buffer[2]);
@@ -130,32 +127,26 @@ void BufferArrayTestObjectType::test<3>()
 	size_t str1_len(strlen(str1));
  	char buffer[256];
 	
-	size_t len = ba->write(str1, str1_len);
+	size_t len = ba->write(0, str1, str1_len);
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", str1_len == ba->size());
 
 	// again...
-	len = ba->write(str1, strlen(str1));
+	len = ba->write(str1_len, str1, strlen(str1));
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", (2 * str1_len) == ba->size());
 
 	// read some data back
-	len = ba->seek(8);
-	ensure("Seek worked", 8 == len);
-
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, 4);
+	len = ba->read(8, buffer, 4);
 	ensure("Read length correct", 4 == len);
 	ensure("Read content correct", 'i' == buffer[0] && 'j' == buffer[1]);
 	ensure("Read content correct", 'a' == buffer[2] && 'b' == buffer[3]);
 	ensure("Read didn't overwrite", 'X' == buffer[4]);
 
 	// Read whole thing
-	len = ba->seek(0);
-	ensure("Seek worked (2)", 0 == len);
-
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, sizeof(buffer));
+	len = ba->read(0, buffer, sizeof(buffer));
 	ensure("Read length correct", (2 * str1_len) == len);
 	ensure("Read content correct (3)", 0 == strncmp(buffer, str1, str1_len));
 	ensure("Read content correct (4)", 0 == strncmp(&buffer[str1_len], str1, str1_len));
@@ -185,33 +176,29 @@ void BufferArrayTestObjectType::test<4>()
 	char str2[] = "ABCDEFGHIJ";
  	char buffer[256];
 	
-	size_t len = ba->write(str1, str1_len);
+	size_t len = ba->write(0, str1, str1_len);
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", str1_len == ba->size());
 
 	// again...
-	len = ba->write(str1, strlen(str1));
+	len = ba->write(str1_len, str1, strlen(str1));
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", (2 * str1_len) == ba->size());
 
 	// reposition and overwrite
-	len = ba->seek(8);
-	ensure("Seek worked", 8 == len);
-	len = ba->write(str2, 4);
+	len = ba->write(8, str2, 4);
 	ensure("Overwrite length correct", 4 == len);
 
-	// Leave position and read verifying content
+	// Leave position and read verifying content (stale really from seek() days)
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, 4);
+	len = ba->read(12, buffer, 4);
 	ensure("Read length correct", 4 == len);
 	ensure("Read content correct", 'c' == buffer[0] && 'd' == buffer[1]);
 	ensure("Read content correct.2", 'e' == buffer[2] && 'f' == buffer[3]);
 	ensure("Read didn't overwrite", 'X' == buffer[4]);
 
 	// reposition and check
-	len = ba->seek(6);
-	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, 8);
+	len = ba->read(6, buffer, 8);
 	ensure("Read length correct.2", 8 == len);
 	ensure("Read content correct.3", 'g' == buffer[0] && 'h' == buffer[1]);
 	ensure("Read content correct.4", 'A' == buffer[2] && 'B' == buffer[3]);
@@ -242,21 +229,18 @@ void BufferArrayTestObjectType::test<5>()
 	size_t str1_len(strlen(str1));
  	char buffer[256];
 	
-	size_t len = ba->write(str1, str1_len);
+	size_t len = ba->write(0, str1, str1_len);
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", str1_len == ba->size());
 
 	// again...
-	len = ba->write(str1, strlen(str1));
+	len = ba->write(str1_len, str1, str1_len);
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", (2 * str1_len) == ba->size());
 
 	// read some data back
-	len = ba->seek(8);
-	ensure("Seek worked", 8 == len);
-
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, 4);
+	len = ba->read(8, buffer, 4);
 	ensure("Read length correct", 4 == len);
 	ensure("Read content correct", 'i' == buffer[0] && 'j' == buffer[1]);
 	ensure("Read content correct.2", 'a' == buffer[2] && 'b' == buffer[3]);
@@ -264,7 +248,7 @@ void BufferArrayTestObjectType::test<5>()
 
 	// Read some more without repositioning
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, sizeof(buffer));
+	len = ba->read(12, buffer, sizeof(buffer));
 	ensure("Read length correct", (str1_len - 2) == len);
 	ensure("Read content correct.3", 0 == strncmp(buffer, str1+2, str1_len-2));
 	ensure("Read didn't overwrite.2", 'X' == buffer[str1_len-1]);
@@ -294,31 +278,27 @@ void BufferArrayTestObjectType::test<6>()
 	size_t str2_len(strlen(str2));
  	char buffer[256];
 	
-	size_t len = ba->write(str1, str1_len);
+	size_t len = ba->write(0, str1, str1_len);
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", str1_len == ba->size());
 
 	// again...
-	len = ba->write(str1, strlen(str1));
+	len = ba->write(str1_len, str1, strlen(str1));
 	ensure("Wrote length correct", str1_len == len);
 	ensure("Recorded size correct", (2 * str1_len) == ba->size());
 
 	// reposition and overwrite
-	len = ba->seek(8);
-	ensure("Seek worked", 8 == len);
-	len = ba->write(str2, str2_len);
+	len = ba->write(8, str2, str2_len);
 	ensure("Overwrite length correct", str2_len == len);
 
 	// Leave position and read verifying content
 	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, 0);
+	len = ba->read(8 + str2_len, buffer, 0);
 	ensure("Read length correct", 0 == len);
 	ensure("Read didn't overwrite", 'X' == buffer[0]);
 
 	// reposition and check
-	len = ba->seek(0);
-	memset(buffer, 'X', sizeof(buffer));
-	len = ba->read(buffer, sizeof(buffer));
+	len = ba->read(0, buffer, sizeof(buffer));
 	ensure("Read length correct.2", (str1_len + str2_len - 2) == len);
 	ensure("Read content correct", 0 == strncmp(buffer, str1, str1_len-2));
 	ensure("Read content correct.2", 0 == strncmp(buffer+str1_len-2, str2, str2_len));
@@ -350,18 +330,17 @@ void BufferArrayTestObjectType::test<7>()
  	char buffer[256];
 
 	// 2x str1
-	size_t len = ba->write(str1, str1_len);
-	len = ba->write(str1, strlen(str1));
+	size_t len = ba->write(0, str1, str1_len);
+	len = ba->write(str1_len, str1, str1_len);
 
 	// reposition and overwrite
-	len = ba->seek(6);
-	len = ba->write(str2, 2);
+	len = ba->write(6, str2, 2);
 	ensure("Overwrite length correct", 2 == len);
 
-	len = ba->write(str2, 2);
+	len = ba->write(8, str2, 2);
 	ensure("Overwrite length correct.2", 2 == len);
 
-	len = ba->write(str2, 2);
+	len = ba->write(10, str2, 2);
 	ensure("Overwrite length correct.3", 2 == len);
 
 	// append some data
@@ -373,13 +352,12 @@ void BufferArrayTestObjectType::test<7>()
 	memcpy(out_buf, str1, str1_len);
 
 	// And some final writes
-	len = ba->write(str2, 2);
+	len = ba->write(3 * str1_len + str2_len, str2, 2);
 	ensure("Write length correct.2", 2 == len);
 	
 	// Check contents
 	memset(buffer, 'X', sizeof(buffer));
-	ba->seek(0);
-	len = ba->read(buffer, sizeof(buffer));
+	len = ba->read(0, buffer, sizeof(buffer));
 	ensure("Final buffer length correct", (3 * str1_len + str2_len + 2) == len);
 	ensure("Read content correct", 0 == strncmp(buffer, str1, 6));
 	ensure("Read content correct.2", 0 == strncmp(buffer + 6, str2, 2));
@@ -417,8 +395,8 @@ void BufferArrayTestObjectType::test<8>()
  	char buffer[256];
 
 	// 2x str1
-	size_t len = ba->write(str1, str1_len);
-	len = ba->write(str1, strlen(str1));
+	size_t len = ba->write(0, str1, str1_len);
+	len = ba->write(str1_len, str1, str1_len);
 
 	// zero-length allocate (we allow this with a valid pointer returned)
 	char * out_buf(ba->appendBufferAlloc(0));
@@ -430,12 +408,11 @@ void BufferArrayTestObjectType::test<8>()
 	ensure("Two zero-length appendBufferAlloc buffers distinct", out_buf != out_buf2);
 
 	// And some final writes
-	len = ba->write(str2, str2_len);
+	len = ba->write(2 * str1_len, str2, str2_len);
 	
 	// Check contents
 	memset(buffer, 'X', sizeof(buffer));
-	ba->seek(0);
-	len = ba->read(buffer, sizeof(buffer));
+	len = ba->read(0, buffer, sizeof(buffer));
 	ensure("Final buffer length correct", (2 * str1_len + str2_len) == len);
 	ensure("Read content correct.1", 0 == strncmp(buffer, str1, str1_len));
 	ensure("Read content correct.2", 0 == strncmp(buffer + str1_len, str1, str1_len));