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 <[email protected]> --------- Co-authored-by: [email protected] <[email protected]> Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Kevin Schoedel <[email protected]>
project-chip · Feb 5, 2024 · 2550009 · 2550009
1 parent 188b457
commit 2550009
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 59 deletions.
diff --git 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<char *>(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<char *>(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<uint8_t *>(tmpBuf), static_cast<uint32_t>(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<uint32_t>(p - mWritePoint);
-        mWritePoint  = p;
-        mRemainingLen -= len;
-        mLenWritten += len;
-        return CHIP_NO_ERROR;
-    }
-    return WriteData(stagingBuf, static_cast<uint32_t>(p - stagingBuf));
+    uint32_t bytesStaged = static_cast<uint32_t>(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<uint32_t>(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
@@ -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}",
   ]