* Riddled UnixBufferQueue class with debug code.

* Implemented temporary work-around for net_buffer append_cloned(),
  which doesn't seem to work right in combination with remove_header().
  Or maybe I'm just misunderstood how it is supposed to be used. Anyway,
  this fixed invalid data in the stream when buffers were split by a
  read.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25070 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/network/protocols/unix/UnixFifo.cpp

@@ -5,6 +5,8 @@
 
 #include "UnixFifo.h"
 
+#include <AutoDeleter.h>
+
 #include "unix.h"
 
 
@@ -13,11 +15,28 @@
 #include "UnixDebug.h"
 
 
+#if TRACE_BUFFER_QUEUE
+#	define TRACEBQ(x...)	ktrace_printf(x)
+#	define TRACEBQ_ONLY(x)	x
+#else
+#	define TRACEBQ(x...)	do {} while (false)
+#	define TRACEBQ_ONLY(x)
+#endif
+
+
 UnixBufferQueue::UnixBufferQueue(size_t capacity)
 	:
 	fSize(0),
 	fCapacity(capacity)
+#if TRACE_BUFFER_QUEUE
+	, fWritten(0)
+	, fRead(0)
+#endif
 {
+	TRACEBQ_ONLY(
+		fParanoiaCheckBuffer = (uint8*)malloc(UNIX_FIFO_MAXIMAL_CAPACITY);
+		fParanoiaCheckBuffer2 = (uint8*)malloc(UNIX_FIFO_MAXIMAL_CAPACITY);
+	)
 }
 
 
@@ -25,6 +44,11 @@ UnixBufferQueue::~UnixBufferQueue()
 {
 	while (net_buffer* buffer = fBuffers.RemoveHead())
 		gBufferModule->free(buffer);
+
+	TRACEBQ_ONLY(
+		free(fParanoiaCheckBuffer);
+		free(fParanoiaCheckBuffer2);
+	)
 }
 
 
@@ -34,23 +58,53 @@ UnixBufferQueue::Read(size_t size, net_buffer** _buffer)
 	if (size > fSize)
 		size = fSize;
 
+	TRACEBQ("unix: UnixBufferQueue::Read(%lu): fSize: %lu, fRead: %lld, "
+		"fWritten: %lld", size, fSize, fRead, fWritten);
+
+	TRACEBQ_ONLY(
+		MethodDeleter<UnixBufferQueue> _(this, &UnixBufferQueue::PostReadWrite);
+	)
+
 	if (size == 0)
 		return B_BAD_VALUE;
 
+TRACEBQ_ONLY(
+	if (fParanoiaCheckBuffer) {
+		size_t bufferSize = 0;
+		for (BufferList::Iterator it = fBuffers.GetIterator();
+			net_buffer* buffer = it.Next();) {
+			size_t toWrite = min_c(buffer->size, size - bufferSize);
+			if (toWrite == 0)
+				break;
+
+			gBufferModule->read(buffer, 0,
+				fParanoiaCheckBuffer + bufferSize, toWrite);
+			bufferSize += toWrite;
+		}
+	}
+	*_buffer = NULL;
+)
+
 	// If the first buffer has the right size or is smaller, we can just
 	// dequeue it.
 	net_buffer* buffer = fBuffers.Head();
 	if (buffer->size <= size) {
 		fBuffers.RemoveHead();
 		fSize -= buffer->size;
+		TRACEBQ_ONLY(fRead += buffer->size);
 		*_buffer = buffer;
 
 		if (buffer->size == size)
+{
+TRACEBQ("unix:   read full buffer %p (%lu)", buffer, buffer->size);
+TRACEBQ_ONLY(ParanoiaReadCheck(*_buffer));
 			return B_OK;
+}
 
 		// buffer is too small
 
  		size_t bytesLeft = size - buffer->size;
+TRACEBQ("unix:   read short buffer %p (%lu/%lu)", buffer, size, buffer->size);
 
 		// Append from the following buffers, until we've read as much as we're
 		// supposed to.
@@ -58,18 +112,46 @@ UnixBufferQueue::Read(size_t size, net_buffer** _buffer)
 			net_buffer* nextBuffer = fBuffers.Head();
 			size_t toCopy = min_c(bytesLeft, nextBuffer->size);
 
+TRACEBQ("unix:   read next buffer %p (%lu/%lu)", nextBuffer, bytesLeft, nextBuffer->size);
+#if 0
 			if (gBufferModule->append_cloned(buffer, nextBuffer, 0, toCopy)
 					!= B_OK) {
 				// Too bad, but we've got some data, so we don't fail.
+TRACEBQ_ONLY(ParanoiaReadCheck(*_buffer));
 				return B_OK;
 			}
+#endif
+
+// TODO: Temporary work-around for the append_cloned() call above, which
+// doesn't seem to work right. Or maybe that's just in combination with the
+// remove_header() below.
+{
+	void* tmpBuffer = malloc(toCopy);
+	if (tmpBuffer == NULL)
+		return B_OK;
+	MemoryDeleter tmpBufferDeleter(tmpBuffer);
+
+	size_t offset = buffer->size;
+	if (gBufferModule->read(nextBuffer, 0, tmpBuffer, toCopy) != B_OK
+		|| gBufferModule->append_size(buffer, toCopy, NULL) != B_OK) {
+		return B_OK;
+	}
+
+	if (gBufferModule->write(buffer, offset, tmpBuffer, toCopy) != B_OK) {
+		gBufferModule->remove_trailer(buffer, toCopy);
+		return B_OK;
+	}
+}
 
 			// transfer the ancillary data
 			gBufferModule->transfer_ancillary_data(nextBuffer, buffer);
 
 			if (nextBuffer->size > toCopy) {
 				// remove the part we've copied
+//gBufferModule->read(nextBuffer, toCopy, fParanoiaCheckBuffer,
+//nextBuffer->size - toCopy);
 				gBufferModule->remove_header(nextBuffer, toCopy);
+//TRACEBQ_ONLY(ParanoiaReadCheck(nextBuffer));
 			} else {
 				// get rid of the buffer completely
 				fBuffers.RemoveHead();
@@ -78,8 +160,10 @@ UnixBufferQueue::Read(size_t size, net_buffer** _buffer)
 
 			bytesLeft -= toCopy;
 			fSize -= toCopy;
+			TRACEBQ_ONLY(fRead += toCopy);
 		}
 
+TRACEBQ_ONLY(ParanoiaReadCheck(*_buffer));
 		return B_OK;
 	}
 
