Skip to content

Commit

Permalink
[nrf fromlist] [app] Fix DeferredAttributePersister memory leak
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Damian-Nordic committed Dec 18, 2023
1 parent d97e091 commit 6c1531c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/app/DeferredAttributePersistenceProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 22 additions & 10 deletions src/lib/support/ScopedBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#pragma once

#include <lib/support/CHIPMem.h>
#include <lib/support/CodeUtils.h>

#include <type_traits>
#include <utility>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -214,15 +221,20 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer<T>
~ScopedMemoryBufferWithSize() { mCount = 0; }

// return the size as count of elements
inline size_t AllocatedSize() const { return mCount; }
size_t AllocatedSize() const { return mCount; }

void Free()
{
mCount = 0;
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;
Expand Down

0 comments on commit 6c1531c

Please sign in to comment.