Skip to content

Commit feb127e

Browse files
chaitanya jandhyalacjandhyala
chaitanya jandhyala
authored and
cjandhyala
committed
fixed review comments, changed functions to taking spans, not separate pointer/length arguments
1 parent 89ad43b commit feb127e

9 files changed

+65
-55
lines changed

‎examples/platform/linux/CommissionableInit.cpp

+14-12
Original file line numberDiff line numberDiff line change
@@ -102,31 +102,33 @@ CHIP_ERROR InitConfigurationManager(ConfigurationManagerImpl & configManager, Li
102102

103103
if (options.vendorName.HasValue())
104104
{
105-
const char * vendorName = options.vendorName.Value().c_str();
106-
configManager.StoreVendorName(vendorName, strlen(vendorName));
105+
chip::Span<const char> vendor_name(options.vendorName.Value().c_str(), options.vendorName.Value().size());
106+
VerifyOrDie(configManager.StoreVendorName(vendor_name) == CHIP_NO_ERROR);
107107
}
108108
if (options.productName.HasValue())
109109
{
110-
const char * productName = options.productName.Value().c_str();
111-
configManager.StoreProductName(productName,strlen(productName));
110+
chip::Span<const char> product_name(options.productName.Value().c_str(), options.productName.Value().size());
111+
VerifyOrDie(configManager.StoreProductName(product_name) == CHIP_NO_ERROR);
112112
}
113113
if (options.hardwareVersionString.HasValue())
114114
{
115-
const char * hardwareVersionString = options.hardwareVersionString.Value().c_str();
116-
configManager.StoreHardwareVersionString(hardwareVersionString,strlen(hardwareVersionString));
115+
chip::Span<const char> hardware_version_string(options.hardwareVersionString.Value().c_str(),
116+
options.hardwareVersionString.Value().size());
117+
VerifyOrDie(configManager.StoreHardwareVersionString(hardware_version_string) == CHIP_NO_ERROR);
117118
}
118119
if (options.softwareVersionString.HasValue())
119120
{
120-
const char * softwareVersionString = options.softwareVersionString.Value().c_str();
121-
configManager.StoreSoftwareVersionString(softwareVersionString,strlen(softwareVersionString));
121+
chip::Span<const char> software_version_string(options.softwareVersionString.Value().c_str(),
122+
options.softwareVersionString.Value().size());
123+
VerifyOrDie(configManager.StoreSoftwareVersionString(software_version_string) == CHIP_NO_ERROR);
122124
}
123-
125+
124126
if (options.serialNumber.HasValue())
125127
{
126-
const char * serialNumber = options.serialNumber.Value().c_str();
127-
configManager.StoreSerialNumber(serialNumber, strlen(serialNumber));
128+
chip::Span<const char> serial_number(options.serialNumber.Value().c_str(), options.serialNumber.Value().size());
129+
VerifyOrDie(configManager.StoreSerialNumber(serial_number) == CHIP_NO_ERROR);
128130
}
129-
131+
130132
return CHIP_NO_ERROR;
131133
}
132134

‎examples/platform/linux/Options.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -215,19 +215,19 @@ const char * sDeviceOptionHelp =
215215
" --product-id <id>\n"
216216
" The Product ID is specified by vendor.\n"
217217
"\n"
218-
" --vendor-name <id>\n"
218+
" --vendor-name <name>\n"
219219
" The vendor name specified by the vendor.\n"
220220
"\n"
221-
" --product-name <id>\n"
221+
" --product-name <name>\n"
222222
" The product name specified by vendor.\n"
223223
"\n"
224-
" --hardware-version-string <id>\n"
224+
" --hardware-version-string <string>\n"
225225
" The hardware version string specified by vendor.\n"
226226
"\n"
227-
" --software-version-string <id>\n"
227+
" --software-version-string <string>\n"
228228
" The software version string specified by vendor.\n"
229229
"\n"
230-
" --serial-number <id>\n"
230+
" --serial-number <serial_number>\n"
231231
" The serial number specified by vendor.\n"
232232
"\n"
233233
" --custom-flow <Standard = 0 | UserActionRequired = 1 | Custom = 2>\n"

‎examples/platform/linux/Options.h

