Address code review comments

src/lib/mdns/Advertiser.h

@@ -26,6 +26,7 @@
 #include <inet/InetLayer.h>
 #include <lib/support/Span.h>
 #include <support/CHIPMemString.h>
+#include <support/SafeString.h>
 
 namespace chip {
 namespace Mdns {
@@ -104,7 +105,7 @@ class OperationalAdvertisingParameters : public BaseAdvertisingParams<Operationa
 {
 public:
     static constexpr uint8_t kTxtMaxNumber     = 2;
-    static constexpr uint8_t kTxtMaxKeySize    = 3; // "CRI"/"CRA" as possible keys
+    static constexpr uint8_t kTxtMaxKeySize    = MaxStringLength("CRI", "CRA"); // possible keys
     static constexpr uint8_t kTxtMaxValueSize  = std::max({ kTxtRetryIntervalIdleMaxLength, kTxtRetryIntervalActiveMaxLength });
     static constexpr size_t kTxtTotalValueSize = kTxtRetryIntervalIdleMaxLength + kTxtRetryIntervalActiveMaxLength;
 
@@ -137,7 +138,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionA
 {
 public:
     static constexpr uint8_t kTxtMaxNumber  = 9;
-    static constexpr uint8_t kTxtMaxKeySize = 2; // "D"/"VP"/"CM"/"DT"/"DN"/"RI"/"PI"/"PH" as possible keys
+    static constexpr uint8_t kTxtMaxKeySize = MaxStringLength("D", "VP", "CM", "DT", "DN", "RI", "PI", "PH"); // possible keys
     static constexpr uint8_t kTxtMaxValueSize =
         std::max({ kKeyDiscriminatorMaxLength, kKeyVendorProductMaxLength, kKeyAdditionalPairingMaxLength,
                    kKeyCommissioningModeMaxLength, kKeyDeviceTypeMaxLength, kKeyDeviceNameMaxLength, kKeyRotatingIdMaxLength,

src/lib/support/FixedBufferAllocator.cpp

@@ -24,7 +24,7 @@ uint8_t * FixedBufferAllocator::Alloc(size_t count)
 {
     if (mBegin + count > mEnd)
     {
-        mOutOfMemory = true;
+        mAnyAllocFailed = true;
         return nullptr;
     }
 

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 <size_t N>
@@ -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

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),

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -1168,7 +1168,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_AddSrpService(c
     srpService->mService.mNumTxtEntries = static_cast<OtNumTxtEntries>(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)));

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;