From 438df7ecac73998a9f98cb417f2e03c3f80a9991 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 20 Apr 2008 03:07:53 +0000 Subject: [PATCH] * 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 --- .../network/protocols/unix/UnixFifo.cpp | 147 ++++++++++++++++++ .../kernel/network/protocols/unix/UnixFifo.h | 14 ++ 2 files changed, 161 insertions(+) diff --git a/src/add-ons/kernel/network/protocols/unix/UnixFifo.cpp b/src/add-ons/kernel/network/protocols/unix/UnixFifo.cpp index d2d694178f..43f65944ee 100644 --- a/src/add-ons/kernel/network/protocols/unix/UnixFifo.cpp +++ b/src/add-ons/kernel/network/protocols/unix/UnixFifo.cpp @@ -5,6 +5,8 @@ #include "UnixFifo.h" +#include + #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 _(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 _(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 - diff --git a/src/add-ons/kernel/network/protocols/unix/UnixFifo.h b/src/add-ons/kernel/network/protocols/unix/UnixFifo.h index 982d9e8400..fe0fc6d1ec 100644 --- a/src/add-ons/kernel/network/protocols/unix/UnixFifo.h +++ b/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 > 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 };