Address code review comments

project-chip · Jul 29, 2024 · b314b5c · b314b5c
1 parent be87822
commit b314b5c
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 59 deletions.
diff --git a/src/app/MessageDef/MessageDefHelper.cpp b/src/app/MessageDef/MessageDefHelper.cpp
@@ -55,9 +55,9 @@ void PrettyPrintIMBlankLine()
 
     for (uint32_t i = 0; i < gPrettyPrintingDepthLevel; i++)
     {
-        if (gLineBuffer.size() > gCurLineBufferSize)
+        if (CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE > gCurLineBufferSize)
         {
-            size_t sizeLeft = gLineBuffer.size() - gCurLineBufferSize;
+            size_t sizeLeft = CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE - gCurLineBufferSize;
             size_t ret      = (size_t) (snprintf(lineBuffer + gCurLineBufferSize, sizeLeft, "\t"));
             if (ret > 0)
             {
@@ -77,9 +77,9 @@ void PrettyPrintIM(bool aIsNewLine, const char * aFmt, ...)
         PrettyPrintIMBlankLine();
     }
 
-    if (gLineBuffer.size() > gCurLineBufferSize)
+    if (CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE > gCurLineBufferSize)
     {
-        size_t sizeLeft = gLineBuffer.size() - gCurLineBufferSize;
+        size_t sizeLeft = CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE - gCurLineBufferSize;
         size_t ret      = (size_t) (vsnprintf(gLineBuffer.get() + gCurLineBufferSize, sizeLeft, aFmt, args));
         if (ret > 0)
         {

diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -260,12 +260,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
 
     {
         ::chip::app::LogStorageResources logStorageResources[] = {
-            { sDebugEventBuffer.get(), sDebugEventBuffer.size(), ::chip::app::PriorityLevel::Debug },
-            { sInfoEventBuffer.get(), sInfoEventBuffer.size(), ::chip::app::PriorityLevel::Info },
-            { sCritEventBuffer.get(), sCritEventBuffer.size(), ::chip::app::PriorityLevel::Critical }
+            { sDebugEventBuffer.get(), CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE, ::chip::app::PriorityLevel::Debug },
+            { sInfoEventBuffer.get(), CHIP_DEVICE_CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE, ::chip::app::PriorityLevel::Info },
+            { sCritEventBuffer.get(), CHIP_DEVICE_CONFIG_EVENT_LOGGING_CRIT_BUFFER_SIZE, ::chip::app::PriorityLevel::Critical }
         };
 
-        chip::app::EventManagement::GetInstance().Init(&mExchangeMgr, CHIP_NUM_EVENT_LOGGING_BUFFERS, &sLoggingBuffer[0],
+        chip::app::EventManagement::GetInstance().Init(&mExchangeMgr, CHIP_NUM_EVENT_LOGGING_BUFFERS, sLoggingBuffer.get(),
                                                        &logStorageResources[0], &sGlobalEventIdCounter.get(),
                                                        std::chrono::duration_cast<System::Clock::Milliseconds64>(mInitTimestamp));
     }

diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h
@@ -188,16 +188,16 @@
 #endif // CHIP_CONFIG_GLOBALS_NO_DESTRUCT
 
 /**
- *  @def CHIP_CONFIG_DYNAMIC_GLOBALS
+ *  @def CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS
  *
  *  @brief
  *    Whether to use dynamic allocation for chip::Global objects.
  *
  *    The default is to use static allocation.
  */
-#ifndef CHIP_CONFIG_DYNAMIC_GLOBALS
-#define CHIP_CONFIG_DYNAMIC_GLOBALS 0
-#endif // CHIP_CONFIG_DYNAMIC_GLOBALS
+#ifndef CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS
+#define CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS 0
+#endif // CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS
 
 /**
  *  @def CHIP_CONFIG_SHA256_CONTEXT_SIZE

diff --git a/src/lib/core/Global.h b/src/lib/core/Global.h
@@ -85,16 +85,6 @@ class Global
     /// NOT thread-safe, external synchronization is required.
     T & get() { return _get(); }
     T * operator->() { return &_get(); }
-    template <class U = std::remove_extent_t<T>>
-    U & operator[](std::size_t i)
-    {
-        return _get()[i];
-    }
-    template <std::size_t N = std::extent_v<T>>
-    constexpr std::size_t size() const
-    {
-        return N;
-    }
 
     // Globals are not copyable or movable
     Global(const Global &)             = delete;
@@ -107,59 +97,70 @@ class Global
     ~Global()          = default;
 
 private:
+#if CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS
+    std::remove_extent_t<T> * mStorage = nullptr;
+
+    T & _get()
+    {
+        mOnce.call(&create, this);
+        return *reinterpret_cast<T *>(mStorage);
+    }
+
+    static void create(void * value)
+    {
+        auto self      = static_cast<Global<T, OnceStrategy> *>(value);
+        self->mStorage = new T();
+#if !CHIP_CONFIG_GLOBALS_NO_DESTRUCT
+        CHIP_CXA_ATEXIT(&destroy, self->mStorage);
+#endif // CHIP_CONFIG_GLOBALS_NO_DESTRUCT
+    }
+
+    template <typename U = T, std::enable_if_t<std::is_array_v<U>, bool> = true>
+    static void destroy(void * value)
+    {
+        delete[] static_cast<std::remove_extent_t<T> *>(value);
+    }
+
+    template <typename U = T, std::enable_if_t<!std::is_array_v<U>, bool> = true>
+    static void destroy(void * value)
+    {
+        delete static_cast<T *>(value);
+    }
+#else // CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS
     // Zero-initialize everything. We should technically leave mStorage uninitialized,
     // but that can sometimes cause clang to be unable to constant-initialize the object.
-#if CHIP_CONFIG_DYNAMIC_GLOBALS
-    unsigned char * mStorage = nullptr;
-#else
     alignas(T) unsigned char mStorage[sizeof(T)] = {};
-#endif // CHIP_CONFIG_DYNAMIC_GLOBALS
-    OnceStrategy mOnce;
 
     T & _get()
     {
-#if CHIP_CONFIG_DYNAMIC_GLOBALS
-        mOnce.call(&create, &mStorage);
-#else
         mOnce.call(&create, mStorage);
-#endif // CHIP_CONFIG_DYNAMIC_GLOBALS
         return *reinterpret_cast<T *>(mStorage);
     }
+
     static void create(void * value)
     {
-#if CHIP_CONFIG_DYNAMIC_GLOBALS
-        unsigned char ** storage = static_cast<unsigned char **>(value);
-        value = *storage = new unsigned char[sizeof(T)];
-#endif // CHIP_CONFIG_DYNAMIC_GLOBALS
         new (value) T();
 #if !CHIP_CONFIG_GLOBALS_NO_DESTRUCT
         CHIP_CXA_ATEXIT(&destroy, value);
 #endif // CHIP_CONFIG_GLOBALS_NO_DESTRUCT
     }
 
-    template <class U, std::size_t N>
-    static void _destroy(U (*value)[N])
-    {
-        _destroy_array(*value, N);
-    }
-    template <class U>
-    static void _destroy_array(U * array, std::size_t N)
-    {
-        for (std::size_t i = 0; i < N; ++i)
-            array[i].~U();
-    }
-    template <class U>
-    static void _destroy(U * value)
+    template <typename U = T, std::enable_if_t<std::is_array_v<U>, bool> = true>
+    static void destroy(void * value)
     {
-        value->~U();
+        using V = std::remove_extent_t<T>;
+        for (std::size_t i = 0; i != std::extent_v<T>; ++i)
+            (static_cast<V *>(value) + i)->~V();
     }
+
+    template <typename U = T, std::enable_if_t<!std::is_array_v<U>, bool> = true>
     static void destroy(void * value)
     {
-        _destroy(static_cast<T *>(value));
-#if CHIP_CONFIG_DYNAMIC_GLOBALS
-        delete[] static_cast<unsigned char *>(value);
-#endif // CHIP_CONFIG_DYNAMIC_GLOBALS
+        static_cast<T *>(value)->~T();
     }
+#endif // CHIP_CONFIG_DYNAMICALLY_ALLOCATED_GLOBALS
+
+    OnceStrategy mOnce;
 
 #else // CHIP_CONFIG_GLOBALS_LAZY_INIT
 public:

diff --git a/src/platform/Linux/CHIPPlatformConfig.h b/src/platform/Linux/CHIPPlatformConfig.h
@@ -27,12 +27,6 @@
 
 #define CHIP_CONFIG_ABORT() abort()
 
-extern "C" int __cxa_atexit(void (*func)(void *), void * arg, void * d);
-#define CHIP_CXA_ATEXIT(f, p) __cxa_atexit((f), (p), nullptr)
-
-#define CHIP_CONFIG_DYNAMIC_GLOBALS 1
-#define CHIP_CONFIG_GLOBALS_LAZY_INIT 1
-
 using CHIP_CONFIG_PERSISTED_STORAGE_KEY_TYPE = const char *;
 #define CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH 16