From 255000901bc453f07cf0a4d3354eeeb895d35e01 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Tue, 30 Jan 2024 23:16:13 -0500 Subject: [PATCH] Remove several direct uses of TLVWriter::mWritePoint (#31770) * Remove several direct uses of TLVWriter::mWritePoint - Remove all writing to mWritePoint that occur outside of WriteData - Remove unsafe usage of CONFIG_HAVE_VSNPRINTF_EX - Does not impact public API as it was one underlying possible optimization, and the remaining code would work. Also, not used in SDK at all. - Removed secondary paths that were potentially unsafe in TLVWriter::WriteElementHead Fixes #31769 Testing done: - All existing unit tests still pass. - Only removed and non-functionally-refactored code. * Restyled by clang-format * Update src/lib/core/TLVWriter.cpp Co-authored-by: Kevin Schoedel --------- Co-authored-by: tennessee.carmelveilleux@gmail.com Co-authored-by: Restyled.io Co-authored-by: Kevin Schoedel --- src/lib/core/TLVWriter.cpp | 69 ++++++-------------------------------- src/system/BUILD.gn | 1 - 2 files changed, 11 insertions(+), 59 deletions(-) diff --git a/src/lib/core/TLVWriter.cpp b/src/lib/core/TLVWriter.cpp index b20cc70adde8f0..825b8c7b671406 100644 --- a/src/lib/core/TLVWriter.cpp +++ b/src/lib/core/TLVWriter.cpp @@ -101,6 +101,7 @@ CHIP_ERROR TLVWriter::Init(TLVBackingStore & backingStore, uint32_t maxLen /* = if (err != CHIP_NO_ERROR) return err; + VerifyOrReturnError(mBufStart != nullptr, CHIP_ERROR_INTERNAL); mWritePoint = mBufStart; mInitializationCookie = kExpectedInitializationCookie; return CHIP_NO_ERROR; @@ -363,11 +364,7 @@ CHIP_ERROR TLVWriter::VPutStringF(Tag tag, const char * fmt, va_list ap) size_t dataLen; CHIP_ERROR err = CHIP_NO_ERROR; TLVFieldSize lenFieldSize; -#if CONFIG_HAVE_VSNPRINTF_EX - size_t skipLen; - size_t writtenBytes; -#elif CONFIG_HAVE_VCBPRINTF -#else +#if !CONFIG_HAVE_VCBPRINTF char * tmpBuf; #endif va_copy(aq, ap); @@ -396,47 +393,14 @@ CHIP_ERROR TLVWriter::VPutStringF(Tag tag, const char * fmt, va_list ap) VerifyOrExit((mLenWritten + dataLen) <= mMaxLen, err = CHIP_ERROR_BUFFER_TOO_SMALL); // write data -#if CONFIG_HAVE_VSNPRINTF_EX - - skipLen = 0; - - do - { - va_copy(aq, ap); - - vsnprintf_ex(reinterpret_cast(mWritePoint), mRemainingLen, skipLen, fmt, aq); - - va_end(aq); - - writtenBytes = (mRemainingLen >= (dataLen - skipLen)) ? dataLen - skipLen : mRemainingLen; - skipLen += writtenBytes; - mWritePoint += writtenBytes; - mRemainingLen -= writtenBytes; - mLenWritten += writtenBytes; - if (skipLen < dataLen) - { - VerifyOrExit(mBackingStore != NULL, err = CHIP_ERROR_NO_MEMORY); - - err = mBackingStore->FinalizeBuffer(*this, mBufHandle, mBufStart, mWritePoint - mBufStart); - SuccessOrExit(err); - - err = mBackingStore->GetNewBuffer(*this, mBufHandle, mBufStart, mRemainingLen); - SuccessOrExit(err); - - mWritePoint = mBufStart; - } - - } while (skipLen < dataLen); - -#elif CONFIG_HAVE_VCBPRINTF +#if CONFIG_HAVE_VCBPRINTF va_copy(aq, ap); vcbprintf(TLVWriterPutcharCB, this, dataLen, fmt, aq); va_end(aq); - -#else // CONFIG_HAVE_VSNPRINTF_EX +#else // CONFIG_HAVE_VCBPRINTF tmpBuf = static_cast(chip::Platform::MemoryAlloc(dataLen + 1)); VerifyOrExit(tmpBuf != nullptr, err = CHIP_ERROR_NO_MEMORY); @@ -450,7 +414,7 @@ CHIP_ERROR TLVWriter::VPutStringF(Tag tag, const char * fmt, va_list ap) err = WriteData(reinterpret_cast(tmpBuf), static_cast(dataLen)); chip::Platform::MemoryFree(tmpBuf); -#endif // CONFIG_HAVE_VSNPRINTF_EX +#endif // CONFIG_HAVE_VCBPRINTF exit: @@ -694,19 +658,13 @@ CHIP_ERROR TLVWriter::WriteElementHead(TLVElementType elemType, Tag tag, uint64_ ABORT_ON_UNINITIALIZED_IF_ENABLED(); VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); - uint8_t * p; - uint8_t stagingBuf[17]; // 17 = 1 control byte + 8 tag bytes + 8 length/value bytes - if (IsContainerOpen()) return CHIP_ERROR_TLV_CONTAINER_OPEN; + uint8_t stagingBuf[17]; // 17 = 1 control byte + 8 tag bytes + 8 length/value bytes + uint8_t * p = stagingBuf; uint32_t tagNum = TagNumFromTag(tag); - if ((mRemainingLen >= sizeof(stagingBuf)) && (mMaxLen >= sizeof(stagingBuf))) - p = mWritePoint; - else - p = stagingBuf; - if (IsSpecialTag(tag)) { if (tagNum <= Tag::kContextTagMaxNum) @@ -799,15 +757,9 @@ CHIP_ERROR TLVWriter::WriteElementHead(TLVElementType elemType, Tag tag, uint64_ break; } - if ((mRemainingLen >= sizeof(stagingBuf)) && (mMaxLen >= sizeof(stagingBuf))) - { - uint32_t len = static_cast(p - mWritePoint); - mWritePoint = p; - mRemainingLen -= len; - mLenWritten += len; - return CHIP_NO_ERROR; - } - return WriteData(stagingBuf, static_cast(p - stagingBuf)); + uint32_t bytesStaged = static_cast(p - stagingBuf); + VerifyOrDie(bytesStaged <= sizeof(stagingBuf)); + return WriteData(stagingBuf, bytesStaged); } CHIP_ERROR TLVWriter::WriteElementWithData(TLVType type, Tag tag, const uint8_t * data, uint32_t dataLen) @@ -855,6 +807,7 @@ CHIP_ERROR TLVWriter::WriteData(const uint8_t * p, uint32_t len) ReturnErrorOnFailure(mBackingStore->FinalizeBuffer(*this, mBufStart, static_cast(mWritePoint - mBufStart))); ReturnErrorOnFailure(mBackingStore->GetNewBuffer(*this, mBufStart, mRemainingLen)); + VerifyOrReturnError(mRemainingLen > 0, CHIP_ERROR_NO_MEMORY); mWritePoint = mBufStart; diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn index 73acdf1bc24d42..5953dc6a0ffdd0 100644 --- a/src/system/BUILD.gn +++ b/src/system/BUILD.gn @@ -118,7 +118,6 @@ buildconfig_header("system_buildconfig") { "HAVE_NETINET_ICMP6_H=true", "HAVE_ICMP6_FILTER=true", "CONFIG_HAVE_VCBPRINTF=false", - "CONFIG_HAVE_VSNPRINTF_EX=false", "HAVE_SYS_SOCKET_H=${chip_system_config_use_sockets}", ]