fixed review comments, changed functions to taking spans, not separat…

…e pointer/length arguments

examples/platform/linux/CommissionableInit.cpp

@@ -102,31 +102,33 @@ CHIP_ERROR InitConfigurationManager(ConfigurationManagerImpl & configManager, Li
 
     if (options.vendorName.HasValue())
     {
-        const char * vendorName = options.vendorName.Value().c_str();
-        configManager.StoreVendorName(vendorName, strlen(vendorName));
+        chip::Span<const char> vendor_name(options.vendorName.Value().c_str(), options.vendorName.Value().size());
+        VerifyOrDie(configManager.StoreVendorName(vendor_name) == CHIP_NO_ERROR);
     }
     if (options.productName.HasValue())
     {
-        const char * productName = options.productName.Value().c_str();
-        configManager.StoreProductName(productName,strlen(productName));
+        chip::Span<const char> product_name(options.productName.Value().c_str(), options.productName.Value().size());
+        VerifyOrDie(configManager.StoreProductName(product_name) == CHIP_NO_ERROR);
     }
     if (options.hardwareVersionString.HasValue())
     {
-        const char * hardwareVersionString = options.hardwareVersionString.Value().c_str();
-        configManager.StoreHardwareVersionString(hardwareVersionString,strlen(hardwareVersionString));
+        chip::Span<const char> hardware_version_string(options.hardwareVersionString.Value().c_str(),
+                                                       options.hardwareVersionString.Value().size());
+        VerifyOrDie(configManager.StoreHardwareVersionString(hardware_version_string) == CHIP_NO_ERROR);
     }
     if (options.softwareVersionString.HasValue())
     {
-        const char * softwareVersionString = options.softwareVersionString.Value().c_str();
-        configManager.StoreSoftwareVersionString(softwareVersionString,strlen(softwareVersionString));
+        chip::Span<const char> software_version_string(options.softwareVersionString.Value().c_str(),
+                                                       options.softwareVersionString.Value().size());
+        VerifyOrDie(configManager.StoreSoftwareVersionString(software_version_string) == CHIP_NO_ERROR);
     }
-  
+
     if (options.serialNumber.HasValue())
     {
-        const char * serialNumber = options.serialNumber.Value().c_str();
-        configManager.StoreSerialNumber(serialNumber, strlen(serialNumber));
+        chip::Span<const char> serial_number(options.serialNumber.Value().c_str(), options.serialNumber.Value().size());
+        VerifyOrDie(configManager.StoreSerialNumber(serial_number) == CHIP_NO_ERROR);
     }
-    
+
     return CHIP_NO_ERROR;
 }
 

examples/platform/linux/Options.cpp

@@ -215,19 +215,19 @@ const char * sDeviceOptionHelp =
     "  --product-id <id>\n"
     "       The Product ID is specified by vendor.\n"
     "\n"
-    "  --vendor-name <id>\n"
+    "  --vendor-name <name>\n"
     "       The vendor name specified by the vendor.\n"
     "\n"
-    "  --product-name <id>\n"
+    "  --product-name <name>\n"
     "       The product name specified by vendor.\n"
     "\n"
-    "  --hardware-version-string <id>\n"
+    "  --hardware-version-string <string>\n"
     "       The hardware version string specified by vendor.\n"
     "\n"
-    "  --software-version-string <id>\n"
+    "  --software-version-string <string>\n"
     "       The software version string specified by vendor.\n"
     "\n"
-    "  --serial-number <id>\n"
+    "  --serial-number <serial_number>\n"
     "       The serial number specified by vendor.\n"
     "\n"
     "  --custom-flow <Standard = 0 | UserActionRequired = 1 | Custom = 2>\n"

examples/platform/linux/Options.h

@@ -80,7 +80,9 @@ struct LinuxDeviceOptions
     chip::Optional<std::string> productName;
     chip::Optional<std::string> hardwareVersionString;
     chip::Optional<std::string> softwareVersionString;
+#if defined(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER)
     chip::Optional<std::string> serialNumber;
+#endif
     static LinuxDeviceOptions & GetInstance();
 };
 

src/include/platform/ConfigurationManager.h

@@ -114,7 +114,6 @@ class ConfigurationManager
 #endif
     virtual CHIP_ERROR GetRegulatoryLocation(uint8_t & location)                       = 0;
     virtual CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen)    = 0;
-    virtual CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen)  = 0;
     virtual CHIP_ERROR StoreManufacturingDate(const char * mfgDate, size_t mfgDateLen) = 0;
     virtual CHIP_ERROR StoreSoftwareVersion(uint32_t softwareVer)                      = 0;
     virtual CHIP_ERROR StoreHardwareVersion(uint16_t hardwareVer)                      = 0;
@@ -133,11 +132,11 @@ class ConfigurationManager
     virtual CHIP_ERROR SetFailSafeArmed(bool val)                                      = 0;
 
     virtual CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) = 0;
