Use null-terminated SerialNumber (#11462)

* Aligned with FirmwareRevisionString, ProductionRevisionString, ProductName, VendorName which are also null-terminated * Adapt SerialNumber test accordingly
project-chip · Feb 24, 2022 · 75d4ffe · 75d4ffe
1 parent 25e4c1f
commit 75d4ffe
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 24 deletions.
diff --git a/examples/common/pigweed/rpc_services/Device.h b/examples/common/pigweed/rpc_services/Device.h
@@ -103,8 +103,8 @@ class Device : public generated::Device<Device>
             response.pairing_info.discriminator = static_cast<uint32_t>(discriminator);
             response.has_pairing_info           = true;
         }
-        size_t serial_size;
-        DeviceLayer::ConfigurationMgr().GetSerialNumber(response.serial_number, sizeof(response.serial_number), serial_size);
+
+        DeviceLayer::ConfigurationMgr().GetSerialNumber(response.serial_number, sizeof(response.serial_number));
 
         return pw::OkStatus();
     }

diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp
@@ -491,15 +491,13 @@ void DnssdServer::StartServer(chip::Dnssd::CommissioningMode mode)
 CHIP_ERROR DnssdServer::GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t rotatingDeviceIdHexBufferSize)
 {
     char serialNumber[chip::DeviceLayer::ConfigurationManager::kMaxSerialNumberLength + 1];
-    size_t serialNumberSize                = 0;
     uint16_t lifetimeCounter               = 0;
     size_t rotatingDeviceIdValueOutputSize = 0;
 
-    ReturnErrorOnFailure(
-        chip::DeviceLayer::ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber), serialNumberSize));
+    ReturnErrorOnFailure(chip::DeviceLayer::ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber)));
     ReturnErrorOnFailure(chip::DeviceLayer::ConfigurationMgr().GetLifetimeCounter(lifetimeCounter));
     return AdditionalDataPayloadGenerator().generateRotatingDeviceIdAsHexString(
-        lifetimeCounter, serialNumber, serialNumberSize, rotatingDeviceIdHexBuffer, rotatingDeviceIdHexBufferSize,
+        lifetimeCounter, serialNumber, strlen(serialNumber), rotatingDeviceIdHexBuffer, rotatingDeviceIdHexBufferSize,
         rotatingDeviceIdValueOutputSize);
 }
 #endif

diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h
@@ -74,7 +74,7 @@ class ConfigurationManager
     virtual CHIP_ERROR GetProductId(uint16_t & productId)                                           = 0;
     virtual CHIP_ERROR GetProductRevisionString(char * buf, size_t bufSize)                         = 0;
     virtual CHIP_ERROR GetProductRevision(uint16_t & productRev)                                    = 0;
-    virtual CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen)           = 0;
+    virtual CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize)                                  = 0;
     virtual CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf)                                    = 0;
     virtual CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf)                                      = 0;
     virtual CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf)                                    = 0;

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp
@@ -79,22 +79,27 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetFirmwareRevisionString
 }
 
 template <class ImplClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen)
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetSerialNumber(char * buf, size_t bufSize)
 {
     CHIP_ERROR err;
-    err = Impl()->ReadConfigValueStr(ImplClass::kConfigKey_SerialNum, buf, bufSize, serialNumLen);
+    size_t serialNumLen = 0; // without counting null-terminator
+    err                 = Impl()->ReadConfigValueStr(ImplClass::kConfigKey_SerialNum, buf, bufSize, serialNumLen);
+
 #ifdef CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER
     if (CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER[0] != 0 && err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND)
     {
-        VerifyOrExit(sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) <= bufSize, err = CHIP_ERROR_BUFFER_TOO_SMALL);
+        ReturnErrorCodeIf(sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) > bufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
         memcpy(buf, CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER, sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER));
         serialNumLen = sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) - 1;
         err          = CHIP_NO_ERROR;
     }
 #endif // CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER
-    SuccessOrExit(err);
 
-exit:
+    ReturnErrorOnFailure(err);
+
+    ReturnErrorCodeIf(serialNumLen >= bufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
+    ReturnErrorCodeIf(buf[serialNumLen] != 0, CHIP_ERROR_INVALID_STRING_LENGTH);
+
     return err;
 }
 
