Improvements from pull request comments.

* Add VerifyOrDie to the ConfigurationMgr getter. * Pass a pointer rather than a reference to ConfigurationMgr. * Explicitly default the instance pointer to nullptr. * Move the singleton implementation out of each platform into a shared file.
project-chip · Nov 9, 2021 · b7986bf · b7986bf
1 parent 1dea460
commit b7986bf
Show file tree

Hide file tree

Showing 45 changed files with 86 additions and 200 deletions.
diff --git a/src/controller/tests/TestDevice.cpp b/src/controller/tests/TestDevice.cpp
@@ -45,7 +45,7 @@ using TestTransportMgr = TransportMgr<Transport::UDP>;
 void TestDevice_EstablishSessionDirectly(nlTestSuite * inSuite, void * inContext)
 {
     Platform::MemoryInit();
-    chip::DeviceLayer::SetConfigurationMgr(chip::DeviceLayer::ConfigurationManagerImpl::GetDefaultInstance());
+    chip::DeviceLayer::SetConfigurationMgr(&chip::DeviceLayer::ConfigurationManagerImpl::GetDefaultInstance());
     DeviceTransportMgr transportMgr;
     SessionManager sessionManager;
     ExchangeManager exchangeMgr;

diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h
@@ -166,9 +166,10 @@ extern ConfigurationManager & ConfigurationMgr();
 /**
  * Sets a reference to a ConfigurationManager object.
  *
- * This must be called before any calls to ConfigurationMgr.
+ * This must be called before any calls to ConfigurationMgr. If a nullptr is passed in,
+ * no changes will be made.
  */
-extern void SetConfigurationMgr(ConfigurationManager & configurationManager);
+extern void SetConfigurationMgr(ConfigurationManager * configurationManager);
 
 } // namespace DeviceLayer
 } // namespace chip

