[log] fix uninitialized string use (#6584)

This commit fixes an issue with some of the log related methods where
`String` object may be used without being initialized (this was added
from commit b39f5b6dd and PR #6564).

This commit ensures that `String` variable definitions are always
immediately  followed by a `StringWriter()` definition which then
initializes the string buffer (sets it as empty).

This commit also updates `test_string` unit test.
This commit is contained in:
Abtin Keshavarzian
2021-05-06 19:27:58 -07:00
committed by GitHub
parent e5cdb56a71
commit f4001aa26d
12 changed files with 92 additions and 76 deletions
+6
View File
@@ -103,6 +103,12 @@ template <uint16_t kSize> class String
static_assert(kSize > 0, "String buffer cannot be empty.");
public:
/**
* This method clears the string (sets it to empty).
*
*/
void Clear(void) { mBuffer[0] = '\0'; }
/**
* This method returns the string as a null-terminated C string.
*
+6 -5
View File
@@ -1447,14 +1447,15 @@ Frame::InfoString Frame::ToInfoString(void) const
BeaconPayload::InfoString BeaconPayload::ToInfoString(void) const
{
NetworkName name;
InfoString string;
NetworkName name;
InfoString string;
StringWriter writer(string);
IgnoreError(name.Set(GetNetworkName()));
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");
writer.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;
}
+11 -8
View File
@@ -65,9 +65,10 @@ void ExtAddress::GenerateRandom(void)
ExtAddress::InfoString ExtAddress::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
StringWriter(string).AppendHexBytes(m8, sizeof(ExtAddress));
writer.AppendHexBytes(m8, sizeof(ExtAddress));
return string;
}
@@ -92,19 +93,20 @@ void ExtAddress::CopyAddress(uint8_t *aDst, const uint8_t *aSrc, CopyByteOrder a
Address::InfoString Address::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
if (mType == kTypeExtended)
{
string = GetExtended().ToString();
writer.AppendHexBytes(GetExtended().m8, sizeof(ExtAddress));
}
else if (mType == kTypeNone)
{
StringWriter(string).Append("None");
writer.Append("None");
}
else
{
StringWriter(string).Append("0x%04x", GetShort());
writer.Append("0x%04x", GetShort());
}
return string;
@@ -112,9 +114,10 @@ Address::InfoString Address::ToString(void) const
ExtendedPanId::InfoString ExtendedPanId::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
StringWriter(string).AppendHexBytes(m8, sizeof(ExtendedPanId));
writer.AppendHexBytes(m8, sizeof(ExtendedPanId));
return string;
}
+4 -2
View File
@@ -88,9 +88,11 @@ exit:
Address::InfoString Address::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
writer.Append("%d.%d.%d.%d", mBytes[0], mBytes[1], mBytes[2], mBytes[3]);
StringWriter(string).Append("%d.%d.%d.%d", mBytes[0], mBytes[1], mBytes[2], mBytes[3]);
return string;
}
+8 -6
View File
@@ -247,9 +247,10 @@ bool InterfaceIdentifier::IsAnycastServiceLocator(void) const
InterfaceIdentifier::InfoString InterfaceIdentifier::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
StringWriter(string).AppendHexBytes(mFields.m8, kSize);
writer.AppendHexBytes(mFields.m8, kSize);
return string;
}
@@ -630,11 +631,12 @@ exit:
Address::InfoString Address::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(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]));
writer.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;
}
+4 -2
View File
@@ -38,9 +38,11 @@ namespace Ip6 {
SockAddr::InfoString SockAddr::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
writer.Append("[%s]:%u", GetAddress().ToString().AsCString(), GetPort());
StringWriter(string).Append("[%s]:%u", GetAddress().ToString().AsCString(), GetPort());
return string;
}
+8 -6
View File
@@ -106,11 +106,11 @@ exit:
RssAverager::InfoString RssAverager::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
VerifyOrExit(mCount != 0);
StringWriter(string).Append("%d.%s", -(mAverage >> kPrecisionBitShift),
kDigitsString[mAverage & kPrecisionBitMask]);
writer.Append("%d.%s", -(mAverage >> kPrecisionBitShift), kDigitsString[mAverage & kPrecisionBitMask]);
exit:
return string;
@@ -167,10 +167,12 @@ uint8_t LinkQualityInfo::GetLinkMargin(void) const
LinkQualityInfo::InfoString LinkQualityInfo::ToInfoString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
writer.Append("aveRss:%s, lastRss:%d, linkQuality:%d", mRssAverager.ToString().AsCString(), GetLastRss(),
GetLinkQuality());
StringWriter(string).Append("aveRss:%s, lastRss:%d, linkQuality:%d", mRssAverager.ToString().AsCString(),
GetLastRss(), GetLinkQuality());
return string;
}
+2 -1
View File
@@ -4114,10 +4114,11 @@ void Mle::Log(MessageAction aAction, MessageType aType, const Ip6::Address &aAdd
};
String<kRlocStringSize> rlocString;
StringWriter writer(rlocString);
if (aRloc != Mac::kShortAddrInvalid)
{
StringWriter(rlocString).Append(",0x%04x", aRloc);
writer.Append(",0x%04x", aRloc);
}
otLogInfoMle("%s %s%s (%s%s)", MessageActionToString(aAction), MessageTypeToString(aType),
+5 -3
View File
@@ -55,10 +55,12 @@ void DeviceMode::Set(const ModeConfig &aModeConfig)
DeviceMode::InfoString DeviceMode::ToString(void) const
{
InfoString string;
InfoString string;
StringWriter writer(string);
writer.Append("rx-on:%s ftd:%s full-net:%s", IsRxOnWhenIdle() ? "yes" : "no", IsFullThreadDevice() ? "yes" : "no",
IsFullNetworkData() ? "yes" : "no");
StringWriter(string).Append("rx-on:%s ftd:%s full-net:%s", IsRxOnWhenIdle() ? "yes" : "no",
IsFullThreadDevice() ? "yes" : "no", IsFullNetworkData() ? "yes" : "no");
return string;
}
+3 -3
View File
@@ -361,6 +361,7 @@ void RadioSelector::Log(otLogLevel aLogLevel,
const Neighbor &aNeighbor)
{
String<kRadioPreferenceStringSize> preferenceString;
StringWriter writer(preferenceString);
bool isFirstEntry = true;
VerifyOrExit(otLoggingGetLevel() >= aLogLevel);
@@ -369,9 +370,8 @@ void RadioSelector::Log(otLogLevel aLogLevel,
{
if (aNeighbor.GetSupportedRadioTypes().Contains(radio))
{
StringWriter(preferenceString)
.Append("%s%s:%d", isFirstEntry ? "" : " ", RadioTypeToString(radio),
aNeighbor.GetRadioPreference(radio));
writer.Append("%s%s:%d", isFirstEntry ? "" : " ", RadioTypeToString(radio),
aNeighbor.GetRadioPreference(radio));
isFirstEntry = false;
}
}
+2 -1
View File
@@ -192,10 +192,11 @@ void ChannelMonitor::LogResults(void)
#if (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_INFO) && (OPENTHREAD_CONFIG_LOG_UTIL == 1)
const size_t kStringSize = 128;
String<kStringSize> logString;
StringWriter writer(logString);
for (uint16_t channel : mChannelOccupancy)
{
StringWriter(logString).Append("%02x ", channel >> 8);
writer.Append("%02x ", channel >> 8);
}
otLogInfoUtil("ChannelMonitor: %u [%s]", mSampleCount, logString.AsCString());
+33 -39
View File
@@ -49,12 +49,13 @@ template <uint16_t kSize> void PrintString(const char *aName, const String<kSize
void TestStringWriter(void)
{
String<kStringSize> str;
StringWriter writer(str);
constexpr char kLongString[] = "abcdefghijklmnopqratuvwxyzabcdefghijklmnopqratuvwxyz";
printf("\nTest 1: StringWriter constructor\n");
VerifyOrQuit(StringWriter(str).GetSize() == kStringSize, "GetSize() failed");
VerifyOrQuit(StringWriter(str).GetLength() == 0, "GetLength() failed for empty string");
VerifyOrQuit(writer.GetSize() == kStringSize, "GetSize() failed");
VerifyOrQuit(writer.GetLength() == 0, "GetLength() failed for empty string");
VerifyOrQuit(strcmp(str.AsCString(), "") == 0, "String content is incorrect");
@@ -62,51 +63,44 @@ void TestStringWriter(void)
printf(" -- PASS\n");
printf("\nTest 2: String::Append() method\n");
printf("\nTest 2: StringWriter::Append() method\n");
{
StringWriter writer(str);
writer.Append("Hi");
VerifyOrQuit(writer.GetLength() == 2, "GetLength() failed");
VerifyOrQuit(strcmp(str.AsCString(), "Hi") == 0, "String content is incorrect");
PrintString("str", str);
writer.Append("Hi");
VerifyOrQuit(writer.GetLength() == 2, "GetLength() failed");
VerifyOrQuit(strcmp(str.AsCString(), "Hi") == 0, "String content is incorrect");
PrintString("str", str);
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);
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);
writer.Append(kLongString);
VerifyOrQuit(writer.IsTruncated() && writer.GetLength() == 5 + sizeof(kLongString) - 1,
"String::Append() did not handle overflow buffer correctly");
PrintString("str", str);
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: StringWriter::Clear() method\n");
printf("\nTest 3: String::Clear() method\n");
writer.Clear();
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);
{
StringWriter writer(str);
writer.Clear();
VerifyOrQuit(writer.GetLength() == 0, "GetLength() failed for empty string");
VerifyOrQuit(strcmp(str.AsCString(), "") == 0, "String content is incorrect");
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);
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();
VerifyOrQuit(writer.GetLength() == 0, "GetLength() failed for empty string");
VerifyOrQuit(strcmp(str.AsCString(), "") == 0, "String content is incorrect");
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);
}
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");
}