Add Format option for buffer writers and string builders (#27572)

* Add Format option for buffer writers and string builders * Update comments * Updated to only have printf inside stringbuilder and NOT bufferwriter * Restyle * Add missing file * remove cpp file comments * Fix cast to make clang happy * Never pass null pointer in vsnprintf, since our size available is never 0 * Restyle * Update src/lib/support/StringBuilder.cpp Co-authored-by: Damian Królik <[email protected]> --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Damian Królik <[email protected]>
project-chip · Oct 19, 2023 · 1995248 · 1995248
1 parent 83ae559
commit 1995248
Show file tree

Hide file tree

Showing 5 changed files with 224 additions and 9 deletions.
diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn
@@ -132,6 +132,8 @@ static_library("support") {
     "SetupDiscriminator.h",
     "SortUtils.h",
     "StateMachine.h",
+    "StringBuilder.cpp",
+    "StringBuilder.h",
     "StringSplitter.h",
     "ThreadOperationalDataset.cpp",
     "ThreadOperationalDataset.h",

diff --git a/src/lib/support/BufferWriter.cpp b/src/lib/support/BufferWriter.cpp
@@ -23,11 +23,7 @@ namespace Encoding {
 BufferWriter & BufferWriter::Put(const char * s)
 {
     static_assert(CHAR_BIT == 8, "We're assuming char and uint8_t are the same size");
-    while (*s != 0)
-    {
-        Put(static_cast<uint8_t>(*s++));
-    }
-    return *this;
+    return Put(s, strlen(s));
 }
 
 BufferWriter & BufferWriter::Put(const void * buf, size_t len)

diff --git a/src/lib/support/StringBuilder.cpp b/src/lib/support/StringBuilder.cpp
@@ -0,0 +1,67 @@
+/*
+ *
+ *    Copyright (c) 2023 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+#include "StringBuilder.h"
+
+namespace chip {
+
+StringBuilderBase & StringBuilderBase::AddFormat(const char * format, ...)
+{
+    va_list args;
+    va_start(args, format);
+
+    char * output = nullptr;
+    if (mWriter.Available() > 0)
+    {
+        output = reinterpret_cast<char *>(mWriter.Buffer() + mWriter.Needed());
+    }
+    else
+    {
+        output = reinterpret_cast<char *>(mWriter.Buffer() + mWriter.Size());
+    }
+
+    // the + 1 size here because StringBuilder reserves one byte for final null terminator
+    int needed = vsnprintf(output, mWriter.Available() + 1, format, args);
+
+    // on invalid formats, printf-style methods return negative numbers
+    if (needed > 0)
+    {
+        mWriter.Skip(static_cast<size_t>(needed));
+    }
+
+    va_end(args);
+    NullTerminate();
+    return *this;
+}
+
+StringBuilderBase & StringBuilderBase::AddMarkerIfOverflow()
+{
+    if (mWriter.Fit())
+    {
+        return *this;
+    }
+
+    for (unsigned i = 0; i < 3; i++)
+    {
+        if (mWriter.Size() >= i + 1)
+        {
+            mWriter.Buffer()[mWriter.Size() - i - 1] = '.';
+        }
+    }
+    return *this;
+}
+} // namespace chip
diff --git a/src/lib/support/StringBuilder.h b/src/lib/support/StringBuilder.h
@@ -56,6 +56,13 @@ class StringBuilderBase
     /// Was nothing written yet?
     bool Empty() const { return mWriter.Needed() == 0; }
 
+    /// Write a formatted string to the stringbuilder
+    StringBuilderBase & AddFormat(const char * format, ...) ENFORCE_FORMAT(2, 3);
+
+    /// For strings we often want to know when they were truncated. If the underlying writer did
+    /// not fit, this replaces the last 3 characters with "."
+    StringBuilderBase & AddMarkerIfOverflow();
+
     /// access the underlying value
     const char * c_str() const { return reinterpret_cast<const char *>(mWriter.Buffer()); }
 

diff --git a/src/lib/support/tests/TestStringBuilder.cpp b/src/lib/support/tests/TestStringBuilder.cpp
@@ -78,11 +78,154 @@ void TestOverflow(nlTestSuite * inSuite, void * inContext)
     }
 }
 
+void TestFormat(nlTestSuite * inSuite, void * inContext)
+{
+    {
+        StringBuilder<100> builder;
+
+        builder.AddFormat("Test: %d Hello %s\n", 123, "world");
+
+        NL_TEST_ASSERT(inSuite, builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "Test: 123 Hello world\n") == 0);
+    }
+
+    {
+        StringBuilder<100> builder;
+
+        builder.AddFormat("Align: %-5s", "abc");
+
+        NL_TEST_ASSERT(inSuite, builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "Align: abc  ") == 0);
+    }
+
+    {
+        StringBuilder<100> builder;
+
+        builder.AddFormat("Multi: %d", 1234);
+        builder.AddFormat(", then 0x%04X", 0xab);
+
+        NL_TEST_ASSERT(inSuite, builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "Multi: 1234, then 0x00AB") == 0);
+    }
+}
+
+void TestFormatOverflow(nlTestSuite * inSuite, void * inContext)
+{
+    {
+        StringBuilder<13> builder;
+
+        builder.AddFormat("Test: %d Hello %s\n", 123, "world");
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "Test: 123 He") == 0);
+    }
+
+    {
+        StringBuilder<11> builder;
+
+        builder.AddFormat("%d %d %d %d %d", 1, 2, 3, 4, 1234);
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1 2 3 4 12") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1 2 3 4...") == 0);
+    }
+
+    {
+        StringBuilder<11> builder;
+
+        builder.AddFormat("%d", 1234);
+        NL_TEST_ASSERT(inSuite, builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1234") == 0);
+
+        builder.AddFormat("%s", "abc");
+        NL_TEST_ASSERT(inSuite, builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1234abc") == 0);
+
+        builder.AddMarkerIfOverflow(); // no overflow
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1234abc") == 0);
+
+        builder.AddFormat("%08x", 0x123456);
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1234abc001") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "1234abc...") == 0);
+    }
+}
+
+void TestOverflowMarker(nlTestSuite * inSuite, void * inContext)
+{
+    {
+        StringBuilder<1> builder; // useless builder, but ok
+
+        builder.Add("abc123");
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "") == 0);
+    }
+
+    {
+        StringBuilder<2> builder;
+
+        builder.Add("abc123");
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "a") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), ".") == 0);
+    }
+
+    {
+        StringBuilder<3> builder;
+
+        builder.Add("abc123");
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "ab") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "..") == 0);
+    }
+
+    {
+        StringBuilder<4> builder;
+
+        builder.Add("abc123");
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "abc") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "...") == 0);
+    }
+
+    {
+        StringBuilder<5> builder;
+
+        builder.Add("abc123");
+
+        NL_TEST_ASSERT(inSuite, !builder.Fit());
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "abc1") == 0);
+
+        builder.AddMarkerIfOverflow();
+        NL_TEST_ASSERT(inSuite, strcmp(builder.c_str(), "a...") == 0);
+    }
+}
+
 const nlTest sTests[] = {
-    NL_TEST_DEF("TestStringBuilder", TestStringBuilder), //
-    NL_TEST_DEF("TestIntegerAppend", TestIntegerAppend), //
-    NL_TEST_DEF("TestOverflow", TestOverflow),           //
-    NL_TEST_SENTINEL()                                   //
+    NL_TEST_DEF("TestStringBuilder", TestStringBuilder),   //
+    NL_TEST_DEF("TestIntegerAppend", TestIntegerAppend),   //
+    NL_TEST_DEF("TestOverflow", TestOverflow),             //
+    NL_TEST_DEF("TestFormat", TestFormat),                 //
+    NL_TEST_DEF("TestFormatOverflow", TestFormatOverflow), //
+    NL_TEST_DEF("TestOverflowMarker", TestOverflowMarker), //
+    NL_TEST_SENTINEL()                                     //
 };
 
 } // namespace