diff --git a/src/platform/Ameba/BUILD.gn b/src/platform/Ameba/BUILD.gn
@@ -21,6 +21,7 @@ assert(chip_device_platform == "ameba")
 static_library("Ameba") {
   sources = [
     "../FreeRTOS/SystemTimeSupport.cpp",
+    "../SingletonConfigurationManager.cpp",
     "AmebaConfig.cpp",
     "AmebaConfig.h",
     "BLEManagerImpl.cpp",

diff --git a/src/platform/Ameba/ConfigurationManagerImpl.cpp b/src/platform/Ameba/ConfigurationManagerImpl.cpp
@@ -36,20 +36,6 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/Ameba/PlatformManagerImpl.cpp b/src/platform/Ameba/PlatformManagerImpl.cpp
@@ -57,7 +57,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
 
     CHIP_ERROR err;
 
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     // Make sure the LwIP core lock has been initialized
     err = Internal::InitLwIPCoreLock();

diff --git a/src/platform/Darwin/BUILD.gn b/src/platform/Darwin/BUILD.gn
@@ -34,6 +34,7 @@ static_library("Darwin") {
   sources = [
     "../DeviceSafeQueue.cpp",
     "../DeviceSafeQueue.h",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.cpp",
     "BLEManagerImpl.h",
     "BlePlatformConfig.h",

diff --git a/src/platform/Darwin/ConfigurationManagerImpl.cpp b/src/platform/Darwin/ConfigurationManagerImpl.cpp
@@ -128,20 +128,6 @@ CHIP_ERROR GetMACAddressFromInterfaces(io_iterator_t primaryInterfaceIterator, u
 }
 #endif // TARGET_OS_OSX
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/Darwin/PlatformManagerImpl.cpp b/src/platform/Darwin/PlatformManagerImpl.cpp
@@ -43,7 +43,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
     // Initialize the configuration system.
     err = Internal::PosixConfig::Init();
     SuccessOrExit(err);
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     mRunLoopSem = dispatch_semaphore_create(0);
 

diff --git a/src/platform/EFR32/BUILD.gn b/src/platform/EFR32/BUILD.gn
@@ -26,6 +26,7 @@ if (chip_enable_openthread) {
 static_library("EFR32") {
   sources = [
     "../FreeRTOS/SystemTimeSupport.cpp",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.cpp",
     "BLEManagerImpl.h",
     "BlePlatformConfig.h",

diff --git a/src/platform/EFR32/ConfigurationManagerImpl.cpp b/src/platform/EFR32/ConfigurationManagerImpl.cpp
@@ -34,20 +34,6 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/EFR32/PlatformManagerImpl.cpp b/src/platform/EFR32/PlatformManagerImpl.cpp
@@ -44,7 +44,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
     // Initialize the configuration system.
     err = Internal::EFR32Config::Init();
     SuccessOrExit(err);
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     // Initialize LwIP.
     tcpip_init(NULL, NULL);

diff --git a/src/platform/ESP32/BUILD.gn b/src/platform/ESP32/BUILD.gn
@@ -21,6 +21,7 @@ assert(chip_device_platform == "esp32")
 static_library("ESP32") {
   sources = [
     "../FreeRTOS/SystemTimeSupport.cpp",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.h",
     "CHIPDevicePlatformConfig.h",
     "CHIPDevicePlatformEvent.h",

diff --git a/src/platform/ESP32/ConfigurationManagerImpl.cpp b/src/platform/ESP32/ConfigurationManagerImpl.cpp
@@ -50,20 +50,6 @@ enum
 
 // TODO: Define a Singleton instance of CHIP Group Key Store here (#1266)
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/ESP32/PlatformManagerImpl.cpp b/src/platform/ESP32/PlatformManagerImpl.cpp
@@ -57,7 +57,7 @@ static int app_entropy_source(void * data, unsigned char * output, size_t len, s
 
 CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
 {
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     esp_err_t err;
     // Arrange for CHIP-encapsulated ESP32 errors to be translated to text

diff --git a/src/platform/Linux/BUILD.gn b/src/platform/Linux/BUILD.gn
@@ -36,6 +36,7 @@ static_library("Linux") {
   sources = [
     "../DeviceSafeQueue.cpp",
     "../DeviceSafeQueue.h",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.cpp",
     "BLEManagerImpl.h",
     "BlePlatformConfig.h",

diff --git a/src/platform/Linux/ConfigurationManagerImpl.cpp b/src/platform/Linux/ConfigurationManagerImpl.cpp
@@ -40,20 +40,6 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp
@@ -209,7 +209,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
     // Initialize the configuration system.
     err = Internal::PosixConfig::Init();
     SuccessOrExit(err);
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
     // Call _InitChipStack() on the generic implementation base class
     // to finish the initialization process.
     err = Internal::GenericPlatformManagerImpl_POSIX<PlatformManagerImpl>::_InitChipStack();

diff --git a/src/platform/P6/BUILD.gn b/src/platform/P6/BUILD.gn
@@ -26,6 +26,7 @@ if (chip_enable_openthread) {
 static_library("P6") {
   sources = [
     "../FreeRTOS/SystemTimeSupport.cpp",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.cpp",
     "BLEManagerImpl.h",
     "BlePlatformConfig.h",

diff --git a/src/platform/P6/ConfigurationManagerImpl.cpp b/src/platform/P6/ConfigurationManagerImpl.cpp
@@ -36,20 +36,6 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/P6/PlatformManagerImpl.cpp b/src/platform/P6/PlatformManagerImpl.cpp
@@ -42,7 +42,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
 {
     CHIP_ERROR err;
 
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     // Make sure the LwIP core lock has been initialized
     err = Internal::InitLwIPCoreLock();

diff --git a/src/platform/SingletonConfigurationManager.cpp b/src/platform/SingletonConfigurationManager.cpp
@@ -0,0 +1,53 @@
+/*
+ *
+ *    Copyright (c) 2021 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+/**
+ *    @file
+ *          Implements a getter and setter for a singleton ConfigurationManager object.
+ */
+
+#include <lib/support/CodeUtils.h>
+
+namespace chip {
+namespace DeviceLayer {
+
+class ConfigurationManager;
+
+namespace {
+
+/** Singleton pointer to the ConfigurationManager implementation.
+ */
+ConfigurationManager * gInstance = nullptr;
+
+} // namespace
+
+ConfigurationManager & ConfigurationMgr()
+{
+    VerifyOrDie(gInstance != nullptr);
+    return *gInstance;
+}
+
+void SetConfigurationMgr(ConfigurationManager * configurationManager)
+{
+    if (configurationManager != nullptr)
+    {
+        gInstance = configurationManager;
+    }
+}
+
+} // namespace DeviceLayer
+} // namespace chip
diff --git a/src/platform/Tizen/BUILD.gn b/src/platform/Tizen/BUILD.gn
@@ -26,6 +26,7 @@ static_library("Tizen") {
   sources = [
     "../DeviceSafeQueue.cpp",
     "../DeviceSafeQueue.h",
+    "../SingletonConfigurationManager.cpp",
     "AppPreference.cpp",
     "AppPreference.h",
     "BLEManagerImpl.cpp",

diff --git a/src/platform/Tizen/ConfigurationManagerImpl.cpp b/src/platform/Tizen/ConfigurationManagerImpl.cpp
@@ -37,20 +37,6 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/Tizen/PlatformManagerImpl.cpp b/src/platform/Tizen/PlatformManagerImpl.cpp
@@ -36,7 +36,7 @@ PlatformManagerImpl PlatformManagerImpl::sInstance;
 CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
 {
     ReturnErrorOnFailure(Internal::PosixConfig::Init());
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     return Internal::GenericPlatformManagerImpl_POSIX<PlatformManagerImpl>::_InitChipStack();
 }

diff --git a/src/platform/Zephyr/ConfigurationManagerImpl.cpp b/src/platform/Zephyr/ConfigurationManagerImpl.cpp
@@ -43,20 +43,6 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton pointer to the ConfigurationManager implementation.
- */
-ConfigurationManager * gInstance;
-
-ConfigurationManager & ConfigurationMgr()
-{
-    return *gInstance;
-}
-
-void SetConfigurationMgr(ConfigurationManager & configurationManager)
-{
-    gInstance = &configurationManager;
-}
-
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
     static ConfigurationManagerImpl sInstance;

diff --git a/src/platform/Zephyr/PlatformManagerImpl.cpp b/src/platform/Zephyr/PlatformManagerImpl.cpp
@@ -72,7 +72,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
     // Initialize the configuration system.
     err = Internal::ZephyrConfig::Init();
     SuccessOrExit(err);
-    SetConfigurationMgr(ConfigurationManagerImpl::GetDefaultInstance());
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
 #if !CONFIG_NORDIC_SECURITY_BACKEND
     // Add entropy source based on Zephyr entropy driver

diff --git a/src/platform/android/BUILD.gn b/src/platform/android/BUILD.gn
@@ -28,6 +28,7 @@ static_library("android") {
   sources = [
     "../DeviceSafeQueue.cpp",
     "../DeviceSafeQueue.h",
+    "../SingletonConfigurationManager.cpp",
     "AndroidChipPlatform-JNI.cpp",
     "AndroidConfig.cpp",
     "AndroidConfig.h",