+2
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ struct LinuxDeviceOptions
8080
chip::Optional<std::string> productName;
8181
chip::Optional<std::string> hardwareVersionString;
8282
chip::Optional<std::string> softwareVersionString;
83+
#if defined(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER)
8384
chip::Optional<std::string> serialNumber;
85+
#endif
8486
static LinuxDeviceOptions & GetInstance();
8587
};
8688

‎src/include/platform/ConfigurationManager.h

+5-6
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ class ConfigurationManager
114114
#endif
115115
virtual CHIP_ERROR GetRegulatoryLocation(uint8_t & location) = 0;
116116
virtual CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen) = 0;
117-
virtual CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) = 0;
118117
virtual CHIP_ERROR StoreManufacturingDate(const char * mfgDate, size_t mfgDateLen) = 0;
119118
virtual CHIP_ERROR StoreSoftwareVersion(uint32_t softwareVer) = 0;
120119
virtual CHIP_ERROR StoreHardwareVersion(uint16_t hardwareVer) = 0;
@@ -133,11 +132,11 @@ class ConfigurationManager
133132
virtual CHIP_ERROR SetFailSafeArmed(bool val) = 0;
134133

135134
virtual CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) = 0;
136-
137-
virtual CHIP_ERROR StoreVendorName(const char * vendorName, size_t vendorNameLen) = 0;
138-
virtual CHIP_ERROR StoreProductName(const char * productName, size_t productNameLen) = 0;
139-
virtual CHIP_ERROR StoreHardwareVersionString(const char * hardwareVersionString, size_t hardwareVersionStringLen) = 0;
140-
virtual CHIP_ERROR StoreSoftwareVersionString(const char * softwareVersionString, size_t softwareVersionStringLen) = 0;
135+
virtual CHIP_ERROR StoreSerialNumber(chip::Span<const char> serialNumber) = 0;
136+
virtual CHIP_ERROR StoreVendorName(chip::Span<const char> vendorName) = 0;
137+
virtual CHIP_ERROR StoreProductName(chip::Span<const char> productName) = 0;
138+
virtual CHIP_ERROR StoreHardwareVersionString(chip::Span<const char> hardwareVersionString) = 0;
139+
virtual CHIP_ERROR StoreSoftwareVersionString(chip::Span<const char> softwareVersionString) = 0;
141140

142141
#if CHIP_CONFIG_TEST
143142
virtual void RunUnitTests() = 0;

‎src/include/platform/internal/GenericConfigurationManagerImpl.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
6868
CHIP_ERROR StoreSoftwareVersion(uint32_t softwareVer) override;
6969
CHIP_ERROR GetFirmwareBuildChipEpochTime(System::Clock::Seconds32 & buildTime) override;
7070
CHIP_ERROR SetFirmwareBuildChipEpochTime(System::Clock::Seconds32 buildTime) override;
71-
CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override;
71+
CHIP_ERROR StoreSerialNumber(chip::Span<const char> serialNumber) override;
7272
CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override;
7373
CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override;
7474
CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf) override;
@@ -104,10 +104,10 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
104104
CHIP_ERROR StoreUniqueId(const char * uniqueId, size_t uniqueIdLen) override;
105105
CHIP_ERROR GenerateUniqueId(char * buf, size_t bufSize) override;
106106

107-
CHIP_ERROR StoreVendorName(const char * vendorName, size_t vendorNameLen) override;
108-
CHIP_ERROR StoreProductName(const char * productName, size_t productNameLen) override;
109-
CHIP_ERROR StoreHardwareVersionString(const char * hardwareVersionString, size_t hardwareVersionStringLen) override;
110-
CHIP_ERROR StoreSoftwareVersionString(const char * softwareVersionString, size_t softwareVersionStringLen) override;
107+
CHIP_ERROR StoreVendorName(chip::Span<const char> vendorName) override;
108+
CHIP_ERROR StoreProductName(chip::Span<const char> productName) override;
109+
CHIP_ERROR StoreHardwareVersionString(chip::Span<const char> hardwareVersionString) override;
110+
CHIP_ERROR StoreSoftwareVersionString(chip::Span<const char> softwareVersionString) override;
111111
#if CHIP_CONFIG_TEST
112112
void RunUnitTests() override;
113113
#endif

‎src/include/platform/internal/GenericConfigurationManagerImpl.ipp