-
-    virtual CHIP_ERROR StoreVendorName(const char * vendorName, size_t vendorNameLen)  = 0;
-    virtual CHIP_ERROR StoreProductName(const char * productName, size_t productNameLen)  = 0;
-    virtual CHIP_ERROR StoreHardwareVersionString(const char * hardwareVersionString, size_t hardwareVersionStringLen)  = 0;
-    virtual CHIP_ERROR StoreSoftwareVersionString(const char * softwareVersionString, size_t softwareVersionStringLen)  = 0;
+    virtual CHIP_ERROR StoreSerialNumber(chip::Span<const char> serialNumber)                              = 0;
+    virtual CHIP_ERROR StoreVendorName(chip::Span<const char> vendorName)                                  = 0;
+    virtual CHIP_ERROR StoreProductName(chip::Span<const char> productName)                                = 0;
+    virtual CHIP_ERROR StoreHardwareVersionString(chip::Span<const char> hardwareVersionString)            = 0;
+    virtual CHIP_ERROR StoreSoftwareVersionString(chip::Span<const char> softwareVersionString)            = 0;
 
 #if CHIP_CONFIG_TEST
     virtual void RunUnitTests() = 0;

src/include/platform/internal/GenericConfigurationManagerImpl.h

@@ -68,7 +68,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreSoftwareVersion(uint32_t softwareVer) override;
     CHIP_ERROR GetFirmwareBuildChipEpochTime(System::Clock::Seconds32 & buildTime) override;
     CHIP_ERROR SetFirmwareBuildChipEpochTime(System::Clock::Seconds32 buildTime) override;
-    CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override;
+    CHIP_ERROR StoreSerialNumber(chip::Span<const char> serialNumber) override;
     CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override;
     CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override;
     CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf) override;
@@ -104,10 +104,10 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreUniqueId(const char * uniqueId, size_t uniqueIdLen) override;
     CHIP_ERROR GenerateUniqueId(char * buf, size_t bufSize) override;
 
-    CHIP_ERROR StoreVendorName(const char * vendorName, size_t vendorNameLen)  override;
-    CHIP_ERROR StoreProductName(const char * productName, size_t productNameLen)  override;
-    CHIP_ERROR StoreHardwareVersionString(const char * hardwareVersionString, size_t hardwareVersionStringLen)  override;
-    CHIP_ERROR StoreSoftwareVersionString(const char * softwareVersionString, size_t softwareVersionStringLen)  override;
+    CHIP_ERROR StoreVendorName(chip::Span<const char> vendorName) override;
+    CHIP_ERROR StoreProductName(chip::Span<const char> productName) override;
+    CHIP_ERROR StoreHardwareVersionString(chip::Span<const char> hardwareVersionString) override;
+    CHIP_ERROR StoreSoftwareVersionString(chip::Span<const char> softwareVersionString) override;
 #if CHIP_CONFIG_TEST
     void RunUnitTests() override;
 #endif

src/include/platform/internal/GenericConfigurationManagerImpl.ipp

@@ -351,20 +351,20 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetSecondaryPairingHint
 template <class ConfigClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetSoftwareVersionString(char * buf, size_t bufSize)
 {
-    ChipError err       = CHIP_NO_ERROR;
+    ChipError err                   = CHIP_NO_ERROR;
     size_t softwareVersionStringLen = 0; // without counting null-terminator
 
     err = ReadConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, buf, bufSize, softwareVersionStringLen);
-#ifdef CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING
+
     if (CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING[0] != 0 && err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND)
     {
         ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING), CHIP_ERROR_BUFFER_TOO_SMALL);
         memcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING, sizeof(CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING));
         softwareVersionStringLen = sizeof(CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING) - 1;
-        err          = CHIP_NO_ERROR;
+        err                      = CHIP_NO_ERROR;
     }
-#endif
-    ReturnErrorOnFailure(err);
+
+    VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INTERNAL);
 
     ReturnErrorCodeIf(softwareVersionStringLen >= bufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
     ReturnErrorCodeIf(buf[softwareVersionStringLen] != 0, CHIP_ERROR_INVALID_STRING_LENGTH);
@@ -373,29 +373,32 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetSoftwareVersionStrin
 }
 
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSerialNumber(const char * serialNum, size_t serialNumLen)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSerialNumber(chip::Span<const char> serialNumber)
 {
-    return WriteConfigValueStr(ConfigClass::kConfigKey_SerialNum, serialNum, serialNumLen);
+    return WriteConfigValueStr(ConfigClass::kConfigKey_SerialNum, serialNumber.data(), serialNumber.size());
 }
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreVendorName(const char * vendorName, size_t vendorNameLen)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreVendorName(chip::Span<const char> vendorName)
 {
-    return WriteConfigValueStr(ConfigClass::kConfigKey_VendorName, vendorName, vendorNameLen);
+
+    return WriteConfigValueStr(ConfigClass::kConfigKey_VendorName, vendorName.data(), vendorName.size());
 }
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreProductName(const char * productName, size_t productNameLen)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreProductName(chip::Span<const char> productName)
 {
-    return WriteConfigValueStr(ConfigClass::kConfigKey_ProductName, productName, productNameLen);
+    return WriteConfigValueStr(ConfigClass::kConfigKey_ProductName, productName.data(), productName.size());
 }
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreHardwareVersionString(const char * hardwareVersionString, size_t hardwareVersionStringLen)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreHardwareVersionString(chip::Span<const char> hardwareVersionString)
 {
-    return WriteConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, hardwareVersionString, hardwareVersionStringLen);
+    return WriteConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, hardwareVersionString.data(),
+                               hardwareVersionString.size());
 }
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSoftwareVersionString(const char * softwareVersionString, size_t softwareVersionStringLen)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSoftwareVersionString(chip::Span<const char> softwareVersionString)
 {
-    return WriteConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, softwareVersionString, softwareVersionStringLen);
+    return WriteConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, softwareVersionString.data(),
+                               softwareVersionString.size());
 }
 
 template <class ConfigClass>

