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

Make sure optional arguments come last for chip-tool. #12064

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

The code that initializes arguments assumes the optional ones are
sorted at the end. So make sure that's actually true.

Fixes #12063

Problem

See #12063

Change overview

Make the chip-tool optional args come after all the required args.

Testing

Needs testing, still. Need to figure out how to check that announce-ota-provider now works right. At least the argument list it shows now has the optional arg at the end.

@github-actions
Copy link

github-actions bot commented Nov 20, 2021

PR #12064: Size comparison from 5f847e0 to 82029b4

Increases (1 build for linux)
platform target config section 5f847e0 82029b4 change % change
linux chip-tool debug (read only) 4982909 4985677 2768 0.1
.rodata 269354 269450 96 0.0
.text 4405397 4408069 2672 0.1
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 5f847e0 82029b4 change % change
efr32 lighting-app BRD4161A (read only) 750448 750448 0 0.0
(read/write) 119692 119692 0 0.0
.bss 117892 117892 0 0.0
.data 1800 1800 0 0.0
.text 750440 750440 0 0.0
BRD4161A+rpc (read only) 737972 737972 0 0.0
(read/write) 136320 136320 0 0.0
.bss 134396 134396 0 0.0
.data 1924 1924 0 0.0
.text 737964 737964 0 0.0
lock-app BRD4161A (read only) 727504 727504 0 0.0
(read/write) 113380 113380 0 0.0
.bss 111620 111620 0 0.0
.data 1756 1756 0 0.0
.text 727496 727496 0 0.0
window-app BRD4161A (read only) 728488 728488 0 0.0
(read/write) 113700 113700 0 0.0
.bss 111940 111940 0 0.0
.data 1760 1760 0 0.0
.text 728480 728480 0 0.0
esp32 all-clusters-app c3devkit (read only) 831588 831588 0 0.0
(read/write) 1221210 1221210 0 0.0
.dram0.bss 57696 57696 0 0.0
.dram0.data 14092 14092 0 0.0
.flash.rodata 164176 164176 0 0.0
.flash.text 831588 831588 0 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 902619 902619 0 0.0
(read/write) 420536 420536 0 0.0
.dram0.bss 63056 63056 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 192144 192144 0 0.0
.flash.text 897235 897235 0 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 707304 707304 0 0.0
.bss 77212 77212 0 0.0
.data 1912 1912 0 0.0
.text 622380 622380 0 0.0
lock-app k32w061+debug (read/write) 598736 598736 0 0.0
.bss 67716 67716 0 0.0
.data 1880 1880 0 0.0
.text 523340 523340 0 0.0
shell k32w061+debug (read/write) 664112 664112 0 0.0
.bss 78876 78876 0 0.0
.data 1848 1848 0 0.0
.text 577588 577588 0 0.0
linux all-clusters-app debug (read only) 1745017 1745017 0 0.0
(read/write) 128392 128392 0 0.0
.bss 58544 58544 0 0.0
.data 1042 1042 0 0.0
.data.rel.ro 63504 63504 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 560 560 0 0.0
.rodata 137493 137493 0 0.0
.text 1471890 1471890 0 0.0
bridge-app debug+rpc (read only) 1324829 1324829 0 0.0
(read/write) 76432 76432 0 0.0
.bss 41520 41520 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 28352 28352 0 0.0
.dynamic 592 592 0 0.0
.got 3984 3984 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 112028 112028 0 0.0
.text 1115397 1115397 0 0.0
chip-tool debug (read only) 4982909 4985677 2768 0.1
(read/write) 165288 165288 0 0.0
.bss 39848 39848 0 0.0
.data 2272 2272 0 0.0
.data.rel.ro 117664 117664 0 0.0
.dynamic 592 592 0 0.0
.got 4416 4416 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 269354 269450 96 0.0
.text 4405397 4408069 2672 0.1
lighting-app debug+rpc (read only) 1593937 1593937 0 0.0
(read/write) 109616 109616 0 0.0
.bss 47152 47152 0 0.0
.data 1234 1234 0 0.0
.data.rel.ro 55920 55920 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 129649 129649 0 0.0
.text 1329282 1329282 0 0.0
ota-provider-app debug (read only) 1272377 1272377 0 0.0
(read/write) 75048 75048 0 0.0
.bss 44096 44096 0 0.0
.data 784 784 0 0.0
.data.rel.ro 25080 25080 0 0.0
.dynamic 592 592 0 0.0
.got 4016 4016 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 114047 114047 0 0.0
.text 1060738 1060738 0 0.0
ota-requestor-app debug (read only) 1358113 1358113 0 0.0
(read/write) 78784 78784 0 0.0
.bss 46560 46560 0 0.0
.data 848 848 0 0.0
.data.rel.ro 26280 26280 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 125064 125064 0 0.0
.text 1133074 1133074 0 0.0
shell debug (read only) 799185 799185 0 0.0
(read/write) 65832 65832 0 0.0
.bss 23336 23336 0 0.0
.data 242 242 0 0.0
.data.rel.ro 37752 37752 0 0.0
.dynamic 592 592 0 0.0
.got 3528 3528 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 78703 78703 0 0.0
.text 616466 616466 0 0.0
tv-app debug (read only) 1876257 1876257 0 0.0
(read/write) 318392 318392 0 0.0
.bss 249800 249800 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 60144 60144 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 158021 158021 0 0.0
.text 1573106 1573106 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2289808 2289808 0 0.0
.bss 180300 180300 0 0.0
.data 5224 5224 0 0.0
.heap 850920 850920 0 0.0
.text 1252408 1252408 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2273936 2273936 0 0.0
.bss 172196 172196 0 0.0
.data 5576 5576 0 0.0
.heap 858672 858672 0 0.0
.text 1236536 1236536 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2249840 2249840 0 0.0
.bss 171084 171084 0 0.0
.data 5568 5568 0 0.0
.heap 859792 859792 0 0.0
.text 1212440 1212440 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139744 1139744 0 0.0
.bss 11752 11752 0 0.0
.data 4368 4368 0 0.0
.heap 1020328 1020328 0 0.0
.text 103128 103128 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2050480 2050480 0 0.0
.bss 156272 156272 0 0.0
.data 4968 4968 0 0.0
.heap 875208 875208 0 0.0
.text 1013080 1013080 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 865027 865027 0 0.0
bss 110604 110604 0 0.0
rodata 95828 95828 0 0.0
text 582972 582972 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 827395 827395 0 0.0
bss 106960 106960 0 0.0
rodata 87012 87012 0 0.0
text 557140 557140 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 790082 790082 0 0.0
bss 111980 111980 0 0.0
rodata 91084 91084 0 0.0
text 512444 512444 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 840427 840427 0 0.0
bss 109628 109628 0 0.0
rodata 92300 92300 0 0.0
text 563064 563064 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 765730 765730 0 0.0
bss 111040 111040 0 0.0
rodata 87604 87604 0 0.0
text 492628 492628 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497327 497327 0 0.0
bss 51824 51824 0 0.0
rodata 45780 45780 0 0.0
text 339436 339436 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 846567 846567 0 0.0
bss 109768 109768 0 0.0
rodata 94008 94008 0 0.0
text 567248 567248 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 840127 840127 0 0.0
bss 109664 109664 0 0.0
rodata 92256 92256 0 0.0
text 562684 562684 0 0.0
shell nrf52840dk_nrf52840 (read/write) 778003 778003 0 0.0
bss 109112 109112 0 0.0
rodata 73064 73064 0 0.0
text 521244 521244 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 693058 693058 0 0.0
bss 110096 110096 0 0.0
rodata 67708 67708 0 0.0
text 441856 441856 0 0.0
p6 all-clusters-app default (read/write) 2302744 2302744 0 0.0
.bss 113280 113280 0 0.0
.data 2528 2528 0 0.0
.heap 917536 917536 0 0.0
.text 1261008 1261008 0 0.0
lock-app default (read/write) 2215944 2215944 0 0.0
.bss 100944 100944 0 0.0
.data 2400 2400 0 0.0
.heap 930000 930000 0 0.0
.text 1174208 1174208 0 0.0
qpg lighting-app qpg6100+debug (read only) 494080 494080 0 0.0
(read/write) 114144 114144 0 0.0
.bss 50256 50256 0 0.0
.data 1008 1008 0 0.0
.text 488760 488760 0 0.0
lock-app qpg6100+debug (read only) 469044 469044 0 0.0
(read/write) 114140 114140 0 0.0
.bss 49200 49200 0 0.0
.data 964 964 0 0.0
.text 463724 463724 0 0.0
persistent-storage-app qpg6100+debug (read only) 105416 105416 0 0.0
(read/write) 114142 114142 0 0.0
.bss 8994 8994 0 0.0
.data 272 272 0 0.0
.text 100096 100096 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 768410 768410 0 0.0
bss 79116 79116 0 0.0
noinit 37160 37160 0 0.0
text 533546 533546 0 0.0