+18-15
Original file line numberDiff line numberDiff line change
@@ -351,20 +351,20 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetSecondaryPairingHint
351351
template <class ConfigClass>
352352
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetSoftwareVersionString(char * buf, size_t bufSize)
353353
{
354-
ChipError err = CHIP_NO_ERROR;
354+
ChipError err = CHIP_NO_ERROR;
355355
size_t softwareVersionStringLen = 0; // without counting null-terminator
356356

357357
err = ReadConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, buf, bufSize, softwareVersionStringLen);
358-
#ifdef CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING
358+
359359
if (CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING[0] != 0 && err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND)
360360
{
361361
ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING), CHIP_ERROR_BUFFER_TOO_SMALL);
362362
memcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING, sizeof(CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING));
363363
softwareVersionStringLen = sizeof(CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING) - 1;
364-
err = CHIP_NO_ERROR;
364+
err = CHIP_NO_ERROR;
365365
}
366-
#endif
367-
ReturnErrorOnFailure(err);
366+
367+
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INTERNAL);
368368

369369
ReturnErrorCodeIf(softwareVersionStringLen >= bufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
370370
ReturnErrorCodeIf(buf[softwareVersionStringLen] != 0, CHIP_ERROR_INVALID_STRING_LENGTH);
@@ -373,29 +373,32 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetSoftwareVersionStrin
373373
}
374374

375375
template <class ConfigClass>
376-
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSerialNumber(const char * serialNum, size_t serialNumLen)
376+
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSerialNumber(chip::Span<const char> serialNumber)
377377
{
378-
return WriteConfigValueStr(ConfigClass::kConfigKey_SerialNum, serialNum, serialNumLen);
378+
return WriteConfigValueStr(ConfigClass::kConfigKey_SerialNum, serialNumber.data(), serialNumber.size());
379379
}
380380
template <class ConfigClass>
381-
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreVendorName(const char * vendorName, size_t vendorNameLen)
381+
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreVendorName(chip::Span<const char> vendorName)
382382
{
383-
return WriteConfigValueStr(ConfigClass::kConfigKey_VendorName, vendorName, vendorNameLen);
383+
384+
return WriteConfigValueStr(ConfigClass::kConfigKey_VendorName, vendorName.data(), vendorName.size());
384385
}
385386
template <class ConfigClass>
386-
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreProductName(const char * productName, size_t productNameLen)
387+
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreProductName(chip::Span<const char> productName)
387388
{
388-
return WriteConfigValueStr(ConfigClass::kConfigKey_ProductName, productName, productNameLen);
389+
return WriteConfigValueStr(ConfigClass::kConfigKey_ProductName, productName.data(), productName.size());
389390
}
390391
template <class ConfigClass>
391-
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreHardwareVersionString(const char * hardwareVersionString, size_t hardwareVersionStringLen)
392+
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreHardwareVersionString(chip::Span<const char> hardwareVersionString)
392393
{
393-
return WriteConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, hardwareVersionString, hardwareVersionStringLen);
394+
return WriteConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, hardwareVersionString.data(),
395+
hardwareVersionString.size());
394396
}
395397
template <class ConfigClass>
396-
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSoftwareVersionString(const char * softwareVersionString, size_t softwareVersionStringLen)
398+
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSoftwareVersionString(chip::Span<const char> softwareVersionString)
397399
{
398-
return WriteConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, softwareVersionString, softwareVersionStringLen);
400+
return WriteConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, softwareVersionString.data(),
401+
softwareVersionString.size());
399402
}
400403

401404
template <class ConfigClass>

‎src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp

