From b39f5b6dd2c890b8155fbf5c79c9af5a5d77c327 Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Thu, 6 May 2021 12:52:59 +0800 Subject: [PATCH] [string] introduce StringWriter class (#6564) This commit moves methods of String out of the header file to save code size. Since most use cases of String writers ignores errors, this commit changes the return of writers to the Writer object itself for chained writes. --- src/core/common/logging.cpp | 35 +++--- src/core/common/notifier.cpp | 7 +- src/core/common/string.cpp | 63 +++++++--- src/core/common/string.hpp | 183 ++++++++++------------------- src/core/mac/channel_mask.cpp | 17 +-- src/core/mac/mac_frame.cpp | 38 +++--- src/core/mac/mac_types.cpp | 34 ++++-- src/core/meshcop/meshcop.cpp | 11 +- src/core/net/ip4_address.cpp | 5 +- src/core/net/ip6_address.cpp | 22 ++-- src/core/net/socket.cpp | 5 +- src/core/radio/trel_packet.cpp | 18 +-- src/core/thread/link_quality.cpp | 10 +- src/core/thread/mle.cpp | 2 +- src/core/thread/mle_types.cpp | 7 +- src/core/thread/radio_selector.cpp | 5 +- src/core/utils/channel_monitor.cpp | 2 +- tests/unit/test_string.cpp | 98 ++++++++------- 18 files changed, 284 insertions(+), 278 deletions(-) diff --git a/src/core/common/logging.cpp b/src/core/common/logging.cpp index 42be86d46..3119b2e8a 100644 --- a/src/core/common/logging.cpp +++ b/src/core/common/logging.cpp @@ -59,6 +59,7 @@ static void Log(otLogLevel aLogLevel, va_list aArgs) { ot::String logString; + ot::StringWriter writer(logString); #if OPENTHREAD_CONFIG_LOG_LEVEL_DYNAMIC_ENABLE VerifyOrExit(otLoggingGetLevel() >= aLogLevel); @@ -96,16 +97,18 @@ static void Log(otLogLevel aLogLevel, break; } - IgnoreError(logString.Append("%s", levelStr)); + writer.Append("%s", levelStr); } #endif // OPENTHREAD_CONFIG_LOG_PREPEND_LEVEL - IgnoreError(logString.Append("%s", aRegionPrefix)); - VerifyOrExit(logString.AppendVarArgs(aFormat, aArgs) != ot::kErrorInvalidArgs); + writer.Append("%s", aRegionPrefix); + writer.AppendVarArgs(aFormat, aArgs); otPlatLog(aLogLevel, aLogRegion, "%s" OPENTHREAD_CONFIG_LOG_SUFFIX, logString.AsCString()); +#if OPENTHREAD_CONFIG_LOG_LEVEL_DYNAMIC_ENABLE exit: return; +#endif } #if OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_CRIT @@ -232,26 +235,29 @@ enum : uint8_t static void DumpLine(otLogLevel aLogLevel, otLogRegion aLogRegion, const uint8_t *aBytes, const size_t aLength) { - ot::String string("|"); + ot::String string; + ot::StringWriter writer(string); + + writer.Append("|"); for (uint8_t i = 0; i < kDumpBytesPerLine; i++) { if (i < aLength) { - IgnoreError(string.Append(" %02X", aBytes[i])); + writer.Append(" %02X", aBytes[i]); } else { - IgnoreError(string.Append(" ..")); + writer.Append(" .."); } if (!((i + 1) % 8)) { - IgnoreError(string.Append(" |")); + writer.Append(" |"); } } - IgnoreError(string.Append(" ")); + writer.Append(" "); for (uint8_t i = 0; i < kDumpBytesPerLine; i++) { @@ -267,7 +273,7 @@ static void DumpLine(otLogLevel aLogLevel, otLogRegion aLogRegion, const uint8_t } } - IgnoreError(string.Append("%c", c)); + writer.Append("%c", c); } otLogDump(aLogLevel, aLogRegion, "%s", string.AsCString()); @@ -282,17 +288,18 @@ void otDump(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aId, const size_t idLen = strlen(aId); ot::String string; + ot::StringWriter writer(string); for (size_t i = 0; i < (kWidth - idLen) / 2 - 5; i++) { - IgnoreError(string.Append("=")); + writer.Append("="); } - IgnoreError(string.Append("[%s len=%03u]", aId, static_cast(aLength))); + writer.Append("[%s len=%03u]", aId, static_cast(aLength)); for (size_t i = 0; i < (kWidth - idLen) / 2 - 4; i++) { - IgnoreError(string.Append("=")); + writer.Append("="); } otLogDump(aLogLevel, aLogRegion, "%s", string.AsCString()); @@ -303,11 +310,11 @@ void otDump(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aId, const OT_MIN((aLength - i), static_cast(kDumpBytesPerLine))); } - string.Clear(); + writer.Clear(); for (size_t i = 0; i < kWidth; i++) { - IgnoreError(string.Append("-")); + writer.Append("-"); } otLogDump(aLogLevel, aLogRegion, "%s", string.AsCString()); diff --git a/src/core/common/notifier.cpp b/src/core/common/notifier.cpp index 610331b3e..4d8b65833 100644 --- a/src/core/common/notifier.cpp +++ b/src/core/common/notifier.cpp @@ -212,6 +212,7 @@ void Notifier::LogEvents(Events aEvents) const bool addSpace = false; bool didLog = false; String string; + StringWriter writer(string); for (uint8_t bit = 0; bit < sizeof(Events::Flags) * CHAR_BIT; bit++) { @@ -219,16 +220,16 @@ void Notifier::LogEvents(Events aEvents) const if (flags & (1 << bit)) { - if (string.GetLength() >= kFlagsStringLineLimit) + if (writer.GetLength() >= kFlagsStringLineLimit) { otLogInfoCore("Notifier: StateChanged (0x%08x) %s%s ...", aEvents.GetAsFlags(), didLog ? "... " : "[", string.AsCString()); - string.Clear(); + writer.Clear(); didLog = true; addSpace = false; } - IgnoreError(string.Append("%s%s", addSpace ? " " : "", EventToString(static_cast(1 << bit)))); + writer.Append("%s%s", addSpace ? " " : "", EventToString(static_cast(1 << bit))); addSpace = true; flags ^= (1 << bit); diff --git a/src/core/common/string.cpp b/src/core/common/string.cpp index b787ee5d7..970d92552 100644 --- a/src/core/common/string.cpp +++ b/src/core/common/string.cpp @@ -32,6 +32,7 @@ */ #include "string.hpp" +#include "debug.hpp" #include @@ -72,30 +73,56 @@ bool StringEndsWith(const char *aString, char aChar) return len > 0 && aString[len - 1] == aChar; } -Error StringBase::Write(char *aBuffer, uint16_t aSize, uint16_t &aLength, const char *aFormat, va_list aArgs) +StringWriter::StringWriter(char *aBuffer, uint16_t aSize) + : mBuffer(aBuffer) + , mLength(0) + , mSize(aSize) { - Error error = kErrorNone; - int len; + mBuffer[0] = '\0'; +} - len = vsnprintf(aBuffer + aLength, aSize - aLength, aFormat, aArgs); +StringWriter &StringWriter::Clear(void) +{ + mBuffer[0] = '\0'; + mLength = 0; + return *this; +} - if (len < 0) +StringWriter &StringWriter::Append(const char *aFormat, ...) +{ + va_list args; + va_start(args, aFormat); + AppendVarArgs(aFormat, args); + va_end(args); + + return *this; +} + +StringWriter &StringWriter::AppendVarArgs(const char *aFormat, va_list aArgs) +{ + int len; + + len = vsnprintf(mBuffer + mLength, (mSize > mLength ? (mSize - mLength) : 0), aFormat, aArgs); + OT_ASSERT(len >= 0); + + mLength += static_cast(len); + + if (IsTruncated()) { - aLength = 0; - aBuffer[0] = 0; - error = kErrorInvalidArgs; - } - else if (len >= aSize - aLength) - { - aLength = aSize - 1; - error = kErrorNoBufs; - } - else - { - aLength += static_cast(len); + mBuffer[mSize - 1] = '\0'; } - return error; + return *this; +} + +StringWriter &StringWriter::AppendHexBytes(const uint8_t *aBytes, uint16_t aLength) +{ + while (aLength--) + { + Append("%02x", *aBytes++); + } + + return *this; } bool IsValidUtf8String(const char *aString) diff --git a/src/core/common/string.hpp b/src/core/common/string.hpp index 48f626643..4b355038b 100644 --- a/src/core/common/string.hpp +++ b/src/core/common/string.hpp @@ -90,79 +90,75 @@ const char *StringFind(const char *aString, char aChar); */ bool StringEndsWith(const char *aString, char aChar); -/** - * This class defines the base class for `String`. - * - */ -class StringBase -{ -protected: - /** - * This method appends `printf()` style formatted data to a given character string buffer. - * - * @param[in] aBuffer A pointer to buffer containing the string. - * @param[in] aSize The size of the buffer (in bytes). - * @param[inout] aLength A reference to variable containing current length of string. On exit length is updated. - * @param[in] aFormat A pointer to the format string. - * @param[in] aArgs Arguments for the format specification. - * - * @retval kErrorNone Updated the string successfully. - * @retval kErrorNoBufs String could not fit in the storage. - * @retval kErrorInvalidArgs Arguments do not match the format string. - */ - static Error Write(char *aBuffer, uint16_t aSize, uint16_t &aLength, const char *aFormat, va_list aArgs); -}; +class StringWriter; /** * This class defines a fixed-size string. * */ -template class String : private StringBase +template class String +{ + friend class StringWriter; + + static_assert(kSize > 0, "String buffer cannot be empty."); + +public: + /** + * This method returns the string as a null-terminated C string. + * + * @returns The null-terminated C string. + * + */ + const char *AsCString(void) const { return mBuffer; } + +private: + char mBuffer[kSize]; +}; + +/** + * This class implements writing to a string buffer. + * + */ +class StringWriter { public: - enum - { - kSize = SIZE, ///< Size (number of characters) in the buffer storage for the string. - }; - /** - * This constructor initializes the `String` object as empty. + * This constructor initializes the object as cleared on the provided buffer. * */ - String(void) - : mLength(0) + StringWriter(char *aBuffer, uint16_t aSize); + + /** + * This constructor initializes the object as cleared on a fixed-size string. + * + */ + template + explicit StringWriter(String &aStringBuffer) + : StringWriter(aStringBuffer.mBuffer, kSize) { - mBuffer[0] = 0; } /** - * This constructor initializes the `String` object using `printf()` style formatted data. + * This method clears the string writer. * - * @param[in] aFormat A pointer to the format string. - * @param[in] ... Arguments for the format specification. + * @returns The string writer. * */ - explicit String(const char *aFormat, ...) - : mLength(0) - { - va_list args; - va_start(args, aFormat); - IgnoreError(Write(mBuffer, kSize, mLength, aFormat, args)); - va_end(args); - } + StringWriter &Clear(void); /** - * This method clears the string. + * This method returns whether the output is truncated. + * + * @note If the output is truncated, the buffer is still null-terminated. + * + * @retval true The output is truncated. + * @retval false The output is not truncated. * */ - void Clear(void) - { - mBuffer[0] = 0; - mLength = 0; - } + bool IsTruncated(void) const { return mLength >= mSize; } /** - * This method gets the length of the string. + * This method gets the length of the wanted string. * * Similar to `strlen()` the length does not include the null character at the end of the string. * @@ -172,107 +168,50 @@ public: uint16_t GetLength(void) const { return mLength; } /** - * This method returns the size (number of chars) in the buffer storage for the string. + * This method returns the size (number of chars) in the buffer. * - * @returns The size of the buffer storage for the string. + * @returns The size of the buffer. * */ - uint16_t GetSize(void) const { return kSize; } + uint16_t GetSize(void) const { return mSize; } /** - * This method returns the string as a null-terminated C string. - * - * @returns The null-terminated C string. - * - */ - const char *AsCString(void) const { return mBuffer; } - - /** - * This method sets the string using `printf()` style formatted data. + * This method appends `printf()` style formatted data to the buffer. * * @param[in] aFormat A pointer to the format string. * @param[in] ... Arguments for the format specification. * - * @retval kErrorNone Updated the string successfully. - * @retval kErrorNoBufs String could not fit in the storage. - * @retval kErrorInvalidArgs Arguments do not match the format string. + * @returns The string writer. * */ - Error Set(const char *aFormat, ...) - { - va_list args; - Error error; - - va_start(args, aFormat); - mLength = 0; - error = Write(mBuffer, kSize, mLength, aFormat, args); - va_end(args); - - return error; - } + StringWriter &Append(const char *aFormat, ...); /** - * This method appends `printf()` style formatted data to the `String` object. - * - * @param[in] aFormat A pointer to the format string. - * @param[in] ... Arguments for the format specification. - * - * @retval kErrorNone Updated the string successfully. - * @retval kErrorNoBufs String could not fit in the storage. - * @retval kErrorInvalidArgs Arguments do not match the format string. - * - */ - Error Append(const char *aFormat, ...) - { - va_list args; - Error error; - - va_start(args, aFormat); - error = Write(mBuffer, kSize, mLength, aFormat, args); - va_end(args); - - return error; - } - - /** - * This method appends `printf()` style formatted data to the `String` object. + * This method appends `printf()` style formatted data to the buffer. * * @param[in] aFormat A pointer to the format string. * @param[in] aArgs Arguments for the format specification (as `va_list`). * - * @retval kErrorNone Updated the string successfully. - * @retval kErrorNoBufs String could not fit in the storage. - * @retval kErrorInvalidArgs Arguments do not match the format string. + * @returns The string writer. * */ - Error AppendVarArgs(const char *aFormat, va_list aArgs) { return Write(mBuffer, kSize, mLength, aFormat, aArgs); } + StringWriter &AppendVarArgs(const char *aFormat, va_list aArgs); /** - * This method appends an array of bytes in hex representation (using "%02x" style) to the `String` object. + * This method appends an array of bytes in hex representation (using "%02x" style) to the buffer. * * @param[in] aBytes A pointer to buffer containing the bytes to append. * @param[in] aLength The length of @p aBytes buffer (in bytes). * - * @retval kErrorNone Updated the string successfully. - * @retval kErrorNoBufs String could not fit in the storage. + * @returns The string writer. * */ - Error AppendHexBytes(const uint8_t *aBytes, uint16_t aLength) - { - Error error = kErrorNone; - - while (aLength--) - { - SuccessOrExit(error = Append("%02x", *aBytes++)); - } - - exit: - return error; - } + StringWriter &AppendHexBytes(const uint8_t *aBytes, uint16_t aLength); private: - uint16_t mLength; - char mBuffer[kSize]; + char * mBuffer; + uint16_t mLength; + const uint16_t mSize; }; /** diff --git a/src/core/mac/channel_mask.cpp b/src/core/mac/channel_mask.cpp index ac787c101..3b490140e 100644 --- a/src/core/mac/channel_mask.cpp +++ b/src/core/mac/channel_mask.cpp @@ -95,12 +95,13 @@ exit: ChannelMask::InfoString ChannelMask::ToString(void) const { - InfoString string; - uint8_t channel = kChannelIteratorFirst; - bool addComma = false; - Error error; + InfoString string; + StringWriter writer(string); + uint8_t channel = kChannelIteratorFirst; + bool addComma = false; + Error error; - IgnoreError(string.Append("{")); + writer.Append("{"); error = GetNextChannel(channel); @@ -119,16 +120,16 @@ ChannelMask::InfoString ChannelMask::ToString(void) const rangeEnd = channel; } - IgnoreError(string.Append("%s%d", addComma ? ", " : " ", rangeStart)); + writer.Append("%s%d", addComma ? ", " : " ", rangeStart); addComma = true; if (rangeStart < rangeEnd) { - IgnoreError(string.Append("%s%d", rangeEnd == rangeStart + 1 ? ", " : "-", rangeEnd)); + writer.Append("%s%d", rangeEnd == rangeStart + 1 ? ", " : "-", rangeEnd); } } - IgnoreError(string.Append(" }")); + writer.Append(" }"); return string; } diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index 798c7786c..8c674e04d 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -1381,26 +1381,27 @@ exit: Frame::InfoString Frame::ToInfoString(void) const { - InfoString string; - uint8_t commandId, type; - Address src, dst; + InfoString string; + StringWriter writer(string); + uint8_t commandId, type; + Address src, dst; - IgnoreError(string.Append("len:%d, seqnum:%d, type:", mLength, GetSequence())); + writer.Append("len:%d, seqnum:%d, type:", mLength, GetSequence()); type = GetType(); switch (type) { case kFcfFrameBeacon: - IgnoreError(string.Append("Beacon")); + writer.Append("Beacon"); break; case kFcfFrameData: - IgnoreError(string.Append("Data")); + writer.Append("Data"); break; case kFcfFrameAck: - IgnoreError(string.Append("Ack")); + writer.Append("Ack"); break; case kFcfFrameMacCmd: @@ -1412,34 +1413,33 @@ Frame::InfoString Frame::ToInfoString(void) const switch (commandId) { case kMacCmdDataRequest: - IgnoreError(string.Append("Cmd(DataReq)")); + writer.Append("Cmd(DataReq)"); break; case kMacCmdBeaconRequest: - IgnoreError(string.Append("Cmd(BeaconReq)")); + writer.Append("Cmd(BeaconReq)"); break; default: - IgnoreError(string.Append("Cmd(%d)", commandId)); + writer.Append("Cmd(%d)", commandId); break; } break; default: - IgnoreError(string.Append("%d", type)); + writer.Append("%d", type); break; } IgnoreError(GetSrcAddr(src)); IgnoreError(GetDstAddr(dst)); - IgnoreError(string.Append(", src:%s, dst:%s, sec:%s, ackreq:%s", src.ToString().AsCString(), - dst.ToString().AsCString(), GetSecurityEnabled() ? "yes" : "no", - GetAckRequest() ? "yes" : "no")); + writer.Append(", src:%s, dst:%s, sec:%s, ackreq:%s", src.ToString().AsCString(), dst.ToString().AsCString(), + GetSecurityEnabled() ? "yes" : "no", GetAckRequest() ? "yes" : "no"); #if OPENTHREAD_CONFIG_MULTI_RADIO - IgnoreError(string.Append(", radio:%s", RadioTypeToString(GetRadioType()))); + writer.Append(", radio:%s", RadioTypeToString(GetRadioType())); #endif return string; @@ -1448,12 +1448,14 @@ Frame::InfoString Frame::ToInfoString(void) const BeaconPayload::InfoString BeaconPayload::ToInfoString(void) const { NetworkName name; + InfoString string; IgnoreError(name.Set(GetNetworkName())); - return InfoString("name:%s, xpanid:%s, id:%d, ver:%d, joinable:%s, native:%s", name.GetAsCString(), - mExtendedPanId.ToString().AsCString(), GetProtocolId(), GetProtocolVersion(), - IsJoiningPermitted() ? "yes" : "no", IsNative() ? "yes" : "no"); + StringWriter(string).Append("name:%s, xpanid:%s, id:%d, ver:%d, joinable:%s, native:%s", name.GetAsCString(), + mExtendedPanId.ToString().AsCString(), GetProtocolId(), GetProtocolVersion(), + IsJoiningPermitted() ? "yes" : "no", IsNative() ? "yes" : "no"); + return string; } #endif // #if (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_NOTE) && (OPENTHREAD_CONFIG_LOG_MAC == 1) diff --git a/src/core/mac/mac_types.cpp b/src/core/mac/mac_types.cpp index 6761d3507..8349bba72 100644 --- a/src/core/mac/mac_types.cpp +++ b/src/core/mac/mac_types.cpp @@ -67,7 +67,7 @@ ExtAddress::InfoString ExtAddress::ToString(void) const { InfoString string; - IgnoreError(string.AppendHexBytes(m8, sizeof(ExtAddress))); + StringWriter(string).AppendHexBytes(m8, sizeof(ExtAddress)); return string; } @@ -92,15 +92,29 @@ void ExtAddress::CopyAddress(uint8_t *aDst, const uint8_t *aSrc, CopyByteOrder a Address::InfoString Address::ToString(void) const { - return (mType == kTypeExtended) ? GetExtended().ToString() - : (mType == kTypeNone ? InfoString("None") : InfoString("0x%04x", GetShort())); + InfoString string; + + if (mType == kTypeExtended) + { + string = GetExtended().ToString(); + } + else if (mType == kTypeNone) + { + StringWriter(string).Append("None"); + } + else + { + StringWriter(string).Append("0x%04x", GetShort()); + } + + return string; } ExtendedPanId::InfoString ExtendedPanId::ToString(void) const { InfoString string; - IgnoreError(string.AppendHexBytes(m8, sizeof(ExtendedPanId))); + StringWriter(string).AppendHexBytes(m8, sizeof(ExtendedPanId)); return string; } @@ -203,13 +217,15 @@ void RadioTypes::AddAll(void) RadioTypes::InfoString RadioTypes::ToString(void) const { - InfoString string("{"); - bool addComma = false; + InfoString string; + StringWriter writer(string); + bool addComma = false; + writer.Append("{"); #if OPENTHREAD_CONFIG_RADIO_LINK_IEEE_802_15_4_ENABLE if (Contains(kRadioTypeIeee802154)) { - IgnoreError(string.Append("%s%s", addComma ? ", " : " ", RadioTypeToString(kRadioTypeIeee802154))); + writer.Append("%s%s", addComma ? ", " : " ", RadioTypeToString(kRadioTypeIeee802154)); addComma = true; } #endif @@ -217,14 +233,14 @@ RadioTypes::InfoString RadioTypes::ToString(void) const #if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE if (Contains(kRadioTypeTrel)) { - IgnoreError(string.Append("%s%s", addComma ? ", " : " ", RadioTypeToString(kRadioTypeTrel))); + writer.Append("%s%s", addComma ? ", " : " ", RadioTypeToString(kRadioTypeTrel)); addComma = true; } #endif OT_UNUSED_VARIABLE(addComma); - IgnoreError(string.Append(" }")); + writer.Append(" }"); return string; } diff --git a/src/core/meshcop/meshcop.cpp b/src/core/meshcop/meshcop.cpp index b0120db57..0995fe1cf 100644 --- a/src/core/meshcop/meshcop.cpp +++ b/src/core/meshcop/meshcop.cpp @@ -164,22 +164,23 @@ bool JoinerDiscerner::operator==(const JoinerDiscerner &aOther) const JoinerDiscerner::InfoString JoinerDiscerner::ToString(void) const { - InfoString str; + InfoString str; + StringWriter writer(str); if (mLength <= sizeof(uint16_t) * CHAR_BIT) { - IgnoreError(str.Set("0x%04x", static_cast(mValue))); + writer.Append("0x%04x", static_cast(mValue)); } else if (mLength <= sizeof(uint32_t) * CHAR_BIT) { - IgnoreError(str.Set("0x%08x", static_cast(mValue))); + writer.Append("0x%08x", static_cast(mValue)); } else { - IgnoreError(str.Set("0x%x-%08x", static_cast(mValue >> 32), static_cast(mValue))); + writer.Append("0x%x-%08x", static_cast(mValue >> 32), static_cast(mValue)); } - IgnoreError(str.Append("/len:%d", mLength)); + writer.Append("/len:%d", mLength); return str; } diff --git a/src/core/net/ip4_address.cpp b/src/core/net/ip4_address.cpp index f291113dd..3e2ceda5a 100644 --- a/src/core/net/ip4_address.cpp +++ b/src/core/net/ip4_address.cpp @@ -88,7 +88,10 @@ exit: Address::InfoString Address::ToString(void) const { - return InfoString("%d.%d.%d.%d", mBytes[0], mBytes[1], mBytes[2], mBytes[3]); + InfoString string; + + StringWriter(string).Append("%d.%d.%d.%d", mBytes[0], mBytes[1], mBytes[2], mBytes[3]); + return string; } } // namespace Ip4 diff --git a/src/core/net/ip6_address.cpp b/src/core/net/ip6_address.cpp index c0a35256d..e5b2b0471 100644 --- a/src/core/net/ip6_address.cpp +++ b/src/core/net/ip6_address.cpp @@ -126,20 +126,21 @@ bool Prefix::IsValidNat64(void) const Prefix::InfoString Prefix::ToString(void) const { - InfoString string; - uint8_t sizeInUint16 = (GetBytesSize() + sizeof(uint16_t) - 1) / sizeof(uint16_t); + InfoString string; + StringWriter writer(string); + uint8_t sizeInUint16 = (GetBytesSize() + sizeof(uint16_t) - 1) / sizeof(uint16_t); for (uint16_t i = 0; i < sizeInUint16; i++) { - IgnoreError(string.Append("%s%x", (i > 0) ? ":" : "", HostSwap16(mPrefix.mFields.m16[i]))); + writer.Append("%s%x", (i > 0) ? ":" : "", HostSwap16(mPrefix.mFields.m16[i])); } if (GetBytesSize() < Address::kSize - 1) { - IgnoreError(string.Append("::")); + writer.Append("::"); } - IgnoreError(string.Append("/%d", mLength)); + writer.Append("/%d", mLength); return string; } @@ -248,7 +249,7 @@ InterfaceIdentifier::InfoString InterfaceIdentifier::ToString(void) const { InfoString string; - IgnoreError(string.AppendHexBytes(mFields.m8, kSize)); + StringWriter(string).AppendHexBytes(mFields.m8, kSize); return string; } @@ -629,9 +630,12 @@ exit: Address::InfoString Address::ToString(void) const { - return InfoString("%x:%x:%x:%x:%x:%x:%x:%x", HostSwap16(mFields.m16[0]), HostSwap16(mFields.m16[1]), - HostSwap16(mFields.m16[2]), HostSwap16(mFields.m16[3]), HostSwap16(mFields.m16[4]), - HostSwap16(mFields.m16[5]), HostSwap16(mFields.m16[6]), HostSwap16(mFields.m16[7])); + InfoString string; + + StringWriter(string).Append("%x:%x:%x:%x:%x:%x:%x:%x", HostSwap16(mFields.m16[0]), HostSwap16(mFields.m16[1]), + HostSwap16(mFields.m16[2]), HostSwap16(mFields.m16[3]), HostSwap16(mFields.m16[4]), + HostSwap16(mFields.m16[5]), HostSwap16(mFields.m16[6]), HostSwap16(mFields.m16[7])); + return string; } const Address &Address::GetLinkLocalAllNodesMulticast(void) diff --git a/src/core/net/socket.cpp b/src/core/net/socket.cpp index 5e4414371..5631c0a06 100644 --- a/src/core/net/socket.cpp +++ b/src/core/net/socket.cpp @@ -38,7 +38,10 @@ namespace Ip6 { SockAddr::InfoString SockAddr::ToString(void) const { - return InfoString("[%s]:%u", GetAddress().ToString().AsCString(), GetPort()); + InfoString string; + + StringWriter(string).Append("[%s]:%u", GetAddress().ToString().AsCString(), GetPort()); + return string; } } // namespace Ip6 diff --git a/src/core/radio/trel_packet.cpp b/src/core/radio/trel_packet.cpp index e60f1bfc2..f4d6ee7e4 100644 --- a/src/core/radio/trel_packet.cpp +++ b/src/core/radio/trel_packet.cpp @@ -77,35 +77,35 @@ uint16_t Header::GetSize(Type aType) Header::InfoString Header::ToString(void) const { - Type type = GetType(); - InfoString string; + Type type = GetType(); + InfoString string; + StringWriter writer(string); switch (type) { case kTypeBroadcast: - IgnoreError(string.Set("broadcast ch:%d", GetChannel())); + writer.Append("broadcast ch:%d", GetChannel()); break; case kTypeUnicast: - IgnoreError(string.Set("unicast ch:%d", GetChannel())); + writer.Append("unicast ch:%d", GetChannel()); break; case kTypeAck: - IgnoreError(string.Set("ack")); + writer.Append("ack"); break; } - IgnoreError( - string.Append(" panid:%04x num:%lu src:%s", GetPanId(), GetPacketNumber(), GetSource().ToString().AsCString())); + writer.Append(" panid:%04x num:%lu src:%s", GetPanId(), GetPacketNumber(), GetSource().ToString().AsCString()); if ((type == kTypeUnicast) || (type == kTypeAck)) { - IgnoreError(string.Append(" dst:%s", GetDestination().ToString().AsCString())); + writer.Append(" dst:%s", GetDestination().ToString().AsCString()); } if ((type == kTypeUnicast) || (type == kTypeBroadcast)) { - IgnoreError(string.Append(GetAckMode() == kNoAck ? " no-ack" : " ack-req")); + writer.Append(GetAckMode() == kNoAck ? " no-ack" : " ack-req"); } return string; diff --git a/src/core/thread/link_quality.cpp b/src/core/thread/link_quality.cpp index 7f94f6777..64df6b8ef 100644 --- a/src/core/thread/link_quality.cpp +++ b/src/core/thread/link_quality.cpp @@ -109,7 +109,8 @@ RssAverager::InfoString RssAverager::ToString(void) const InfoString string; VerifyOrExit(mCount != 0); - IgnoreError(string.Set("%d.%s", -(mAverage >> kPrecisionBitShift), kDigitsString[mAverage & kPrecisionBitMask])); + StringWriter(string).Append("%d.%s", -(mAverage >> kPrecisionBitShift), + kDigitsString[mAverage & kPrecisionBitMask]); exit: return string; @@ -166,8 +167,11 @@ uint8_t LinkQualityInfo::GetLinkMargin(void) const LinkQualityInfo::InfoString LinkQualityInfo::ToInfoString(void) const { - return InfoString("aveRss:%s, lastRss:%d, linkQuality:%d", mRssAverager.ToString().AsCString(), GetLastRss(), - GetLinkQuality()); + InfoString string; + + StringWriter(string).Append("aveRss:%s, lastRss:%d, linkQuality:%d", mRssAverager.ToString().AsCString(), + GetLastRss(), GetLinkQuality()); + return string; } uint8_t LinkQualityInfo::ConvertRssToLinkMargin(int8_t aNoiseFloor, int8_t aRss) diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 8c6a26025..00bd582a6 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -4117,7 +4117,7 @@ void Mle::Log(MessageAction aAction, MessageType aType, const Ip6::Address &aAdd if (aRloc != Mac::kShortAddrInvalid) { - IgnoreError(rlocString.Set(",0x%04x", aRloc)); + StringWriter(rlocString).Append(",0x%04x", aRloc); } otLogInfoMle("%s %s%s (%s%s)", MessageActionToString(aAction), MessageTypeToString(aType), diff --git a/src/core/thread/mle_types.cpp b/src/core/thread/mle_types.cpp index 9402b5551..32d55e2d8 100644 --- a/src/core/thread/mle_types.cpp +++ b/src/core/thread/mle_types.cpp @@ -55,8 +55,11 @@ void DeviceMode::Set(const ModeConfig &aModeConfig) DeviceMode::InfoString DeviceMode::ToString(void) const { - return InfoString("rx-on:%s ftd:%s full-net:%s", IsRxOnWhenIdle() ? "yes" : "no", - IsFullThreadDevice() ? "yes" : "no", IsFullNetworkData() ? "yes" : "no"); + InfoString string; + + StringWriter(string).Append("rx-on:%s ftd:%s full-net:%s", IsRxOnWhenIdle() ? "yes" : "no", + IsFullThreadDevice() ? "yes" : "no", IsFullNetworkData() ? "yes" : "no"); + return string; } void MeshLocalPrefix::SetFromExtendedPanId(const Mac::ExtendedPanId &aExtendedPanId) diff --git a/src/core/thread/radio_selector.cpp b/src/core/thread/radio_selector.cpp index b45c44f4c..32086b592 100644 --- a/src/core/thread/radio_selector.cpp +++ b/src/core/thread/radio_selector.cpp @@ -369,8 +369,9 @@ void RadioSelector::Log(otLogLevel aLogLevel, { if (aNeighbor.GetSupportedRadioTypes().Contains(radio)) { - IgnoreError(preferenceString.Append("%s%s:%d", isFirstEntry ? "" : " ", RadioTypeToString(radio), - aNeighbor.GetRadioPreference(radio))); + StringWriter(preferenceString) + .Append("%s%s:%d", isFirstEntry ? "" : " ", RadioTypeToString(radio), + aNeighbor.GetRadioPreference(radio)); isFirstEntry = false; } } diff --git a/src/core/utils/channel_monitor.cpp b/src/core/utils/channel_monitor.cpp index 526789249..ca7bb4987 100644 --- a/src/core/utils/channel_monitor.cpp +++ b/src/core/utils/channel_monitor.cpp @@ -195,7 +195,7 @@ void ChannelMonitor::LogResults(void) for (uint16_t channel : mChannelOccupancy) { - IgnoreError(logString.Append("%02x ", channel >> 8)); + StringWriter(logString).Append("%02x ", channel >> 8); } otLogInfoUtil("ChannelMonitor: %u [%s]", mSampleCount, logString.AsCString()); diff --git a/tests/unit/test_string.cpp b/tests/unit/test_string.cpp index 9f265d356..c1e8489d1 100644 --- a/tests/unit/test_string.cpp +++ b/tests/unit/test_string.cpp @@ -43,76 +43,70 @@ enum template void PrintString(const char *aName, const String aString) { - printf("\t%s = [%d] \"%s\"\n", aName, aString.GetLength(), aString.AsCString()); + printf("\t%s = [%zu] \"%s\"\n", aName, strlen(aString.AsCString()), aString.AsCString()); } -void TestString(void) +void TestStringWriter(void) { - otError error; - String str1; - String str2("abc"); - String str3("%d", 12); + String str; + constexpr char kLongString[] = "abcdefghijklmnopqratuvwxyzabcdefghijklmnopqratuvwxyz"; - printf("\nTest 1: String constructor\n"); + printf("\nTest 1: StringWriter constructor\n"); - VerifyOrQuit(str1.GetSize() == kStringSize, "GetSize() failed"); + VerifyOrQuit(StringWriter(str).GetSize() == kStringSize, "GetSize() failed"); + VerifyOrQuit(StringWriter(str).GetLength() == 0, "GetLength() failed for empty string"); - VerifyOrQuit(str1.GetLength() == 0, "GetLength() failed for empty string"); - VerifyOrQuit(str2.GetLength() == 3, "GetLength() failed"); - VerifyOrQuit(str3.GetLength() == 2, "GetLength() failed"); + VerifyOrQuit(strcmp(str.AsCString(), "") == 0, "String content is incorrect"); - VerifyOrQuit(strcmp(str1.AsCString(), "") == 0, "String content is incorrect"); - VerifyOrQuit(strcmp(str2.AsCString(), "abc") == 0, "String content is incorrect"); - VerifyOrQuit(strcmp(str3.AsCString(), "12") == 0, "String content is incorrect"); - - PrintString("str1", str1); - PrintString("str2", str2); - PrintString("str3", str3); + PrintString("str", str); printf(" -- PASS\n"); - printf("\nTest 2: String::Set() and String::Clear() method\n"); + printf("\nTest 2: String::Append() method\n"); - error = str1.Set("Hello"); - SuccessOrQuit(error, "String::Set() failed unexpectedly"); - VerifyOrQuit(str1.GetLength() == 5, "GetLength() failed for empty string"); - VerifyOrQuit(strcmp(str1.AsCString(), "Hello") == 0, "String content is incorrect"); - PrintString("str1", str1); + { + StringWriter writer(str); - str1.Clear(); - VerifyOrQuit(str1.GetLength() == 0, "GetLength() failed for empty string"); - VerifyOrQuit(strcmp(str1.AsCString(), "") == 0, "String content is incorrect"); + writer.Append("Hi"); + VerifyOrQuit(writer.GetLength() == 2, "GetLength() failed"); + VerifyOrQuit(strcmp(str.AsCString(), "Hi") == 0, "String content is incorrect"); + PrintString("str", str); - IgnoreError(str1.Set("%d", 12)); - VerifyOrQuit(str1.GetLength() == 2, "GetLength() failed"); - VerifyOrQuit(strcmp(str1.AsCString(), "12") == 0, "String content is incorrect"); - PrintString("str1", str1); + writer.Append("%s%d", "!", 12); + VerifyOrQuit(writer.GetLength() == 5, "GetLength() failed"); + VerifyOrQuit(strcmp(str.AsCString(), "Hi!12") == 0, "String content is incorrect"); + PrintString("str", str); - error = str1.Set("abcdefghijklmnopqratuvwxyzabcdefghijklmnopqratuvwxyz"); - VerifyOrQuit(error == OT_ERROR_NO_BUFS, "String::Set() did not handle overflow buffer correctly"); - PrintString("str1", str1); + writer.Append(kLongString); + VerifyOrQuit(writer.IsTruncated() && writer.GetLength() == 5 + sizeof(kLongString) - 1, + "String::Append() did not handle overflow buffer correctly"); + PrintString("str", str); + } - printf("\nTest 3: String::Append() method\n"); + printf("\nTest 3: String::Clear() method\n"); - str2.Clear(); - VerifyOrQuit(str2.GetLength() == 0, "GetLength() failed for empty string"); - VerifyOrQuit(strcmp(str2.AsCString(), "") == 0, "String content is incorrect"); + { + StringWriter writer(str); - error = str2.Append("Hi"); - SuccessOrQuit(error, "String::Append() failed unexpectedly"); - VerifyOrQuit(str2.GetLength() == 2, "GetLength() failed"); - VerifyOrQuit(strcmp(str2.AsCString(), "Hi") == 0, "String content is incorrect"); - PrintString("str2", str2); + writer.Append("Hello"); + VerifyOrQuit(writer.GetLength() == 5, "GetLength() failed for empty string"); + VerifyOrQuit(strcmp(str.AsCString(), "Hello") == 0, "String content is incorrect"); + PrintString("str", str); - error = str2.Append("%s%d", "!", 12); - SuccessOrQuit(error, "String::Append() failed unexpectedly"); - VerifyOrQuit(str2.GetLength() == 5, "GetLength() failed"); - VerifyOrQuit(strcmp(str2.AsCString(), "Hi!12") == 0, "String content is incorrect"); - PrintString("str2", str2); + writer.Clear(); + VerifyOrQuit(writer.GetLength() == 0, "GetLength() failed for empty string"); + VerifyOrQuit(strcmp(str.AsCString(), "") == 0, "String content is incorrect"); - error = str2.Append("abcdefghijklmnopqratuvwxyzabcdefghijklmnopqratuvwxyz"); - VerifyOrQuit(error == OT_ERROR_NO_BUFS, "String::Append() did not handle overflow buffer correctly"); - PrintString("str2", str2); + writer.Append("%d", 12); + VerifyOrQuit(writer.GetLength() == 2, "GetLength() failed"); + VerifyOrQuit(strcmp(str.AsCString(), "12") == 0, "String content is incorrect"); + PrintString("str", str); + + writer.Clear().Append(kLongString); + VerifyOrQuit(writer.IsTruncated() && writer.GetLength() == sizeof(kLongString) - 1, + "String::Clear() + String::Append() did not handle overflow buffer correctly"); + PrintString("str", str); + } printf(" -- PASS\n"); } @@ -179,7 +173,7 @@ void TestStringFind(void) int main(void) { - ot::TestString(); + ot::TestStringWriter(); ot::TestStringLength(); ot::TestUtf8(); ot::TestStringFind();