From dd55b52c899841cf6b0ec9607baf6e6759f2437b Mon Sep 17 00:00:00 2001
From: Damian Krolik <damian.krolik@nordicsemi.no>
Date: Mon, 18 Dec 2023 12:14:14 +0100
Subject: [PATCH] [app] Fix DeferredAttributePerister memory leak

ScopedMemoryBuffer's Release() method was used instead of
Free(). Add CHECK_RETURN_VALUE annotation to the Release()
method to prevent from making such a mistake in the future.

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
---
 .../DeferredAttributePersistenceProvider.cpp  |  2 +-
 src/darwin/Framework/CHIP/MTRBaseDevice.mm    |  4 +--
 src/lib/support/ScopedBuffer.h                | 30 +++++++++++++------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/app/DeferredAttributePersistenceProvider.cpp b/src/app/DeferredAttributePersistenceProvider.cpp
index 62d5c446fe2091..82652792806d5e 100644
--- a/src/app/DeferredAttributePersistenceProvider.cpp
+++ b/src/app/DeferredAttributePersistenceProvider.cpp
@@ -39,7 +39,7 @@ void DeferredAttribute::Flush(AttributePersistenceProvider & persister)
 {
     VerifyOrReturn(IsArmed());
     persister.WriteValue(mPath, ByteSpan(mValue.Get(), mValue.AllocatedSize()));
-    mValue.Release();
+    mValue.Free();
 }
 
 CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue)
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
index d053cc8f007cf8..41c16debd4138f 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -1142,8 +1142,8 @@ - (void)readAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullable)attri
             //
             callback->AdoptReadClient(std::move(readClient));
             callback.release();
-            attributePathParamsList.Release();
-            eventPathParamsList.Release();
+            (void) attributePathParamsList.Release();
+            (void) eventPathParamsList.Release();
             return err;
         });
     std::move(*bridge).DispatchAction(self);
diff --git a/src/lib/support/ScopedBuffer.h b/src/lib/support/ScopedBuffer.h
index 258b7f262935fb..7a3c0aaca5902c 100644
--- a/src/lib/support/ScopedBuffer.h
+++ b/src/lib/support/ScopedBuffer.h
@@ -25,6 +25,7 @@
 #pragma once
 
 #include <lib/support/CHIPMem.h>
+#include <lib/support/CodeUtils.h>
 
 #include <type_traits>
 #include <utility>
@@ -84,10 +85,11 @@ class ScopedMemoryBufferBase
     const void * Ptr() const { return mBuffer; }
 
     /**
-     * Releases the undelying buffer. Buffer stops being managed and will not be
-     * auto-freed.
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
      */
-    void * Release()
+    CHECK_RETURN_VALUE void * Release()
     {
         void * buffer = mBuffer;
         mBuffer       = nullptr;
@@ -139,13 +141,18 @@ class ScopedMemoryBuffer : public Impl::ScopedMemoryBufferBase<MemoryManagement>
 
     static_assert(std::is_trivially_destructible<T>::value, "Destructors won't get run");
 
-    inline T * Get() { return static_cast<T *>(Base::Ptr()); }
-    inline T & operator[](size_t index) { return Get()[index]; }
+    T * Get() { return static_cast<T *>(Base::Ptr()); }
+    T & operator[](size_t index) { return Get()[index]; }
 
-    inline const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
-    inline const T & operator[](size_t index) const { return Get()[index]; }
+    const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
+    const T & operator[](size_t index) const { return Get()[index]; }
 
-    inline T * Release() { return static_cast<T *>(Base::Release()); }
+    /**
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
+     */
+    CHECK_RETURN_VALUE T * Release() { return static_cast<T *>(Base::Release()); }
 
     ScopedMemoryBuffer & Calloc(size_t elementCount)
     {
@@ -222,7 +229,12 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer<T>
         ScopedMemoryBuffer<T>::Free();
     }
 
-    T * Release()
+    /**
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
+     */
+    CHECK_RETURN_VALUE T * Release()
     {
         T * buffer = ScopedMemoryBuffer<T>::Release();
         mCount     = 0;