@@ -100,11 +184,14 @@ UnixBufferQueue::Read(size_t size, net_buffer** _buffer)
 	gBufferModule->transfer_ancillary_data(buffer, newBuffer);
 
 	// remove the part we've copied
+TRACEBQ("unix:   read long buffer %p (%lu/%lu)", buffer, size, buffer->size);
 	gBufferModule->remove_header(buffer, size);
 
 	fSize -= size;
+	TRACEBQ_ONLY(fRead += size);
 	*_buffer = newBuffer;
 
+TRACEBQ_ONLY(ParanoiaReadCheck(*_buffer));
 	return B_OK;
 }
 
@@ -112,11 +199,19 @@ UnixBufferQueue::Read(size_t size, net_buffer** _buffer)
 status_t
 UnixBufferQueue::Write(net_buffer* buffer)
 {
+	TRACEBQ("unix: UnixBufferQueue::Write(%lu): fSize: %lu, fRead: %lld, "
+		"fWritten: %lld", buffer->size, fSize, fRead, fWritten);
+
+	TRACEBQ_ONLY(
+		MethodDeleter<UnixBufferQueue> _(this, &UnixBufferQueue::PostReadWrite);
+	)
+
 	if (buffer->size > Writable())
 		return ENOBUFS;
 
 	fBuffers.Add(buffer);
 	fSize += buffer->size;
+	TRACEBQ_ONLY(fWritten += buffer->size);
 
 	return B_OK;
 }
@@ -129,6 +224,58 @@ UnixBufferQueue::SetCapacity(size_t capacity)
 }
 
 
+#if TRACE_BUFFER_QUEUE
+
+void
+UnixBufferQueue::ParanoiaReadCheck(net_buffer* buffer)
+{
+	if (!buffer || !fParanoiaCheckBuffer || !fParanoiaCheckBuffer2)
+		return;
+
+	gBufferModule->read(buffer, 0, fParanoiaCheckBuffer2, buffer->size);
+
+	if (memcmp(fParanoiaCheckBuffer, fParanoiaCheckBuffer2, buffer->size)
+			!= 0) {
+		// find offset of first difference
+		size_t i = 0;
+		for (; i < buffer->size; i++) {
+			if (fParanoiaCheckBuffer[i] != fParanoiaCheckBuffer2[i])
+				break;
+		}
+
+		panic("unix: UnixBufferQueue::ParanoiaReadCheck(): incorrect read! "
+			"offset of first difference: %lu", i);
+	}
+}
+
+
+void
+UnixBufferQueue::PostReadWrite()
+{
+	TRACEBQ("unix: post read/write: fSize: %lu, fRead: %lld, fWritten: %lld",
+		fSize, fRead, fWritten);
+
+	if (fWritten - fRead != fSize) {
+		panic("UnixBufferQueue::PostReadWrite(): fSize: %lu, fRead: %lld, "
+			"fWritten: %lld", fSize, fRead, fWritten);
+	}
+
+	// check buffer size sum
+	size_t bufferSize = 0;
+	for (BufferList::Iterator it = fBuffers.GetIterator();
+		 net_buffer* buffer = it.Next();) {
+		bufferSize += buffer->size;
+	}
+
+	if (bufferSize != fSize) {
+		panic("UnixBufferQueue::PostReadWrite(): fSize: %lu, bufferSize: %lu",
+			fSize, bufferSize);
+	}
+}
+
+#endif	// TRACE_BUFFER_QUEUE
+
+
 // #pragma mark -
 
 

src/add-ons/kernel/network/protocols/unix/UnixFifo.h

@@ -24,6 +24,9 @@
 #define UNIX_FIFO_MAXIMAL_CAPACITY	(128 * 1024)
 
 
+#define TRACE_BUFFER_QUEUE	1
+
+
 class UnixBufferQueue {
 public:
 	UnixBufferQueue(size_t capacity);
@@ -39,6 +42,11 @@ public:
 	size_t Capacity() const		{ return fCapacity; }
 	void SetCapacity(size_t capacity);
 
+#if TRACE_BUFFER_QUEUE
+	void ParanoiaReadCheck(net_buffer* buffer);
+	void PostReadWrite();
+#endif
+
 private:
 	typedef DoublyLinkedList<net_buffer, DoublyLinkedListCLink<net_buffer> >
 		BufferList;
@@ -46,6 +54,12 @@ private:
 	BufferList	fBuffers;
 	size_t		fSize;
 	size_t		fCapacity;
+#if TRACE_BUFFER_QUEUE
+	off_t		fWritten;
+	off_t		fRead;
+	uint8*		fParanoiaCheckBuffer;
+	uint8*		fParanoiaCheckBuffer2;
+#endif
 };