From 1761394ed9de9ea74a102f3e81c31436dedc2a9a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jul 2023 09:24:00 -0400 Subject: [PATCH] Add Format option for buffer writers and string builders (#27572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 <66667989+Damian-Nordic@users.noreply.github.com> --------- Co-authored-by: Andrei Litvin Co-authored-by: Damian Królik <66667989+Damian-Nordic@users.noreply.github.com> --- src/lib/support/BUILD.gn | 2 + src/lib/support/BufferWriter.cpp | 6 +- src/lib/support/StringBuilder.cpp | 67 +++++++++ src/lib/support/StringBuilder.h | 7 + src/lib/support/tests/TestStringBuilder.cpp | 151 +++++++++++++++++++- 5 files changed, 224 insertions(+), 9 deletions(-) create mode 100644 src/lib/support/StringBuilder.cpp diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn index 618adc4e446e45..68b39ebb0a802b 100644 --- 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 index 1d7da851ea0ba1..e9394703cbea28 100644 --- 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(*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 new file mode 100644 index 00000000000000..febabc0cb50773 --- /dev/null +++ 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(mWriter.Buffer() + mWriter.Needed()); + } + else + { + output = reinterpret_cast(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(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 index 85452c217afb9e..9de1538da1df51 100644 --- 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(mWriter.Buffer()); } diff --git a/src/lib/support/tests/TestStringBuilder.cpp b/src/lib/support/tests/TestStringBuilder.cpp index 2886d6fe4b307d..34d04304f4eff1 100644 --- 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