Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ThreadOperationalDataset: various bug fixes #34331

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

ksperling-apple
Copy link
Contributor

@ksperling-apple ksperling-apple commented Jul 15, 2024

  • Ensure TLVs read have the correct length
  • Default construct as empty (mLength == 0)
  • Change MakeRoom() size argument to size_t to avoid chance of truncation
  • Check for null in SetNetworkName
  • Use Encoding::BigEndian instead of hand-rolled math where possible
  • Use ReturnErrorOnFailure / VerifyOrReturnError consistently
  • Make tests independent from each other (non-static dataset member)
  • Add more tests

- Ensure TLVs read have the correct length
- Default construct as empty (mLength == 0)
- Change MakeRoom() size argument to size_t to avoid chance of truncation
- Check for null in SetNetworkName
- Use Encoding::BigEndian instead of hand-rolled math
- Use ReturnErrorOnFailure / VerifyOrReturnError consistently
- Make tests independent from each other (non-static dataset member)
- Add more tests
Copy link

github-actions bot commented Jul 15, 2024

PR #34331: Size comparison from 05e4c10 to 25316e8

Full report (52 builds for cc13x4_26x4, cyw30739, efr32, mbed, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 05e4c10 25316e8 change % change
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 798504 798528 24 0.0
RAM 109180 109180 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 816548 816572 24 0.0
RAM 116948 116948 0 0.0
lock-mtd LP_EM_CC1354P10_6 FLASH 808176 808208 32 0.0
RAM 111236 111236 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 760840 760872 32 0.0
RAM 105328 105328 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 746552 746584 32 0.0
RAM 105576 105576 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 667625 667649 24 0.0
RAM 77644 77644 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 687469 687501 32 0.0
RAM 80276 80276 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 687469 687501 32 0.0
RAM 80276 80276 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 644405 644445 40 0.0
RAM 72712 72712 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 609265 609297 32 0.0
RAM 70804 70804 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 628901 628933 32 0.0
RAM 73356 73356 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 628901 628933 32 0.0
RAM 73356 73356 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 625001 625033 32 0.0
RAM 73820 73820 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 644717 644741 24 0.0
RAM 76372 76372 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 644717 644741 24 0.0
RAM 76372 76372 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 593349 593389 40 0.0
RAM 67788 67788 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 613209 613241 32 0.0
RAM 70420 70420 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 613209 613241 32 0.0
RAM 70420 70420 0 0.0
efr32 lighting-app BRD4187C FLASH 924588 924612 24 0.0
RAM 137528 137528 0 0.0
lock-app BRD4338a FLASH 733836 733828 -8 -0.0
RAM 207892 207892 0 0.0
window-app BRD4187C FLASH 1012628 1012652 24 0.0
RAM 129632 129632 0 0.0
mbed lock-app-release cy8cproto_062_4343w FLASH 1502884 1502884 0 0.0
RAM 226648 226648 0 0.0
nxp contact k32w0+release FLASH 576132 576164 32 0.0
RAM 70024 70024 0 0.0
k32w1+release FLASH 591496 591520 24 0.0
RAM 74056 74056 0 0.0
light k32w0+release FLASH 610360 610392 32 0.0
RAM 69500 69500 0 0.0
k32w1+release FLASH 675104 675128 24 0.0
RAM 82816 82816 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1615564 1615564 0 0.0
RAM 209692 209692 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1536436 1536436 0 0.0
RAM 206588 206588 0 0.0
light cy8ckit_062s2_43012 FLASH 1463044 1463044 0 0.0
RAM 199876 199876 0 0.0
lock cy8ckit_062s2_43012 FLASH 1463788 1463788 0 0.0
RAM 224388 224388 0 0.0
qpg lighting-app qpg6105+debug FLASH 651372 651404 32 0.0
RAM 104564 104564 0 0.0
lock-app qpg6105+debug FLASH 611912 611936 24 0.0
RAM 99240 99240 0 0.0
stm32 light STM32WB5MM-DK FLASH 473712 473728 16 0.0
RAM 144196 144196 0 0.0
telink air-quality-sensor-app tlsr9528a_retention FLASH 632926 632956 30 0.0
RAM 50528 50528 0 0.0
all-clusters-app tlsr9118bdk40d FLASH 658818 658818 0 0.0
RAM 148408 148408 0 0.0
all-clusters-minimal-app tlsr9528a FLASH 779090 779120 30 0.0
RAM 113212 113212 0 0.0
bridge-app tlsr9258a FLASH 675922 675952 30 0.0
RAM 95304 95304 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 634510 634540 30 0.0
RAM 50572 50572 0 0.0
light-switch-app-ota-shell-factory-data tlsr9528a FLASH 720370 720400 30 0.0
RAM 77148 77148 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 613924 613924 0 0.0
RAM 144636 144636 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 801676 801706 30 0.0
RAM 103040 103040 0 0.0
lock-app-dfu tlsr9528a FLASH 666340 666356 16 0.0
RAM 69852 69852 0 0.0
ota-requestor-app tlsr9258a FLASH 695252 695282 30 0.0
RAM 95028 95028 0 0.0
pump-app tlsr9518adk80d FLASH 616784 616814 30 0.0
RAM 56952 56952 0 0.0
pump-controller-app tlsr9518adk80d FLASH 607168 607198 30 0.0
RAM 56752 56752 0 0.0
shell tlsr9518adk80d FLASH 466356 466356 0 0.0
RAM 72484 72484 0 0.0
smoke_co_alarm-app tlsr9528a_retention FLASH 641128 641158 30 0.0
RAM 52200 52200 0 0.0
temperature-measurement-app-mars-ota tlsr9518adk80d FLASH 650994 651024 30 0.0
RAM 60388 60388 0 0.0
thermostat tlsr9518adk80d FLASH 626058 626088 30 0.0
RAM 57084 57084 0 0.0
window-covering tlsr9118bdk40d FLASH 519318 519318 0 0.0
RAM 97800 97800 0 0.0
tizen all-clusters-app arm unknown 1584 1584 0 0.0
FLASH 1639268 1639300 32 0.0
RAM 48548 48548 0 0.0
chip-tool-ubsan arm unknown 2384 2384 0 0.0
FLASH 16292958 16293278 320 0.0
RAM 7156248 7156300 52 0.0

Copy link

github-actions bot commented Jul 15, 2024

PR #34331: Size comparison from 05e4c10 to c8d051f

Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 05e4c10 c8d051f change % change
bl602 lighting-app bl602 FLASH 1271432 1271432 0 0.0
RAM 95328 95328 0 0.0
bl602+mfd FLASH 1285946 1285946 0 0.0
RAM 95472 95472 0 0.0
bl602+rpc FLASH 1310648 1310648 0 0.0
RAM 103752 103752 0 0.0
bl702 lighting-app bl702 FLASH 1092522 1092542 20 0.0
RAM 15161 15161 0 0.0
bl702+mfd FLASH 1103216 1103236 20 0.0
RAM 15313 15313 0 0.0
bl702+rpc FLASH 1182332 1182352 20 0.0
RAM 24181 24181 0 0.0
bl706-eth FLASH 875672 875672 0 0.0
RAM 27272 27272 0 0.0
bl706-wifi FLASH 1128098 1128098 0 0.0
RAM 14605 14605 0 0.0
bl702l lighting-app bl702l FLASH 1079408 1079428 20 0.0
RAM 21732 21732 0 0.0
bl702l+mfd FLASH 1090670 1090690 20 0.0
RAM 21892 21892 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 798504 798528 24 0.0
RAM 109180 109180 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 816548 816572 24 0.0
RAM 116948 116948 0 0.0
lock-mtd LP_EM_CC1354P10_6 FLASH 808176 808208 32 0.0
RAM 111236 111236 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 760840 760872 32 0.0
RAM 105328 105328 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 746552 746584 32 0.0
RAM 105576 105576 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 606406 606406 0 0.0
RAM 204508 204508 0 0.0
lock CC3235SF_LAUNCHXL FLASH 651730 651730 0 0.0
RAM 204780 204780 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 667625 667649 24 0.0
RAM 77644 77644 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 687469 687501 32 0.0
RAM 80276 80276 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 687469 687501 32 0.0
RAM 80276 80276 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 644405 644445 40 0.0
RAM 72712 72712 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 609265 609297 32 0.0
RAM 70804 70804 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 628901 628933 32 0.0
RAM 73356 73356 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 628901 628933 32 0.0
RAM 73356 73356 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 625001 625033 32 0.0
RAM 73820 73820 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 644717 644741 24 0.0
RAM 76372 76372 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 644717 644741 24 0.0
RAM 76372 76372 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 593349 593389 40 0.0
RAM 67788 67788 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 613209 613241 32 0.0
RAM 70420 70420 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 613209 613241 32 0.0
RAM 70420 70420 0 0.0
efr32 lighting-app BRD4187C FLASH 924588 924612 24 0.0
RAM 137528 137528 0 0.0
lock-app BRD4338a FLASH 733836 733828 -8 -0.0
RAM 207892 207892 0 0.0
window-app BRD4187C FLASH 1012628 1012652 24 0.0
RAM 129632 129632 0 0.0
esp32 all-clusters-app c3devkit DRAM 90924 90860 -64 -0.1
FLASH 1469484 1470656 1172 0.1
IRAM 75570 75570 0 0.0
m5stack DRAM 117412 117340 -72 -0.1
FLASH 1538415 1539519 1104 0.1
IRAM 125403 125403 0 0.0
linux air-purifier-app debug unknown 4592 4592 0 0.0
FLASH 2531184 2531280 96 0.0
RAM 125112 125112 0 0.0
all-clusters-app debug unknown 5368 5368 0 0.0
FLASH 5592254 5592382 128 0.0
RAM 493240 493240 0 0.0
all-clusters-minimal-app debug unknown 5288 5288 0 0.0
FLASH 5065344 5065440 96 0.0
RAM 235704 235704 0 0.0
bridge-app debug unknown 5256 5256 0 0.0
FLASH 4479584 4479680 96 0.0
RAM 212832 212832 0 0.0
chip-tool debug unknown 5728 5728 0 0.0
FLASH 11797831 11797879 48 0.0
RAM 547618 547618 0 0.0
chip-tool-ipv6only arm64 unknown 20128 20128 0 0.0
FLASH 10908044 10908108 64 0.0
RAM 596616 596616 0 0.0
fabric-admin debug unknown 5616 5616 0 0.0
FLASH 10879159 10879207 48 0.0
RAM 544650 544650 0 0.0
fabric-bridge-app debug unknown 4544 4544 0 0.0
FLASH 4248240 4248656 416 0.0
RAM 198992 199000 8 0.0
lighting-app debug+rpc+ui unknown 5936 5936 0 0.0
FLASH 5383938 5384066 128 0.0
RAM 224136 224136 0 0.0
lock-app debug unknown 5192 5192 0 0.0
FLASH 4546000 4546000 0 0.0
RAM 200456 200456 0 0.0
ota-provider-app debug unknown 4576 4576 0 0.0
FLASH 4198992 4199120 128 0.0
RAM 194544 194544 0 0.0
ota-requestor-app debug unknown 4512 4512 0 0.0
FLASH 4324352 4324448 96 0.0
RAM 199168 199168 0 0.0
shell debug unknown 4112 4112 0 0.0
FLASH 2805709 2805741 32 0.0
RAM 153008 153008 0 0.0
thermostat-no-ble arm64 unknown 9184 9184 0 0.0
FLASH 4169596 4169676 80 0.0
RAM 235840 235840 0 0.0
tv-app debug unknown 5472 5472 0 0.0
FLASH 5627600 5627648 48 0.0
RAM 342120 342120 0 0.0
tv-casting-app debug unknown 5096 5096 0 0.0
FLASH 9967742 9967822 80 0.0
RAM 402936 402936 0 0.0
mbed lock-app-release cy8cproto_062_4343w FLASH 1502884 1502884 0 0.0
RAM 226648 226648 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 882612 882636 24 0.0
RAM 142229 142229 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 953128 953128 0 0.0
RAM 140657 140657 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 828104 828128 24 0.0
RAM 141123 141123 0 0.0
nxp contact k32w0+release FLASH 576132 576164 32 0.0
RAM 70024 70024 0 0.0
k32w1+release FLASH 591496 591520 24 0.0
RAM 74056 74056 0 0.0
light k32w0+release FLASH 610360 610392 32 0.0
RAM 69500 69500 0 0.0
k32w1+release FLASH 675104 675128 24 0.0
RAM 82816 82816 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1615564 1615564 0 0.0
RAM 209692 209692 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1536436 1536436 0 0.0
RAM 206588 206588 0 0.0
light cy8ckit_062s2_43012 FLASH 1463044 1463044 0 0.0
RAM 199876 199876 0 0.0
lock cy8ckit_062s2_43012 FLASH 1463788 1463788 0 0.0
RAM 224388 224388 0 0.0
qpg lighting-app qpg6105+debug FLASH 651372 651404 32 0.0
RAM 104564 104564 0 0.0
lock-app qpg6105+debug FLASH 611912 611936 24 0.0
RAM 99240 99240 0 0.0
stm32 light STM32WB5MM-DK FLASH 473712 473728 16 0.0
RAM 144196 144196 0 0.0
telink air-quality-sensor-app tlsr9528a_retention FLASH 632926 632956 30 0.0
RAM 50528 50528 0 0.0
all-clusters-app tlsr9118bdk40d FLASH 658818 658818 0 0.0
RAM 148408 148408 0 0.0
all-clusters-minimal-app tlsr9528a FLASH 779090 779120 30 0.0
RAM 113212 113212 0 0.0
bridge-app tlsr9258a FLASH 675922 675952 30 0.0
RAM 95304 95304 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 634510 634540 30 0.0
RAM 50572 50572 0 0.0
light-switch-app-ota-shell-factory-data tlsr9528a FLASH 720370 720400 30 0.0
RAM 77148 77148 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 613924 613924 0 0.0
RAM 144636 144636 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 801676 801706 30 0.0
RAM 103040 103040 0 0.0
lock-app-dfu tlsr9528a FLASH 666340 666356 16 0.0
RAM 69852 69852 0 0.0
ota-requestor-app tlsr9258a FLASH 695252 695282 30 0.0
RAM 95028 95028 0 0.0
pump-app tlsr9518adk80d FLASH 616784 616814 30 0.0
RAM 56952 56952 0 0.0
pump-controller-app tlsr9518adk80d FLASH 607168 607198 30 0.0
RAM 56752 56752 0 0.0
shell tlsr9518adk80d FLASH 466356 466356 0 0.0
RAM 72484 72484 0 0.0
smoke_co_alarm-app tlsr9528a_retention FLASH 641128 641158 30 0.0
RAM 52200 52200 0 0.0
temperature-measurement-app-mars-ota tlsr9518adk80d FLASH 650994 651024 30 0.0
RAM 60388 60388 0 0.0
thermostat tlsr9518adk80d FLASH 626058 626088 30 0.0
RAM 57084 57084 0 0.0
window-covering tlsr9118bdk40d FLASH 519318 519318 0 0.0
RAM 97800 97800 0 0.0
tizen all-clusters-app arm unknown 1584 1584 0 0.0
FLASH 1639268 1639300 32 0.0
RAM 48548 48548 0 0.0
chip-tool-ubsan arm unknown 2384 2384 0 0.0
FLASH 16292958 16293278 320 0.0
RAM 7156248 7156300 52 0.0

@mergify mergify bot merged commit 74768a8 into project-chip:master Jul 16, 2024
68 checks passed
@ksperling-apple ksperling-apple deleted the thread-ds branch July 16, 2024 00:41
VerifyOrReturnError(tlv->GetLength() == 3, CHIP_ERROR_INVALID_TLV_ELEMENT);
// Note: The channel page (byte 0) is not returned
const uint8_t * value = tlv->GetValue();
aChannel = static_cast<uint16_t>((value[1] << 8) | value[2]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just BigEndian::Get16 starting at the right byte?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, I'll include that change in my other PR.

j-ororke pushed a commit to j-ororke/connectedhomeip that referenced this pull request Jul 31, 2024
* ThreadOperationalDataset: various bug fixes

- Ensure TLVs read have the correct length
- Default construct as empty (mLength == 0)
- Change MakeRoom() size argument to size_t to avoid chance of truncation
- Check for null in SetNetworkName
- Use Encoding::BigEndian instead of hand-rolled math
- Use ReturnErrorOnFailure / VerifyOrReturnError consistently
- Make tests independent from each other (non-static dataset member)
- Add more tests

* Fix includes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants