From a6234f277d67deec53bfdabe35ef31a3de1f0c92 Mon Sep 17 00:00:00 2001 From: Cecille Freeman Date: Fri, 10 Sep 2021 20:34:48 -0400 Subject: [PATCH 1/3] Add tests for CHIPMemString. --- src/lib/support/tests/BUILD.gn | 1 + src/lib/support/tests/TestCHIPMemString.cpp | 170 ++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 src/lib/support/tests/TestCHIPMemString.cpp diff --git a/src/lib/support/tests/BUILD.gn b/src/lib/support/tests/BUILD.gn index ee1af161003649..f85bf6c5d3e25f 100644 --- a/src/lib/support/tests/BUILD.gn +++ b/src/lib/support/tests/BUILD.gn @@ -29,6 +29,7 @@ chip_test_suite("tests") { "TestCHIPArgParser.cpp", "TestCHIPCounter.cpp", "TestCHIPMem.cpp", + "TestCHIPMemString.cpp", "TestDefer.cpp", "TestErrorStr.cpp", "TestFixedBufferAllocator.cpp", diff --git a/src/lib/support/tests/TestCHIPMemString.cpp b/src/lib/support/tests/TestCHIPMemString.cpp new file mode 100644 index 00000000000000..f87c4694833971 --- /dev/null +++ b/src/lib/support/tests/TestCHIPMemString.cpp @@ -0,0 +1,170 @@ +/* + * + * Copyright (c) 2021 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. + */ + +/** + * @file + * This file implements a unit test suite for CHIP Memory Management + * code functionality. + * + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace chip; +using namespace chip::Platform; + +// ================================= +// Unit tests +// ================================= + +namespace { +template +struct TestBuffers +{ + char correctSizeBuf[testStrLen + 1]; + char tooSmallBuf[testStrLen]; + char wayTooSmallBuf[1]; + char tooBigBuf[testStrLen + 10]; + void Reset() + { + memset(correctSizeBuf, 0, sizeof(correctSizeBuf)); + memset(tooSmallBuf, 0, sizeof(tooSmallBuf)); + memset(wayTooSmallBuf, 0, sizeof(wayTooSmallBuf)); + memset(tooBigBuf, 0, sizeof(tooBigBuf)); + } + void CheckCorrectness(nlTestSuite * inSuite, const char * testStr) + { + // correctSizeBuf and tooBigBuf should have the complete string. + NL_TEST_ASSERT(inSuite, correctSizeBuf[testStrLen] == '\0'); + NL_TEST_ASSERT(inSuite, tooBigBuf[testStrLen] == '\0'); + NL_TEST_ASSERT(inSuite, strcmp(correctSizeBuf, testStr) == 0); + NL_TEST_ASSERT(inSuite, strcmp(tooBigBuf, testStr) == 0); + + // wayTooSmallBuf is tiny and thus will only have the null terminator + NL_TEST_ASSERT(inSuite, wayTooSmallBuf[0] == '\0'); + + // tooSmallBuf should still have a null terminator on the end + NL_TEST_ASSERT(inSuite, tooSmallBuf[testStrLen - 1] == '\0'); + } +}; + +} // namespace + +static void TestCopyString(nlTestSuite * inSuite, void * inContext) +{ + constexpr char testWord[] = "testytest"; + ByteSpan testWordSpan = ByteSpan(reinterpret_cast(testWord), sizeof(testWord)); + TestBuffers testBuffers; + + // CopyString with explicit size + testBuffers.Reset(); + CopyString(testBuffers.correctSizeBuf, sizeof(testBuffers.correctSizeBuf), testWord); + CopyString(testBuffers.tooSmallBuf, sizeof(testBuffers.tooSmallBuf), testWord); + CopyString(testBuffers.wayTooSmallBuf, sizeof(testBuffers.wayTooSmallBuf), testWord); + CopyString(testBuffers.tooBigBuf, sizeof(testBuffers.tooBigBuf), testWord); + testBuffers.CheckCorrectness(inSuite, testWord); + + // CopyString with array size + testBuffers.Reset(); + CopyString(testBuffers.correctSizeBuf, testWord); + CopyString(testBuffers.tooSmallBuf, testWord); + CopyString(testBuffers.wayTooSmallBuf, testWord); + CopyString(testBuffers.tooBigBuf, testWord); + testBuffers.CheckCorrectness(inSuite, testWord); + + // CopyString with explicit size from ByteSpan + testBuffers.Reset(); + CopyString(testBuffers.correctSizeBuf, sizeof(testBuffers.correctSizeBuf), testWordSpan); + CopyString(testBuffers.tooSmallBuf, sizeof(testBuffers.tooSmallBuf), testWordSpan); + CopyString(testBuffers.wayTooSmallBuf, sizeof(testBuffers.wayTooSmallBuf), testWordSpan); + CopyString(testBuffers.tooBigBuf, sizeof(testBuffers.tooBigBuf), testWordSpan); + testBuffers.CheckCorrectness(inSuite, testWord); + + // CopyString with array size from ByteSpan + testBuffers.Reset(); + CopyString(testBuffers.correctSizeBuf, testWordSpan); + CopyString(testBuffers.tooSmallBuf, testWordSpan); + CopyString(testBuffers.wayTooSmallBuf, testWordSpan); + CopyString(testBuffers.tooBigBuf, testWordSpan); + testBuffers.CheckCorrectness(inSuite, testWord); +} + +static void TestMemoryAllocString(nlTestSuite * inSuite, void * inContext) +{ + constexpr char testStr[] = "testytestString"; + char * allocatedStr = MemoryAllocString(testStr, sizeof(testStr)); + NL_TEST_ASSERT(inSuite, allocatedStr != nullptr); + if (allocatedStr == nullptr) + { + return; + } + NL_TEST_ASSERT(inSuite, strcmp(testStr, allocatedStr) == 0); + MemoryFree(allocatedStr); +} + +static void TestScopedBuffer(nlTestSuite * inSuite, void * inContext) +{ + // Scoped buffer has its own tests that check the memory properly. Here we are just testing that the string is copied in + // properly. + constexpr char testStr[] = "testytestString"; + ScopedMemoryString scopedString = ScopedMemoryString(testStr, sizeof(testStr)); + NL_TEST_ASSERT(inSuite, strcmp(scopedString.Get(), testStr) == 0); +} + +static const nlTest sTests[] = { NL_TEST_DEF("Test CopyString", TestCopyString), + NL_TEST_DEF("Test MemoryAllocString", TestMemoryAllocString), + NL_TEST_DEF("Test ScopedBuffer", TestScopedBuffer), NL_TEST_SENTINEL() }; + +int TestMemString_Setup(void * inContext) +{ + CHIP_ERROR error = MemoryInit(); + if (error != CHIP_NO_ERROR) + return (FAILURE); + return (SUCCESS); +} + +/** + * Tear down the test suite. + */ +int TestMemString_Teardown(void * inContext) +{ + MemoryShutdown(); + return (SUCCESS); +} + +int TestMemString() +{ + nlTestSuite theSuite = { "CHIP Memory Allocation tests", &sTests[0], TestMemString_Setup, TestMemString_Teardown }; + + // Run test suit againt one context. + nlTestRunner(&theSuite, nullptr); + return nlTestRunnerStats(&theSuite); +} + +CHIP_REGISTER_TEST_SUITE(TestMemString) From cd8c94890911324dd160d7dcfed41b59bbcebff8 Mon Sep 17 00:00:00 2001 From: C Freeman Date: Mon, 13 Sep 2021 09:50:02 -0400 Subject: [PATCH 2/3] Update src/lib/support/tests/TestCHIPMemString.cpp Co-authored-by: Boris Zbarsky --- src/lib/support/tests/TestCHIPMemString.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/support/tests/TestCHIPMemString.cpp b/src/lib/support/tests/TestCHIPMemString.cpp index f87c4694833971..8dbe6693e4d8b8 100644 --- a/src/lib/support/tests/TestCHIPMemString.cpp +++ b/src/lib/support/tests/TestCHIPMemString.cpp @@ -79,7 +79,7 @@ struct TestBuffers static void TestCopyString(nlTestSuite * inSuite, void * inContext) { constexpr char testWord[] = "testytest"; - ByteSpan testWordSpan = ByteSpan(reinterpret_cast(testWord), sizeof(testWord)); + ByteSpan testWordSpan = ByteSpan(reinterpret_cast(testWord), sizeof(testWord) - 1); TestBuffers testBuffers; // CopyString with explicit size From 75a8053a2724dd802c206facddb4ce093e84b332 Mon Sep 17 00:00:00 2001 From: Cecille Freeman Date: Mon, 13 Sep 2021 13:30:41 -0400 Subject: [PATCH 3/3] Address review comments Use buf len instead of strlen to make it more clear about what the buffer sizes really mean. --- src/lib/support/tests/TestCHIPMemString.cpp | 32 +++++++++------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/lib/support/tests/TestCHIPMemString.cpp b/src/lib/support/tests/TestCHIPMemString.cpp index 8dbe6693e4d8b8..f887995079f9b8 100644 --- a/src/lib/support/tests/TestCHIPMemString.cpp +++ b/src/lib/support/tests/TestCHIPMemString.cpp @@ -16,13 +16,6 @@ * limitations under the License. */ -/** - * @file - * This file implements a unit test suite for CHIP Memory Management - * code functionality. - * - */ - #include #include #include @@ -44,33 +37,36 @@ using namespace chip::Platform; // ================================= namespace { -template +template struct TestBuffers { - char correctSizeBuf[testStrLen + 1]; - char tooSmallBuf[testStrLen]; + char correctSizeBuf[kTestBufLen]; + char tooSmallBuf[kTestBufLen - 1]; char wayTooSmallBuf[1]; - char tooBigBuf[testStrLen + 10]; + char tooBigBuf[kTestBufLen + 10]; void Reset() { - memset(correctSizeBuf, 0, sizeof(correctSizeBuf)); - memset(tooSmallBuf, 0, sizeof(tooSmallBuf)); - memset(wayTooSmallBuf, 0, sizeof(wayTooSmallBuf)); - memset(tooBigBuf, 0, sizeof(tooBigBuf)); + memset(correctSizeBuf, 1, sizeof(correctSizeBuf)); + memset(tooSmallBuf, 1, sizeof(tooSmallBuf)); + memset(wayTooSmallBuf, 1, sizeof(wayTooSmallBuf)); + memset(tooBigBuf, 1, sizeof(tooBigBuf)); } void CheckCorrectness(nlTestSuite * inSuite, const char * testStr) { // correctSizeBuf and tooBigBuf should have the complete string. - NL_TEST_ASSERT(inSuite, correctSizeBuf[testStrLen] == '\0'); - NL_TEST_ASSERT(inSuite, tooBigBuf[testStrLen] == '\0'); + NL_TEST_ASSERT(inSuite, correctSizeBuf[kTestBufLen - 1] == '\0'); + NL_TEST_ASSERT(inSuite, tooBigBuf[kTestBufLen - 1] == '\0'); NL_TEST_ASSERT(inSuite, strcmp(correctSizeBuf, testStr) == 0); NL_TEST_ASSERT(inSuite, strcmp(tooBigBuf, testStr) == 0); + NL_TEST_ASSERT(inSuite, strlen(correctSizeBuf) == strlen(testStr)); + NL_TEST_ASSERT(inSuite, strlen(tooBigBuf) == strlen(testStr)); // wayTooSmallBuf is tiny and thus will only have the null terminator NL_TEST_ASSERT(inSuite, wayTooSmallBuf[0] == '\0'); // tooSmallBuf should still have a null terminator on the end - NL_TEST_ASSERT(inSuite, tooSmallBuf[testStrLen - 1] == '\0'); + NL_TEST_ASSERT(inSuite, tooSmallBuf[kTestBufLen - 2] == '\0'); + NL_TEST_ASSERT(inSuite, memcmp(tooSmallBuf, testStr, kTestBufLen - 2) == 0); } };