src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp

@@ -38,7 +38,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetProductId(uint16_t
 template <class ConfigClass>
 CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetVendorName(char * buf, size_t bufSize)
 {
-    ChipError err       = CHIP_NO_ERROR;
+    ChipError err        = CHIP_NO_ERROR;
     size_t vendorNameLen = 0; // without counting null-terminator
 
     err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_VendorName, buf, bufSize, vendorNameLen);
@@ -48,7 +48,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetVendorName(char *
         ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME), CHIP_ERROR_BUFFER_TOO_SMALL);
         memcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME, sizeof(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME));
         vendorNameLen = sizeof(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME) - 1;
-         err          = CHIP_NO_ERROR;
+        err           = CHIP_NO_ERROR;
     }
 #endif
     ReturnErrorOnFailure(err);
@@ -62,7 +62,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetVendorName(char *
 template <class ConfigClass>
 CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetProductName(char * buf, size_t bufSize)
 {
-    ChipError err       = CHIP_NO_ERROR;
+    ChipError err         = CHIP_NO_ERROR;
     size_t productNameLen = 0; // without counting null-terminator
 
     err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_ProductName, buf, bufSize, productNameLen);
@@ -73,7 +73,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetProductName(char *
         ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME), CHIP_ERROR_BUFFER_TOO_SMALL);
         memcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME, sizeof(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME));
         productNameLen = sizeof(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME) - 1;
-        err          = CHIP_NO_ERROR;
+        err            = CHIP_NO_ERROR;
     }
 #endif
     ReturnErrorOnFailure(err);
@@ -208,18 +208,20 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetHardwareVersion(ui
 template <class ConfigClass>
 CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetHardwareVersionString(char * buf, size_t bufSize)
 {
-    ChipError err       = CHIP_NO_ERROR;
+    ChipError err                   = CHIP_NO_ERROR;
     size_t hardwareVersionStringLen = 0; // without counting null-terminator
 
-    err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, buf, bufSize, hardwareVersionStringLen);
+    err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, buf, bufSize,
+                                                   hardwareVersionStringLen);
 
 #ifdef CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING
     if (CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING[0] != 0 && err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND)
     {
         ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING), CHIP_ERROR_BUFFER_TOO_SMALL);
-        memcpy(buf, CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING, sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING));
-        hardwareVersionStringLen = sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) - 1;
-        err          = CHIP_NO_ERROR;
+        memcpy(buf, CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING,
+               sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING));
+        hardwareVersionStringLen = sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING) - 1;
+        err                      = CHIP_NO_ERROR;
     }
 #endif
     ReturnErrorOnFailure(err);

src/platform/fake/ConfigurationManagerImpl.h

@@ -49,7 +49,7 @@ class ConfigurationManagerImpl : public ConfigurationManager
     }
     CHIP_ERROR GetSoftwareVersion(uint32_t & softwareVer) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR StoreSoftwareVersion(uint32_t softwareVer) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
-    CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR StoreSerialNumber(chip::Span<const char> serialNumber) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; }

src/platform/tests/TestConfigurationMgr.cpp

@@ -83,8 +83,9 @@ TEST_F(TestConfigurationMgr, SerialNumber)
 
     char buf[64];
     const char * serialNumber = "89051AAZZ236";
+    chip::Span<const char> serial_number(serialNumber.Value().c_str(), serialNumber.Value().size());
 
-    err = ConfigurationMgr().StoreSerialNumber(serialNumber, strlen(serialNumber));
+    err = ConfigurationMgr().StoreSerialNumber(serial_number);
     EXPECT_EQ(err, CHIP_NO_ERROR);
 
     err = GetDeviceInstanceInfoProvider()->GetSerialNumber(buf, 64);
@@ -93,7 +94,8 @@ TEST_F(TestConfigurationMgr, SerialNumber)
     EXPECT_EQ(strlen(buf), 12u);
     EXPECT_STREQ(buf, serialNumber);
 
-    err = ConfigurationMgr().StoreSerialNumber(serialNumber, 5);
+    chip::Span<const char> serial_number(serialNumber.Value().c_str(), 5);
+    err = ConfigurationMgr().StoreSerialNumber(serial_number);
     EXPECT_EQ(err, CHIP_NO_ERROR);
 
     err = GetDeviceInstanceInfoProvider()->GetSerialNumber(buf, 64);