From 5602f179a177a8513b96c762590829c012c28175 Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 5 Oct 2022 17:01:35 -1000 Subject: [PATCH] Add varint-128 encode/decode to IoRead/IoWrite. This makes it more efficient to read/write (especially read) varint-128 to/from IO. Update the Pack type to take advantage of the more efficient read and remove some duplicate code. --- doc/xml/release.xml | 3 ++ src/common/io/read.c | 58 +++++++++++++++++++++++++++ src/common/io/read.h | 3 ++ src/common/io/write.c | 20 +++++++++ src/common/io/write.h | 3 ++ src/common/type/pack.c | 29 +++----------- test/define.yaml | 2 +- test/src/module/common/ioTest.c | 40 ++++++++++++++++++ test/src/module/common/typePackTest.c | 6 +-- 9 files changed, 136 insertions(+), 28 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index cae0b126a..2a9789ad6 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -57,6 +57,9 @@ + + + diff --git a/src/common/io/read.c b/src/common/io/read.c index c28d20566..321165938 100644 --- a/src/common/io/read.c +++ b/src/common/io/read.c @@ -355,6 +355,64 @@ ioReadLineParam(IoRead *this, bool allowEof) FUNCTION_LOG_RETURN(STRING, result); } +/**********************************************************************************************************************************/ +uint64_t +ioReadVarIntU64(IoRead *const this) +{ + FUNCTION_LOG_BEGIN(logLevelTrace); + FUNCTION_LOG_PARAM(IO_READ, this); + FUNCTION_LOG_END(); + + // Allocate the internal output buffer if it has not already been allocated + if (this->output == NULL) + { + MEM_CONTEXT_BEGIN(this->pub.memContext) + { + this->output = bufNew(ioBufferSize()); + } + MEM_CONTEXT_END(); + } + + uint64_t result = 0; + uint8_t byte; + + // Convert bytes from varint-128 encoding to a uint64 + for (unsigned int bufferIdx = 0; bufferIdx < CVT_VARINT128_BUFFER_SIZE; bufferIdx++) + { + // Get more bytes if needed + if (bufUsed(this->output) - this->outputPos == 0) + { + // Clear the internal output buffer since all data was copied already + bufUsedZero(this->output); + this->outputPos = 0; + + // Read into the internal output buffer + ioReadInternal(this, this->output, false); + + // Error on eof + if (bufUsed(this->output) == 0) + THROW(FileReadError, "unexpected eof"); + } + + // Get the next encoded byte + byte = bufPtr(this->output)[this->outputPos++]; + + // Shift the lower order 7 encoded bits into the uint64 in reverse order + result |= (uint64_t)(byte & 0x7f) << (7 * bufferIdx); + + // Done if the high order bit is not set to indicate more data + if (byte < 0x80) + break; + } + + // By this point all bytes should have been read so error if this is not the case. This could be due to a coding error or + // corrupton in the data stream. + if (byte >= 0x80) + THROW(FormatError, "unterminated base-128 integer"); + + FUNCTION_LOG_RETURN(UINT64, result); +} + /**********************************************************************************************************************************/ bool ioReadReady(IoRead *this, IoReadReadyParam param) diff --git a/src/common/io/read.h b/src/common/io/read.h index e8e19422b..52866bb0a 100644 --- a/src/common/io/read.h +++ b/src/common/io/read.h @@ -77,6 +77,9 @@ typedef struct IoReadReadyParam bool error; // Error when read not ready } IoReadReadyParam; +// Read varint-128 encoding +uint64_t ioReadVarIntU64(IoRead *this); + #define ioReadReadyP(this, ...) \ ioReadReady(this, (IoReadReadyParam){VAR_PARAM_INIT, __VA_ARGS__}) diff --git a/src/common/io/write.c b/src/common/io/write.c index 9289fdf90..cef02f494 100644 --- a/src/common/io/write.c +++ b/src/common/io/write.c @@ -195,6 +195,26 @@ ioWriteStrLine(IoWrite *this, const String *string) FUNCTION_LOG_RETURN_VOID(); } +/**********************************************************************************************************************************/ +void +ioWriteVarIntU64(IoWrite *const this, const uint64_t value) +{ + FUNCTION_LOG_BEGIN(logLevelTrace); + FUNCTION_LOG_PARAM(IO_WRITE, this); + FUNCTION_LOG_PARAM(UINT64, value); + FUNCTION_LOG_END(); + + ASSERT(this != NULL); + + unsigned char buffer[CVT_VARINT128_BUFFER_SIZE]; + size_t bufferPos = 0; + + cvtUInt64ToVarInt128(value, buffer, &bufferPos, sizeof(buffer)); + ioWrite(this, BUF(buffer, bufferPos)); + + FUNCTION_LOG_RETURN_VOID(); +} + /**********************************************************************************************************************************/ void ioWriteFlush(IoWrite *this) diff --git a/src/common/io/write.h b/src/common/io/write.h index a1aecc998..88b16c5f4 100644 --- a/src/common/io/write.h +++ b/src/common/io/write.h @@ -67,6 +67,9 @@ void ioWriteStr(IoWrite *this, const String *string); // Write linefeed-terminated string void ioWriteStrLine(IoWrite *this, const String *string); +// Write varint-128 encoding +void ioWriteVarIntU64(IoWrite *this, uint64_t value); + // Flush any data in the output buffer. This does not end writing and will not work if filters are present. void ioWriteFlush(IoWrite *this); diff --git a/src/common/type/pack.c b/src/common/type/pack.c index 4902468b8..fd6df3d07 100644 --- a/src/common/type/pack.c +++ b/src/common/type/pack.c @@ -484,33 +484,14 @@ pckReadU64Internal(PackRead *this) ASSERT(this != NULL); - uint64_t result = 0; - uint8_t byte; - - // Convert bytes from varint-128 encoding to a uint64 - for (unsigned int bufferIdx = 0; bufferIdx < CVT_VARINT128_BUFFER_SIZE; bufferIdx++) + if (this->read != NULL) { - // Get the next encoded byte - pckReadBuffer(this, 1); - byte = this->bufferPtr[this->bufferPos]; - - // Shift the lower order 7 encoded bits into the uint64 in reverse order - result |= (uint64_t)(byte & 0x7f) << (7 * bufferIdx); - - // Increment buffer position to indicate that the byte has been processed - this->bufferPos++; - - // Done if the high order bit is not set to indicate more data - if (byte < 0x80) - break; + // Internal buffer should be empty + ASSERT(this->bufferUsed == this->bufferPos); + FUNCTION_TEST_RETURN(UINT64, ioReadVarIntU64(this->read)); } - // By this point all bytes should have been read so error if this is not the case. This could be due to a coding error or - // corrupton in the data stream. - if (byte >= 0x80) - THROW(FormatError, "unterminated base-128 integer"); - - FUNCTION_TEST_RETURN(UINT64, result); + FUNCTION_TEST_RETURN(UINT64, cvtUInt64FromVarInt128(this->bufferPtr, &this->bufferPos, this->bufferUsed)); } /*********************************************************************************************************************************** diff --git a/test/define.yaml b/test/define.yaml index 07079cd57..7245facf0 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -260,7 +260,7 @@ unit: # ---------------------------------------------------------------------------------------------------------------------------- - name: io - total: 4 + total: 5 feature: IO harness: pack diff --git a/test/src/module/common/ioTest.c b/test/src/module/common/ioTest.c index e0f2c1f30..50d9caa9d 100644 --- a/test/src/module/common/ioTest.c +++ b/test/src/module/common/ioTest.c @@ -587,6 +587,46 @@ testRun(void) pckReadU64P(ioFilterGroupResultP(filterGroup, STRID5("size2", 0x1c2e9330))), 22, " check filter result"); } + // ***************************************************************************************************************************** + if (testBegin("ioReadVarIntU64() and ioWriteVarIntU64()")) + { + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("ioWriteVarIntU64()"); + + Buffer *buffer = bufNew(32); + IoWrite *write = ioBufferWriteNewOpen(buffer); + + TEST_RESULT_VOID(ioWriteVarIntU64(write, 7777777), "write varint"); + TEST_RESULT_VOID(ioWriteVarIntU64(write, 0), "write varint"); + TEST_RESULT_VOID(ioWriteClose(write), "close write"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("ioReadVarIntU64()"); + + IoRead *read = ioBufferReadNewOpen(buffer); + + TEST_RESULT_UINT(ioReadVarIntU64(read), 7777777, "read varint"); + TEST_RESULT_UINT(ioReadVarIntU64(read), 0, "read varint"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error on eof"); + + bufPtr(buffer)[0] = 0xFF; + bufUsedSet(buffer, 1); + read = ioBufferReadNewOpen(buffer); + + TEST_ERROR(ioReadVarIntU64(read), FileReadError, "unexpected eof"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error on too many bytes"); + + memset(bufPtr(buffer), 0xFF, bufSize(buffer)); + bufUsedSet(buffer, bufSize(buffer)); + read = ioBufferReadNewOpen(buffer); + + TEST_ERROR(ioReadVarIntU64(read), FormatError, "unterminated base-128 integer"); + } + // ***************************************************************************************************************************** if (testBegin("IoFdRead, IoFdWrite, and ioFdWriteOneStr()")) { diff --git a/test/src/module/common/typePackTest.c b/test/src/module/common/typePackTest.c index b6db1ca76..82253638a 100644 --- a/test/src/module/common/typePackTest.c +++ b/test/src/module/common/typePackTest.c @@ -323,14 +323,14 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("EOF on short buffer"); - TEST_ASSIGN(packRead, pckReadNew(pckFromBuf(BUFSTRDEF("\255"))), "new read"); - TEST_ERROR(pckReadU64Internal(packRead), FormatError, "unexpected EOF"); + TEST_ASSIGN(packRead, pckReadNew(pckFromBuf(BUFSTRDEF(""))), "new read"); + TEST_ERROR(pckReadBuffer(packRead, 2), FormatError, "unexpected EOF"); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("error on invalid uint64"); TEST_ASSIGN(packRead, pckReadNew(pckFromBuf(BUFSTRDEF("\255\255\255\255\255\255\255\255\255\255"))), "new read"); - TEST_ERROR(pckReadU64Internal(packRead), FormatError, "unterminated base-128 integer"); + TEST_ERROR(pckReadU64Internal(packRead), FormatError, "unterminated varint-128 integer"); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("pack/unpack pointer");