+11-9
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetProductId(uint16_t
3838
template <class ConfigClass>
3939
CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetVendorName(char * buf, size_t bufSize)
4040
{
41-
ChipError err = CHIP_NO_ERROR;
41+
ChipError err = CHIP_NO_ERROR;
4242
size_t vendorNameLen = 0; // without counting null-terminator
4343

4444
err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_VendorName, buf, bufSize, vendorNameLen);
@@ -48,7 +48,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetVendorName(char *
4848
ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME), CHIP_ERROR_BUFFER_TOO_SMALL);
4949
memcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME, sizeof(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME));
5050
vendorNameLen = sizeof(CHIP_DEVICE_CONFIG_DEVICE_VENDOR_NAME) - 1;
51-
err = CHIP_NO_ERROR;
51+
err = CHIP_NO_ERROR;
5252
}
5353
#endif
5454
ReturnErrorOnFailure(err);
@@ -62,7 +62,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetVendorName(char *
6262
template <class ConfigClass>
6363
CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetProductName(char * buf, size_t bufSize)
6464
{
65-
ChipError err = CHIP_NO_ERROR;
65+
ChipError err = CHIP_NO_ERROR;
6666
size_t productNameLen = 0; // without counting null-terminator
6767

6868
err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_ProductName, buf, bufSize, productNameLen);
@@ -73,7 +73,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetProductName(char *
7373
ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME), CHIP_ERROR_BUFFER_TOO_SMALL);
7474
memcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME, sizeof(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME));
7575
productNameLen = sizeof(CHIP_DEVICE_CONFIG_DEVICE_PRODUCT_NAME) - 1;
76-
err = CHIP_NO_ERROR;
76+
err = CHIP_NO_ERROR;
7777
}
7878
#endif
7979
ReturnErrorOnFailure(err);
@@ -208,18 +208,20 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetHardwareVersion(ui
208208
template <class ConfigClass>
209209
CHIP_ERROR GenericDeviceInstanceInfoProvider<ConfigClass>::GetHardwareVersionString(char * buf, size_t bufSize)
210210
{
211-
ChipError err = CHIP_NO_ERROR;
211+
ChipError err = CHIP_NO_ERROR;
212212
size_t hardwareVersionStringLen = 0; // without counting null-terminator
213213

214-
err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, buf, bufSize, hardwareVersionStringLen);
214+
err = mGenericConfigManager.ReadConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, buf, bufSize,
215+
hardwareVersionStringLen);
215216

216217
#ifdef CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING
217218
if (CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING[0] != 0 && err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND)
218219
{
219220
ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING), CHIP_ERROR_BUFFER_TOO_SMALL);
220-
memcpy(buf, CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING, sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING));
221-
hardwareVersionStringLen = sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) - 1;
222-
err = CHIP_NO_ERROR;
221+
memcpy(buf, CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING,
222+
sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING));
223+
hardwareVersionStringLen = sizeof(CHIP_DEVICE_CONFIG_DEFAULT_DEVICE_HARDWARE_VERSION_STRING) - 1;
224+
err = CHIP_NO_ERROR;
223225
}
224226
#endif
225227
ReturnErrorOnFailure(err);

‎src/platform/fake/ConfigurationManagerImpl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class ConfigurationManagerImpl : public ConfigurationManager
4949
}
5050
CHIP_ERROR GetSoftwareVersion(uint32_t & softwareVer) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
5151
CHIP_ERROR StoreSoftwareVersion(uint32_t softwareVer) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
52-
CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
52+
CHIP_ERROR StoreSerialNumber(chip::Span<const char> serialNumber) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
5353
CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
5454
CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
5555
CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; }

‎src/platform/tests/TestConfigurationMgr.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ TEST_F(TestConfigurationMgr, SerialNumber)
8383

8484
char buf[64];
8585
const char * serialNumber = "89051AAZZ236";
86+
chip::Span<const char> serial_number(serialNumber.Value().c_str(), serialNumber.Value().size());
8687

87-
err = ConfigurationMgr().StoreSerialNumber(serialNumber, strlen(serialNumber));
88+
err = ConfigurationMgr().StoreSerialNumber(serial_number);
8889
EXPECT_EQ(err, CHIP_NO_ERROR);
8990

9091
err = GetDeviceInstanceInfoProvider()->GetSerialNumber(buf, 64);
@@ -93,7 +94,8 @@ TEST_F(TestConfigurationMgr, SerialNumber)
9394
EXPECT_EQ(strlen(buf), 12u);
9495
EXPECT_STREQ(buf, serialNumber);
9596

96-
err = ConfigurationMgr().StoreSerialNumber(serialNumber, 5);
97+
chip::Span<const char> serial_number(serialNumber.Value().c_str(), 5);
98+
err = ConfigurationMgr().StoreSerialNumber(serial_number);
9799
EXPECT_EQ(err, CHIP_NO_ERROR);
98100

99101
err = GetDeviceInstanceInfoProvider()->GetSerialNumber(buf, 64);

0 commit comments

Comments
 (0)