From 2ef15a3ec7d212bfa5a8c5f5eca7de85259a8c44 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Wed, 14 Jul 2021 18:26:02 +0200 Subject: [PATCH] Address code review comments --- src/lib/mdns/Advertiser.h | 5 +++-- src/lib/support/FixedBufferAllocator.cpp | 2 +- src/lib/support/FixedBufferAllocator.h | 16 ++++++++-------- .../support/tests/TestFixedBufferAllocator.cpp | 4 ++-- .../GenericThreadStackManagerImpl_OpenThread.cpp | 2 +- .../GenericThreadStackManagerImpl_OpenThread.h | 10 ++++++---- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/lib/mdns/Advertiser.h b/src/lib/mdns/Advertiser.h index 8250df117cce8f..7efc4cb9c49063 100644 --- a/src/lib/mdns/Advertiser.h +++ b/src/lib/mdns/Advertiser.h @@ -26,6 +26,7 @@ #include #include #include +#include namespace chip { namespace Mdns { @@ -104,7 +105,7 @@ class OperationalAdvertisingParameters : public BaseAdvertisingParams mEnd) { - mOutOfMemory = true; + mAnyAllocFailed = true; return nullptr; } diff --git a/src/lib/support/FixedBufferAllocator.h b/src/lib/support/FixedBufferAllocator.h index 9d283e1ddc7e6a..b3ebf98a9b2110 100644 --- a/src/lib/support/FixedBufferAllocator.h +++ b/src/lib/support/FixedBufferAllocator.h @@ -42,9 +42,9 @@ class FixedBufferAllocator void Init(uint8_t * buffer, size_t capacity) { - mBegin = buffer; - mEnd = buffer + capacity; - mOutOfMemory = false; + mBegin = buffer; + mEnd = buffer + capacity; + mAnyAllocFailed = false; } template @@ -80,17 +80,17 @@ class FixedBufferAllocator char * Clone(const char * str); /** - * Returns if any allocation has failed so far. + * Returns whether any allocation has failed so far. */ - bool IsOutOfMemory() const { return mOutOfMemory; } + bool AnyAllocFailed() const { return mAnyAllocFailed; } private: FixedBufferAllocator(const FixedBufferAllocator &) = delete; void operator=(const FixedBufferAllocator &) = delete; - uint8_t * mBegin = nullptr; - uint8_t * mEnd = nullptr; - bool mOutOfMemory = false; + uint8_t * mBegin = nullptr; + uint8_t * mEnd = nullptr; + bool mAnyAllocFailed = false; }; } // namespace chip diff --git a/src/lib/support/tests/TestFixedBufferAllocator.cpp b/src/lib/support/tests/TestFixedBufferAllocator.cpp index 5a8f036a71c03f..f7e3ea81d2a47b 100644 --- a/src/lib/support/tests/TestFixedBufferAllocator.cpp +++ b/src/lib/support/tests/TestFixedBufferAllocator.cpp @@ -55,11 +55,11 @@ void TestOutOfMemory(nlTestSuite * inSuite, void * inContext) // Allocating 16 bytes still works... NL_TEST_ASSERT(inSuite, alloc.Clone(kTestData, 16) != nullptr); - NL_TEST_ASSERT(inSuite, !alloc.IsOutOfMemory()); + NL_TEST_ASSERT(inSuite, !alloc.AnyAllocFailed()); // ...but cannot allocate even one more byte... NL_TEST_ASSERT(inSuite, alloc.Clone(kTestData, 1) == nullptr); - NL_TEST_ASSERT(inSuite, alloc.IsOutOfMemory()); + NL_TEST_ASSERT(inSuite, alloc.AnyAllocFailed()); } const nlTest sTests[] = { NL_TEST_DEF("Test successfull clone", TestClone), NL_TEST_DEF("Test out of memory", TestOutOfMemory), diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp index f7bac636b6a0f8..f99465e033c8e1 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp @@ -1168,7 +1168,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_AddSrpService(c srpService->mService.mNumTxtEntries = static_cast(aTxtEntries.size()); srpService->mService.mTxtEntries = srpService->mTxtEntries; - VerifyOrExit(!alloc.IsOutOfMemory(), error = CHIP_ERROR_BUFFER_TOO_SMALL); + VerifyOrExit(!alloc.AnyAllocFailed(), error = CHIP_ERROR_BUFFER_TOO_SMALL); ChipLogProgress(DeviceLayer, "advertising srp service: %s.%s", srpService->mService.mInstanceName, srpService->mService.mName); error = MapOpenThreadError(otSrpClientAddService(mOTInst, &(srpService->mService))); diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h index 79df49f69a73cf..4e6c472418d095 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h @@ -136,10 +136,12 @@ class GenericThreadStackManagerImpl_OpenThread #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY // Thread supports both operational and commissionable discovery, so buffers sizes must be worst case. - static constexpr size_t kSubTypeMaxNumber = Mdns::kSubTypeMaxNumber; - static constexpr size_t kSubTypeTotalLength = Mdns::kSubTypeTotalLength; - static constexpr size_t kTxtMaxNumber = Mdns::CommissionAdvertisingParameters::kTxtMaxNumber; - static constexpr size_t kTxtTotalValueLength = Mdns::CommissionAdvertisingParameters::kTxtTotalValueSize; + static constexpr size_t kSubTypeMaxNumber = Mdns::kSubTypeMaxNumber; + static constexpr size_t kSubTypeTotalLength = Mdns::kSubTypeTotalLength; + static constexpr size_t kTxtMaxNumber = + std::max(Mdns::CommissionAdvertisingParameters::kTxtMaxNumber, Mdns::OperationalAdvertisingParameters::kTxtMaxNumber); + static constexpr size_t kTxtTotalValueLength = std::max(Mdns::CommissionAdvertisingParameters::kTxtTotalValueSize, + Mdns::OperationalAdvertisingParameters::kTxtTotalValueSize); #else // Thread only supports operational discovery. static constexpr size_t kSubTypeMaxNumber = 0;