The code that initializes arguments assumes the optional ones are
sorted at the end.  So make sure that's actually true.

Fixes project-chip#12063
@bzbarsky-apple bzbarsky-apple force-pushed the fix-optional-arg-ordering branch from 82029b4 to eb93240 Compare November 20, 2021 08:11
@github-actions
Copy link

github-actions bot commented Nov 20, 2021

PR #12064: Size comparison from 6cb75f4 to eb93240

Increases (1 build for linux)
platform target config section 6cb75f4 eb93240 change % change
linux chip-tool debug (read only) 4982909 4985677 2768 0.1
.rodata 269354 269450 96 0.0
.text 4405397 4408069 2672 0.1
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 6cb75f4 eb93240 change % change
efr32 lighting-app BRD4161A (read only) 750448 750448 0 0.0
(read/write) 119692 119692 0 0.0
.bss 117892 117892 0 0.0
.data 1800 1800 0 0.0
.text 750440 750440 0 0.0
BRD4161A+rpc (read only) 737972 737972 0 0.0
(read/write) 136320 136320 0 0.0
.bss 134396 134396 0 0.0
.data 1924 1924 0 0.0
.text 737964 737964 0 0.0
lock-app BRD4161A (read only) 727504 727504 0 0.0
(read/write) 113380 113380 0 0.0
.bss 111620 111620 0 0.0
.data 1756 1756 0 0.0
.text 727496 727496 0 0.0
window-app BRD4161A (read only) 728488 728488 0 0.0
(read/write) 113700 113700 0 0.0
.bss 111940 111940 0 0.0
.data 1760 1760 0 0.0
.text 728480 728480 0 0.0
esp32 all-clusters-app c3devkit (read only) 831588 831588 0 0.0
(read/write) 1221210 1221210 0 0.0
.dram0.bss 57696 57696 0 0.0
.dram0.data 14092 14092 0 0.0
.flash.rodata 164176 164176 0 0.0
.flash.text 831588 831588 0 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 902619 902619 0 0.0
(read/write) 420536 420536 0 0.0
.dram0.bss 63056 63056 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 192144 192144 0 0.0
.flash.text 897235 897235 0 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 707304 707304 0 0.0
.bss 77212 77212 0 0.0
.data 1912 1912 0 0.0
.text 622380 622380 0 0.0
lock-app k32w061+debug (read/write) 598736 598736 0 0.0
.bss 67716 67716 0 0.0
.data 1880 1880 0 0.0
.text 523340 523340 0 0.0
shell k32w061+debug (read/write) 664112 664112 0 0.0
.bss 78876 78876 0 0.0
.data 1848 1848 0 0.0
.text 577588 577588 0 0.0
linux all-clusters-app debug (read only) 1745017 1745017 0 0.0
(read/write) 128392 128392 0 0.0
.bss 58544 58544 0 0.0
.data 1042 1042 0 0.0
.data.rel.ro 63504 63504 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 560 560 0 0.0
.rodata 137493 137493 0 0.0
.text 1471890 1471890 0 0.0
bridge-app debug+rpc (read only) 1324829 1324829 0 0.0
(read/write) 76432 76432 0 0.0
.bss 41520 41520 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 28352 28352 0 0.0
.dynamic 592 592 0 0.0
.got 3984 3984 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 112028 112028 0 0.0
.text 1115397 1115397 0 0.0
chip-tool debug (read only) 4982909 4985677 2768 0.1
(read/write) 165288 165288 0 0.0
.bss 39848 39848 0 0.0
.data 2272 2272 0 0.0
.data.rel.ro 117664 117664 0 0.0
.dynamic 592 592 0 0.0
.got 4416 4416 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 269354 269450 96 0.0
.text 4405397 4408069 2672 0.1
lighting-app debug+rpc (read only) 1593937 1593937 0 0.0
(read/write) 109616 109616 0 0.0
.bss 47152 47152 0 0.0
.data 1234 1234 0 0.0
.data.rel.ro 55920 55920 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 129649 129649 0 0.0
.text 1329282 1329282 0 0.0
ota-provider-app debug (read only) 1272377 1272377 0 0.0
(read/write) 75048 75048 0 0.0
.bss 44096 44096 0 0.0
.data 784 784 0 0.0
.data.rel.ro 25080 25080 0 0.0
.dynamic 592 592 0 0.0
.got 4016 4016 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 114047 114047 0 0.0
.text 1060738 1060738 0 0.0
ota-requestor-app debug (read only) 1358113 1358113 0 0.0
(read/write) 78784 78784 0 0.0
.bss 46560 46560 0 0.0
.data 848 848 0 0.0
.data.rel.ro 26280 26280 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 125064 125064 0 0.0
.text 1133074 1133074 0 0.0
shell debug (read only) 799185 799185 0 0.0
(read/write) 65832 65832 0 0.0
.bss 23336 23336 0 0.0
.data 242 242 0 0.0
.data.rel.ro 37752 37752 0 0.0
.dynamic 592 592 0 0.0
.got 3528 3528 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 78703 78703 0 0.0
.text 616466 616466 0 0.0
tv-app debug (read only) 1876257 1876257 0 0.0
(read/write) 318392 318392 0 0.0
.bss 249800 249800 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 60144 60144 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 158021 158021 0 0.0
.text 1573106 1573106 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2289808 2289808 0 0.0
.bss 180300 180300 0 0.0
.data 5224 5224 0 0.0
.heap 850920 850920 0 0.0
.text 1252408 1252408 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2273936 2273936 0 0.0
.bss 172196 172196 0 0.0
.data 5576 5576 0 0.0
.heap 858672 858672 0 0.0
.text 1236536 1236536 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2249840 2249840 0 0.0
.bss 171084 171084 0 0.0
.data 5568 5568 0 0.0
.heap 859792 859792 0 0.0
.text 1212440 1212440 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139744 1139744 0 0.0
.bss 11752 11752 0 0.0
.data 4368 4368 0 0.0
.heap 1020328 1020328 0 0.0
.text 103128 103128 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2050480 2050480 0 0.0
.bss 156272 156272 0 0.0
.data 4968 4968 0 0.0
.heap 875208 875208 0 0.0
.text 1013080 1013080 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 865027 865027 0 0.0
bss 110604 110604 0 0.0
rodata 95828 95828 0 0.0
text 582972 582972 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 827395 827395 0 0.0
bss 106960 106960 0 0.0
rodata 87012 87012 0 0.0
text 557140 557140 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 790082 790082 0 0.0
bss 111980 111980 0 0.0
rodata 91084 91084 0 0.0
text 512444 512444 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 840427 840427 0 0.0
bss 109628 109628 0 0.0
rodata 92300 92300 0 0.0
text 563064 563064 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 765730 765730 0 0.0
bss 111040 111040 0 0.0
rodata 87604 87604 0 0.0
text 492628 492628 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497327 497327 0 0.0
bss 51824 51824 0 0.0
rodata 45780 45780 0 0.0
text 339436 339436 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 846567 846567 0 0.0
bss 109768 109768 0 0.0
rodata 94008 94008 0 0.0
text 567248 567248 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 840127 840127 0 0.0
bss 109664 109664 0 0.0
rodata 92256 92256 0 0.0
text 562684 562684 0 0.0
shell nrf52840dk_nrf52840 (read/write) 778003 778003 0 0.0
bss 109112 109112 0 0.0
rodata 73064 73064 0 0.0
text 521244 521244 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 693058 693058 0 0.0
bss 110096 110096 0 0.0
rodata 67708 67708 0 0.0
text 441856 441856 0 0.0
p6 all-clusters-app default (read/write) 2302744 2302744 0 0.0
.bss 113280 113280 0 0.0
.data 2528 2528 0 0.0
.heap 917536 917536 0 0.0
.text 1261008 1261008 0 0.0
lock-app default (read/write) 2215944 2215944 0 0.0
.bss 100944 100944 0 0.0
.data 2400 2400 0 0.0
.heap 930000 930000 0 0.0
.text 1174208 1174208 0 0.0
qpg lighting-app qpg6100+debug (read only) 494080 494080 0 0.0
(read/write) 114144 114144 0 0.0
.bss 50256 50256 0 0.0
.data 1008 1008 0 0.0
.text 488760 488760 0 0.0
lock-app qpg6100+debug (read only) 469044 469044 0 0.0
(read/write) 114140 114140 0 0.0
.bss 49200 49200 0 0.0
.data 964 964 0 0.0
.text 463724 463724 0 0.0
persistent-storage-app qpg6100+debug (read only) 105416 105416 0 0.0
(read/write) 114142 114142 0 0.0
.bss 8994 8994 0 0.0
.data 272 272 0 0.0
.text 100096 100096 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 768410 768410 0 0.0
bss 79116 79116 0 0.0
noinit 37160 37160 0 0.0
text 533546 533546 0 0.0

@bzbarsky-apple
Copy link
Contributor Author

@vivien-apple Please review?

@andy31415
Copy link
Contributor

fast track: chip tool trivial change, chip-tool is exercised quite a bit in CI (so assume tested)

@andy31415 andy31415 merged commit dbf7d9d into project-chip:master Nov 22, 2021
@bzbarsky-apple bzbarsky-apple deleted the fix-optional-arg-ordering branch November 22, 2021 16:54
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.

chip-tool handling of optional arguments that are not at the end of the list is all broken
2 participants