@@ -469,8 +474,7 @@ void GenericConfigurationManagerImpl<ImplClass>::LogDeviceConfig()
 
     {
         char serialNum[ConfigurationManager::kMaxSerialNumberLength + 1];
-        size_t serialNumLen;
-        err = GetSerialNumber(serialNum, sizeof(serialNum), serialNumLen);
+        err = GetSerialNumber(serialNum, sizeof(serialNum));
         ChipLogProgress(DeviceLayer, "  Serial Number: %s", (err == CHIP_NO_ERROR) ? serialNum : "(not set)");
     }
 

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h
@@ -63,7 +63,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreProductRevision(uint16_t productRev) override;
     CHIP_ERROR GetFirmwareRevisionString(char * buf, size_t bufSize) override;
     CHIP_ERROR GetFirmwareRevision(uint16_t & firmwareRev) override;
-    CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) override;
+    CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize) override;
     CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override;
     CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override;
     CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override;

diff --git a/src/platform/Linux/bluez/Helper.cpp b/src/platform/Linux/bluez/Helper.cpp
@@ -1191,20 +1191,19 @@ static void UpdateAdditionalDataCharacteristic(BluezGattCharacteristic1 * charac
     chip::System::PacketBufferHandle bufferHandle;
 
     char serialNumber[ConfigurationManager::kMaxSerialNumberLength + 1];
-    size_t serialNumberSize  = 0;
     uint16_t lifetimeCounter = 0;
     BitFlags<AdditionalDataFields> additionalDataFields;
 
 #if CHIP_ENABLE_ROTATING_DEVICE_ID
-    err = ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber), serialNumberSize);
+    err = ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber));
     SuccessOrExit(err);
     err = ConfigurationMgr().GetLifetimeCounter(lifetimeCounter);
     SuccessOrExit(err);
 
     additionalDataFields.Set(AdditionalDataFields::RotatingDeviceId);
 #endif
 
-    err = AdditionalDataPayloadGenerator().generateAdditionalDataPayload(lifetimeCounter, serialNumber, serialNumberSize,
+    err = AdditionalDataPayloadGenerator().generateAdditionalDataPayload(lifetimeCounter, serialNumber, strlen(serialNumber),
                                                                          bufferHandle, additionalDataFields);
     SuccessOrExit(err);
 

diff --git a/src/platform/fake/ConfigurationManagerImpl.h b/src/platform/fake/ConfigurationManagerImpl.h
@@ -40,7 +40,7 @@ class ConfigurationManagerImpl final : public ConfigurationManager
     CHIP_ERROR StoreProductRevision(uint16_t productRev) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetFirmwareRevisionString(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetFirmwareRevision(uint16_t & firmwareRev) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
-    CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) 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; }

diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp
@@ -66,25 +66,24 @@ static void TestConfigurationMgr_SerialNumber(nlTestSuite * inSuite, void * inCo
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     char buf[64];
-    size_t serialNumberLen    = 0;
     const char * serialNumber = "89051AAZZ236";
 
     err = ConfigurationMgr().StoreSerialNumber(serialNumber, strlen(serialNumber));
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
 
-    err = ConfigurationMgr().GetSerialNumber(buf, 64, serialNumberLen);
+    err = ConfigurationMgr().GetSerialNumber(buf, 64);
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
 
-    NL_TEST_ASSERT(inSuite, serialNumberLen == strlen(serialNumber));
+    NL_TEST_ASSERT(inSuite, strlen(buf) == 12);
     NL_TEST_ASSERT(inSuite, strcmp(buf, serialNumber) == 0);
 
     err = ConfigurationMgr().StoreSerialNumber(serialNumber, 5);
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
 
-    err = ConfigurationMgr().GetSerialNumber(buf, 64, serialNumberLen);
+    err = ConfigurationMgr().GetSerialNumber(buf, 64);
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
 
-    NL_TEST_ASSERT(inSuite, serialNumberLen == 5);
+    NL_TEST_ASSERT(inSuite, strlen(buf) == 5);
     NL_TEST_ASSERT(inSuite, strcmp(buf, "89051") == 0);
 }