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.

Sign up for GitHub

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

Jump to bottom

[Fix] Add missing return cause after AddStatus #15186

Merged
merged 3 commits into from
Feb 16, 2022
Merged

[Fix] Add missing return cause after AddStatus #15186

merged 3 commits into from
Feb 16, 2022

Conversation

erjiaqing
Copy link
Contributor

Problem

The code added in PR #15125 will happily do:

mConnectingNetworkIDLen = static_cast<uint8_t>(req.networkID.size());
memcpy(mConnectingNetworkID, req.networkID.data(), mConnectingNetworkIDLen);
even if mConnectingNetworkIDLen (which is provided on the wire here) is larger than the size of the mConnectingNetworkID buffer.

Change overview

  • Adds the missing return

Testing

  • Adds a simple negative test case in cirque since we need a real network driver here.

bzbarsky-apple
bzbarsky-apple approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

Can we please add a YAML test too?

@github-actions
Copy link

github-actions bot commented Feb 15, 2022 •
edited
Loading

PR #15186: Size comparison from f05070a to bb72c55

Increases (1 build for telink)
platform target config section f05070a bb72c55 change % change
telink lighting-app tlsr9518adk80d (read/write) 875938 875946 8 0.0
text 615704 615708 4 0.0
Decreases (3 builds for efr32, esp32)
platform target config section f05070a bb72c55 change % change
efr32 window-app BRD4161A (read only) 845156 845140 -16 -0.0
.text 845148 845132 -16 -0.0
esp32 all-clusters-app c3devkit (read only) 945792 945788 -4 -0.0
.flash.text 945792 945788 -4 -0.0
m5stack (read only) 995651 995643 -8 -0.0
.flash.text 990267 990259 -8 -0.0
Full report (34 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section f05070a bb72c55 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 593150 593150 0 0.0
.app_xip_area 498772 498772 0 0.0
.bss 77076 77076 0 0.0
.data 644 644 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 551210 551210 0 0.0
.app_xip_area 458400 458400 0 0.0
.bss 75548 75548 0 0.0
.data 608 608 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor cyw930739m2evb_01 (read/write) 569726 569726 0 0.0
.app_xip_area 467660 467660 0 0.0
.bss 84476 84476 0 0.0
.data 552 552 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 909912 909912 0 0.0
(read/write) 127240 127240 0 0.0
.bss 125208 125208 0 0.0
.data 2032 2032 0 0.0
.text 909904 909904 0 0.0
BRD4161A+rpc (read only) 938632 938632 0 0.0
(read/write) 144160 144160 0 0.0
.bss 141984 141984 0 0.0
.data 2172 2172 0 0.0
.text 938624 938624 0 0.0
window-app BRD4161A (read only) 845156 845140 -16 -0.0
(read/write) 125300 125300 0 0.0
.bss 123400 123400 0 0.0
.data 1900 1900 0 0.0
.text 845148 845132 -16 -0.0
esp32 all-clusters-app c3devkit (read only) 945792 945788 -4 -0.0
(read/write) 1398562 1398562 0 0.0
.dram0.bss 66248 66248 0 0.0
.dram0.data 14268 14268 0 0.0
.flash.rodata 199144 199144 0 0.0
.flash.text 945792 945788 -4 -0.0
.iram0.text 62056 62056 0 0.0
m5stack (read only) 995651 995643 -8 -0.0
(read/write) 463840 463840 0 0.0
.dram0.bss 71392 71392 0 0.0
.dram0.data 34064 34064 0 0.0
.flash.rodata 226256 226256 0 0.0
.flash.text 990267 990259 -8 -0.0
.iram0.text 123399 123399 0 0.0
k32w light k32w061+release (read/write) 683848 683848 0 0.0
.bss 76512 76512 0 0.0
.data 1904 1904 0 0.0
.text 599632 599632 0 0.0
lock k32w061+release (read/write) 689396 689396 0 0.0
.bss 76904 76904 0 0.0
.data 1948 1948 0 0.0
.text 604744 604744 0 0.0
linux chip-tool-ipv6only arm64 (read only) 8311060 8311060 0 0.0
(read/write) 353649 353649 0 0.0
.bss 50113 50113 0 0.0
.data 1216 1216 0 0.0
.data.rel.ro 249304 249304 0 0.0
.dynamic 560 560 0 0.0
.got 49224 49224 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 435500 435500 0 0.0
.text 7113076 7113076 0 0.0
thermostat-no-ble arm64 (read only) 2152684 2152684 0 0.0
(read/write) 140337 140337 0 0.0
.bss 57169 57169 0 0.0
.data 1032 1032 0 0.0
.data.rel.ro 75024 75024 0 0.0
.dynamic 560 560 0 0.0
.got 4144 4144 0 0.0
.init 24 24 0 0.0
.init_array 328 328 0 0.0
.rodata 133020 133020 0 0.0
.text 1801408 1801408 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2398544 2398544 0 0.0
.bss 188252 188252 0 0.0
.data 5320 5320 0 0.0
.text 1361144 1361144 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2360768 2360768 0 0.0
.bss 180864 180864 0 0.0
.data 5624 5624 0 0.0
.text 1323368 1323368 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2324368 2324368 0 0.0
.bss 180752 180752 0 0.0
.data 5600 5600 0 0.0
.text 1286968 1286968 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139648 1139648 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.text 103032 103032 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2309324 2309324 0 0.0
.bss 178004 178004 0 0.0
.data 5424 5424 0 0.0
.text 1271896 1271896 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 1016735 1016735 0 0.0
bss 121272 121272 0 0.0
rodata 120204 120204 0 0.0
text 696284 696284 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 986011 986011 0 0.0
bss 118460 118460 0 0.0
rodata 111728 111728 0 0.0
text 676172 676172 0 0.0
nrf52840dongle_nrf52840 (read/write) 1032595 1032595 0 0.0
bss 122660 122660 0 0.0
rodata 119084 119084 0 0.0
text 700904 700904 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 923378 923378 0 0.0
bss 117836 117836 0 0.0
rodata 113444 113444 0 0.0
text 611300 611300 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 946463 946463 0 0.0
bss 119644 119644 0 0.0
rodata 108964 108964 0 0.0
text 639316 639316 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 853990 853990 0 0.0
bss 116236 116236 0 0.0
rodata 102136 102136 0 0.0
text 555108 555108 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 527595 527595 0 0.0
bss 53632 53632 0 0.0
rodata 49976 49976 0 0.0
text 361016 361016 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 945279 945279 0 0.0
bss 119364 119364 0 0.0
rodata 107916 107916 0 0.0
text 639372 639372 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 941227 941227 0 0.0
bss 119368 119368 0 0.0
rodata 107616 107616 0 0.0
text 635612 635612 0 0.0
shell nrf52840dk_nrf52840 (read/write) 807147 807147 0 0.0
bss 111216 111216 0 0.0
rodata 79104 79104 0 0.0
text 539244 539244 0 0.0
p6 all-clusters-app default (read/write) 2484072 2484072 0 0.0
.bss 117608 117608 0 0.0
.data 2672 2672 0 0.0
.text 1442336 1442336 0 0.0
light-app default (read/write) 2390920 2390920 0 0.0
.bss 107248 107248 0 0.0
.data 2520 2520 0 0.0
.text 1349184 1349184 0 0.0
lock-app default (read/write) 2354384 2354384 0 0.0
.bss 106976 106976 0 0.0
.data 2480 2480 0 0.0
.text 1312648 1312648 0 0.0
qpg lighting-app qpg6105+debug (read only) 595624 595624 0 0.0
(read/write) 146936 146936 0 0.0
.bss 88904 88904 0 0.0
.data 1108 1108 0 0.0
.text 590304 590304 0 0.0
lock-app qpg6105+debug (read only) 561360 561360 0 0.0
(read/write) 146936 146936 0 0.0
.bss 88888 88888 0 0.0
.data 1060 1060 0 0.0
.text 556040 556040 0 0.0
persistent-storage-app qpg6105+debug (read only) 99520 99520 0 0.0
(read/write) 146940 146940 0 0.0
.bss 24004 24004 0 0.0
.data 176 176 0 0.0
.text 94200 94200 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 875938 875946 8 0.0
bss 88580 88580 0 0.0
noinit 37160 37160 0 0.0
text 615704 615708 4 0.0

@erjiaqing
Copy link
Contributor Author

Can we please add a YAML test too?

The related logic is only enabled with a valid WirelessNetworkDriver, I'm considering adding a mock network driver for tests.

@erjiaqing erjiaqing merged commit 76430b3 into project-chip:master Feb 16, 2022
@erjiaqing erjiaqing deleted the issue/15124 branch February 16, 2022 04:09
jamesluo11 pushed a commit to jamesluo11/connectedhomeip that referenced this pull request Apr 26, 2022
@erjiaqing @woody-apple
@Damian-Nordic @fuxingguo16
[Fix] Add missing return cause after AddStatus (project-chip#15186)
f1bdd20 
* [Fix] Add missing return cause after AddStatus

* Add negative tests

* Update src/controller/python/test/test_scripts/network_commissioning.py

Co-authored-by: Damian Królik <[email protected]>

Co-authored-by: Justin Wood <[email protected]>
Co-authored-by: Damian Królik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

@anush-apple anush-apple Awaiting requested review from anush-apple

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

Assignees
No one assigned
Labels
app controller review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@erjiaqing @bzbarsky-apple @Damian-Nordic @woody-apple