Bug fixes:

- Fixed bugs on CRollingBuffer and CShiftBuffer
  - Remove "virtual" from methods declarations
  - Correct validation checks on methods
  - Use memmove to shift data in ShiftBuffer
  - Initialise buffer in RollingBuffer
  - FindStr() -> Return start position of match
  - Fix incorrect params passed to Peek()/PeekCopy() in Pop()/PopCopy()
  - Fix endless lopp in RollingBuffer::Push()
This commit is contained in:
Charl Wentzel
2017-03-22 07:52:58 +02:00
parent b5763d7f50
commit 5b4e6822c7
3 changed files with 39 additions and 36 deletions

View File

@@ -26,8 +26,10 @@ CRollingBuffer::CRollingBuffer( int BufferSize ) : BufSize( BufferSize )
{ {
// Create Buffer // Create Buffer
Buffer = (char *)malloc( BufSize+1 ); Buffer = (char *)malloc( BufSize+1 );
memset( Buffer, 0, BufSize+1 );
// Create temp output buffer big enough to handle all data
OutBuffer = (char *)malloc( BufSize+1 ); OutBuffer = (char *)malloc( BufSize+1 );
OutBuffer[0] = 0;
} }
else { else {
// No buffers created // No buffers created
@@ -77,7 +79,7 @@ char CRollingBuffer::PeekChar( int Pos )
char CharValue = 0; char CharValue = 0;
// Validate buffer and Pos // Validate buffer and Pos
if (!BufLen || (Pos < 0) || (Pos > BufLen)) { if (!BufSize || (Pos < 0) || (Pos > BufLen)) {
return 0; return 0;
} }
@@ -103,7 +105,7 @@ int CRollingBuffer::PeekDirect( char ** Data, int PeekPos )
int StartPos = 0; int StartPos = 0;
// Validate // Validate
if (!BufLen || !Data || (PeekPos >= BufLen)) { if (!BufSize || !Data || (PeekPos > BufLen)) {
*Data = NULL; *Data = NULL;
return 0; return 0;
} }
@@ -133,7 +135,7 @@ int CRollingBuffer::Peek( char ** Data, int PeekPos, int MaxLen )
int StartPos = 0; int StartPos = 0;
// Validate // Validate
if (!BufLen || !*Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) { if (!BufSize || !Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) {
*Data = NULL; *Data = NULL;
return 0; return 0;
} }
@@ -178,7 +180,7 @@ int CRollingBuffer::PeekCopy( char ** Data, int PeekPos, int MaxLen )
int StartPos = 0; int StartPos = 0;
// Check if any data // Check if any data
if (!BufLen || !*Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) { if (!BufSize || !Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) {
if (*Data) (*Data)[0] = 0; if (*Data) (*Data)[0] = 0;
return 0; return 0;
} }
@@ -224,7 +226,7 @@ bool CRollingBuffer::FindStr( const char * SearchStr, int SearchLen, int &FoundP
char * MatchLimit = NULL; char * MatchLimit = NULL;
// Check if buffer exists // Check if buffer exists
if (!BufLen || !SearchStr || !SearchLen) { if (!BufSize || !SearchStr || !SearchLen) {
FoundPos = -1; FoundPos = -1;
return false; return false;
} }
@@ -251,7 +253,7 @@ bool CRollingBuffer::FindStr( const char * SearchStr, int SearchLen, int &FoundP
if (*CheckPos == *MatchPos) { if (*CheckPos == *MatchPos) {
if (MatchPos == MatchLimit) { if (MatchPos == MatchLimit) {
// Full match found // Full match found
FoundPos -= SearchLen; FoundPos -= SearchLen-1;
return true; return true;
} }
else { else {
@@ -326,7 +328,7 @@ int CRollingBuffer::Push( bool Overwrite, const char * Data, int Len )
DataRemain = (!Overwrite && (Len > BufSize-BufLen))? BufSize-BufLen : Len; DataRemain = (!Overwrite && (Len > BufSize-BufLen))? BufSize-BufLen : Len;
// Read Data into buffer // Read Data into buffer
while (TotalPushed < Len) while (DataRemain)
{ {
// Read from file descriptor // Read from file descriptor
BufRemain = BufSize - BufEnd; BufRemain = BufSize - BufEnd;
@@ -362,7 +364,7 @@ int CRollingBuffer::Pop( char ** Data, int MaxLen )
int BytesWritten = 0; int BytesWritten = 0;
// Read & clear data // Read & clear data
BytesWritten = Peek( Data, MaxLen ); BytesWritten = Peek( Data, 0, MaxLen );
Clear( BytesWritten ); Clear( BytesWritten );
// Return temp buffer // Return temp buffer
@@ -377,7 +379,7 @@ int CRollingBuffer::PopCopy( char ** Data, int MaxLen )
int BytesWritten = 0; int BytesWritten = 0;
// Copy & clear data // Copy & clear data
BytesWritten = PeekCopy( Data, MaxLen ); BytesWritten = PeekCopy( Data, 0, MaxLen );
Clear( BytesWritten ); Clear( BytesWritten );
// Return temp buffer // Return temp buffer
@@ -486,6 +488,7 @@ CShiftBuffer::CShiftBuffer( int BufferSize ) : BufSize( BufferSize )
Buffer = (char *)malloc( BufSize+1 ); Buffer = (char *)malloc( BufSize+1 );
Buffer[0] = 0; Buffer[0] = 0;
// Create temp output buffer big enough to handle all data
OutBuffer = (char *)malloc( BufSize+1 ); OutBuffer = (char *)malloc( BufSize+1 );
} }
else { else {
@@ -529,7 +532,7 @@ int CShiftBuffer::Reset()
char CShiftBuffer::PeekChar( int Pos ) char CShiftBuffer::PeekChar( int Pos )
{ {
// Validate buffer and Pos // Validate buffer and Pos
if (!BufLen || (Pos < 0) || (Pos > BufLen)) { if (!BufSize || (Pos < 0) || (Pos > BufLen)) {
return 0; return 0;
} }
@@ -543,7 +546,7 @@ char CShiftBuffer::PeekChar( int Pos )
int CShiftBuffer::PeekDirect( char ** Data, int PeekPos ) int CShiftBuffer::PeekDirect( char ** Data, int PeekPos )
{ {
// Validate // Validate
if (!BufLen || !Data || (PeekPos >= BufLen)) { if (!BufSize || !Data || (PeekPos > BufLen)) {
*Data = NULL; *Data = NULL;
return 0; return 0;
} }
@@ -560,7 +563,7 @@ int CShiftBuffer::Peek( char ** Data, int PeekPos, int MaxLen )
int BytesReturned = 0; int BytesReturned = 0;
// Validate // Validate
if (!BufLen || !Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) { if (!BufSize || !Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) {
*Data = NULL; *Data = NULL;
return 0; return 0;
} }
@@ -572,7 +575,7 @@ int CShiftBuffer::Peek( char ** Data, int PeekPos, int MaxLen )
BytesReturned = (MaxLen > BufLen - PeekPos)? (BufLen - PeekPos) : MaxLen; BytesReturned = (MaxLen > BufLen - PeekPos)? (BufLen - PeekPos) : MaxLen;
// Copy Data from Buffer // Copy Data from Buffer
memcpy( OutBuffer, Buffer, BytesReturned ); memcpy( OutBuffer, &Buffer[PeekPos], BytesReturned );
OutBuffer[BytesReturned] = 0; OutBuffer[BytesReturned] = 0;
// Return Temp buffer // Return Temp buffer
@@ -589,7 +592,7 @@ int CShiftBuffer::PeekCopy( char ** Data, int PeekPos, int MaxLen )
int StartPos = 0; int StartPos = 0;
// Check if any data // Check if any data
if (!BufLen || !Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) { if (!BufSize || !Data || !MaxLen || (PeekPos < 0) || (PeekPos > BufLen)) {
if (*Data) (*Data)[0] = 0; if (*Data) (*Data)[0] = 0;
return 0; return 0;
} }
@@ -621,7 +624,7 @@ bool CShiftBuffer::FindStr( const char * SearchStr, int SearchLen, int &FoundPos
char * MatchLimit = NULL; char * MatchLimit = NULL;
// Check if buffer exists // Check if buffer exists
if (!BufLen || !SearchStr || !SearchLen) { if (!BufSize || !SearchStr || !SearchLen) {
FoundPos = -1; FoundPos = -1;
return false; return false;
} }
@@ -648,7 +651,7 @@ bool CShiftBuffer::FindStr( const char * SearchStr, int SearchLen, int &FoundPos
if (*CheckPos == *MatchPos) { if (*CheckPos == *MatchPos) {
if (MatchPos == MatchLimit) { if (MatchPos == MatchLimit) {
// Full match found // Full match found
FoundPos -= SearchLen; FoundPos -= SearchLen-1;
return true; return true;
} }
else { else {
@@ -692,7 +695,7 @@ int CShiftBuffer::Clear( int ClearLen )
else { else {
// Shift remaining data // Shift remaining data
BytesCleared = ClearLen; BytesCleared = ClearLen;
memcpy( Buffer, &Buffer[ClearLen], BufLen-ClearLen ); // Shift remaining old data left memmove( Buffer, &Buffer[ClearLen], BufLen-ClearLen ); // Shift remaining old data left
BufLen -= ClearLen; BufLen -= ClearLen;
Buffer[BufLen] = 0; Buffer[BufLen] = 0;
} }
@@ -756,7 +759,7 @@ int CShiftBuffer::Pop( char ** Data, int MaxLen )
int BytesWritten = 0; int BytesWritten = 0;
// Read and clear bytes // Read and clear bytes
BytesWritten = Peek( Data, MaxLen ); BytesWritten = Peek( Data, 0, MaxLen );
Clear( BytesWritten ); Clear( BytesWritten );
return BytesWritten; return BytesWritten;
@@ -770,7 +773,7 @@ int CShiftBuffer::PopCopy( char ** Data, int MaxLen )
int BytesWritten = 0; int BytesWritten = 0;
// Copy and clear bytes // Copy and clear bytes
BytesWritten = PeekCopy( Data, MaxLen ); BytesWritten = PeekCopy( Data, 0, MaxLen );
Clear( BytesWritten ); Clear( BytesWritten );
// Return temp buffer // Return temp buffer

View File

@@ -53,7 +53,7 @@ private:
public: public:
// Life Cycle // Life Cycle
CRollingBuffer( int BufferSize ); CRollingBuffer( int BufferSize );
virtual ~CRollingBuffer(); ~CRollingBuffer();
// Direct Operations // Direct Operations
int Reset(); // Clear buffer and reset all pointers to start of buffer int Reset(); // Clear buffer and reset all pointers to start of buffer
@@ -67,15 +67,15 @@ public:
bool FindStr( const char * SearchStr, int SearchLen, int &FoundPos, int StartPos = 0 ); // Search buffer directly bool FindStr( const char * SearchStr, int SearchLen, int &FoundPos, int StartPos = 0 ); // Search buffer directly
// Change data on buffer // Change data on buffer
virtual int Push( bool Overwrite, const char * Data, int Len = -1 ); int Push( bool Overwrite, const char * Data, int Len = -1 );
virtual int Clear( int ClearLen = -1 ); int Clear( int ClearLen = -1 );
virtual int Pop( char ** Data, int MaxLen = -1 ); int Pop( char ** Data, int MaxLen = -1 );
virtual int PopCopy( char ** Data, int MaxLen = -1 ); int PopCopy( char ** Data, int MaxLen = -1 );
// File operations // File operations
virtual int ReadFromFD( int Handle, int MaxRead = -1, bool Overwrite = false ); int ReadFromFD( int Handle, int MaxRead = -1, bool Overwrite = false );
virtual int WriteToFD( int Handle, int MaxWrite = -1 ); int WriteToFD( int Handle, int MaxWrite = -1 );
// Miscellaneous // Miscellaneous
int Size() { return BufSize; }; int Size() { return BufSize; };
@@ -97,7 +97,7 @@ private:
public: public:
// Life Cycle // Life Cycle
CShiftBuffer( int BufferSize ); CShiftBuffer( int BufferSize );
virtual ~CShiftBuffer(); ~CShiftBuffer();
// Direct Operations // Direct Operations
int Reset(); // Clear buffer and reset all pointers to start of buffer int Reset(); // Clear buffer and reset all pointers to start of buffer
@@ -110,16 +110,16 @@ public:
bool FindStr( const char * SearchStr, int SearchLen, int &FoundPos, int StartPos = 0 ); // Search buffer directly bool FindStr( const char * SearchStr, int SearchLen, int &FoundPos, int StartPos = 0 ); // Search buffer directly
// Chagne data on buffer // Change data on buffer
virtual int Push( bool Overwrite, const char * Data, int Len = -1 ); int Push( bool Overwrite, const char * Data, int Len = -1 );
virtual int Clear( int ClearLen = -1 ); int Clear( int ClearLen = -1 );
virtual int Pop( char ** Data, int MaxLen = -1 ); int Pop( char ** Data, int MaxLen = -1 );
virtual int PopCopy( char ** Data, int MaxLen = -1 ); int PopCopy( char ** Data, int MaxLen = -1 );
// File operations // File operations
virtual int ReadFromFD( int Handle, int MaxRead = -1 ); int ReadFromFD( int Handle, int MaxRead = -1 );
virtual int WriteToFD( int Handle, int MaxWrite = -1 ); int WriteToFD( int Handle, int MaxWrite = -1 );
// Miscellaneous // Miscellaneous
int Size() { return BufSize; }; int Size() { return BufSize; };

View File

@@ -1060,7 +1060,7 @@ bool CSelectableCore::ProcessBuffer( THandle * Handle, bool Force )
while (Handle->InBuffer->FindStr( Handle->InMarker, Handle->InMarkerLen, Pos )) while (Handle->InBuffer->FindStr( Handle->InMarker, Handle->InMarkerLen, Pos ))
{ {
// Show Packet // Show Packet
Len = Handle->InBuffer->Peek( &Data, 0, Pos+Handle->InMarkerLen+1 ); Len = Handle->InBuffer->Peek( &Data, 0, Pos+Handle->InMarkerLen );
if (Log) Log->Output( DebugLevel, dlHigh, OutputDisplay, Data, Len, "%s: Handle '%s' - IN-M:", Name, Handle->Name ); if (Log) Log->Output( DebugLevel, dlHigh, OutputDisplay, Data, Len, "%s: Handle '%s' - IN-M:", Name, Handle->Name );
// Write buffer to Outputs // Write buffer to Outputs