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

Linux KVS Get impl should return buffer too small if the buffer is too small #19914

Merged
merged 3 commits into from
Jun 23, 2022
Merged

Linux KVS Get impl should return buffer too small if the buffer is too small #19914

merged 3 commits into from
Jun 23, 2022

Conversation

tehampson
Copy link
Contributor

Problem

  • KeyValueStoreManager::Get is expected to return CHIP_ERROR_BUFFER_TOO_SMALL if the buffer could not fit the entire value, but as many bytes as possible were written to it. Linux impl was not doing this.

Change overview

  • Return appropriate return code for KeyValueStoreManagerImpl::_Get

Testing

  • In PR that expecting KeyValueStoreManager::Get to return CHIP_ERROR_BUFFER_TOO_SMALL when giving a buffer of size 0, confirmed we are now getting CHIP_ERROR_BUFFER_TOO_SMALL instead of CHIP_NO_ERROR.

woody-apple
woody-apple approved these changes Jun 23, 2022
View reviewed changes
Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

Fast tracking platform changes.

@restyled-io restyled-io bot mentioned this pull request Jun 23, 2022
Closed
@github-actions
Copy link

github-actions bot commented Jun 23, 2022 •
edited
Loading

PR #19914: Size comparison from 7b78861 to ecb01b3

Increases (30 builds for cc13x2_26x2, efr32, esp32, linux, p6, telink)
platform target config section 7b78861 ecb01b3 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 191620 191644 24 0.0
lock-ftd LP_CC2652R7 (read/write) 150084 150116 32 0.0
pump-app LP_CC2652R7 (read/write) 169736 169760 24 0.0
pump-controller-app LP_CC2652R7 (read/write) 183968 184000 32 0.0
shell LP_CC2652R7 (read/write) 157284 157316 32 0.0
efr32 lighting-app BRD4161A (read/write) 1071784 1071824 40 0.0
.text 936000 936040 40 0.0
BRD4161A+rpc (read/write) 1125852 1125892 40 0.0
.text 973164 973204 40 0.0
BRD4161A+rs911x (read/write) 936456 936512 56 0.0
.text 794300 794356 56 0.0
window-app BRD4161A (read/write) 1057036 1057076 40 0.0
.text 921152 921192 40 0.0
esp32 all-clusters-app c3devkit (read only) 1014146 1014218 72 0.0
.flash.text 1014146 1014218 72 0.0
m5stack (read only) 1068475 1068515 40 0.0
.flash.text 1063091 1063131 40 0.0
linux all-clusters-app debug (read only) 2939145 2939161 16 0.0
.text 2500994 2501074 80 0.0
all-clusters-minimal-app debug (read only) 2792921 2792969 48 0.0
.text 2355346 2355426 80 0.0
bridge-app debug+rpc (read only) 2296905 2296953 48 0.0
.text 1939202 1939282 80 0.0
chip-tool debug (read only) 10168893 10168973 80 0.0
.text 8252165 8252293 128 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9905060 9905148 88 0.0
.text 7894660 7894788 128 0.0
lighting-app debug+rpc (read only) 2533673 2533737 64 0.0
.text 2151634 2151730 96 0.0
lock-app debug (read only) 2466977 2467009 32 0.0
.text 2076610 2076706 96 0.0
ota-provider-app debug (read only) 2304817 2304865 48 0.0
.text 1940418 1940498 80 0.0
ota-requestor-app debug (read only) 2421921 2421937 16 0.0
.text 2045266 2045346 80 0.0
shell debug (read only) 2612633 2612889 256 0.0
.text 2221154 2221442 288 0.0
thermostat-no-ble arm64 (read only) 2579044 2579116 72 0.0
.text 2175104 2175216 112 0.0
tv-app debug (read only) 3082929 3082945 16 0.0
.text 2648098 2648146 48 0.0
tv-casting-app debug (read only) 5573553 5573585 32 0.0
.text 4955234 4955330 96 0.0
p6 all-clusters-app default (read/write) 2555952 2555992 40 0.0
.text 1514216 1514256 40 0.0
all-clusters-minimal-app default (read/write) 2501800 2501840 40 0.0
.text 1460064 1460104 40 0.0
light-app default (read/write) 2432552 2432592 40 0.0
.text 1390816 1390856 40 0.0
lock-app default (read/write) 2453280 2453320 40 0.0
.text 1411544 1411584 40 0.0
telink light-switch-app tlsr9518adk80d (read/write) 793000 793040 40 0.0
text 561086 561124 38 0.0
lighting-app tlsr9518adk80d (read/write) 812712 812752 40 0.0
text 577546 577586 40 0.0
Decreases (28 builds for cc13x2_26x2, cyw30739, efr32, k32w, linux, nrfconnect)
platform target config section 7b78861 ecb01b3 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 660451 660427 -24 -0.0
.text 573128 573104 -24 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 649351 649319 -32 -0.0
.text 558408 558376 -32 -0.0
lock-ftd LP_CC2652R7 (read only) 692099 692067 -32 -0.0
.text 592148 592116 -32 -0.0
lock-mtd LP_CC2652R7 (read only) 641515 641491 -24 -0.0
.text 541668 541644 -24 -0.0
pump-app LP_CC2652R7 (read only) 673343 673319 -24 -0.0
.text 585076 585052 -24 -0.0
pump-controller-app LP_CC2652R7 (read only) 659223 659191 -32 -0.0
.text 575120 575088 -32 -0.0
shell LP_CC2652R7 (read only) 690290 690258 -32 -0.0
.text 580112 580080 -32 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 592626 592610 -16 -0.0
.app_xip_area 462740 462724 -16 -0.0
lock cyw930739m2evb_01 (read/write) 590010 589994 -16 -0.0
.app_xip_area 459948 459932 -16 -0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 595142 595126 -16 -0.0
.app_xip_area 466128 466112 -16 -0.0
efr32 lock-app BRD4161A+wf200 (read/write) 1112156 1112124 -32 -0.0
.text 969576 969544 -32 -0.0
k32w light k32w061+release (read/write) 662204 662188 -16 -0.0
.text 584016 584000 -16 -0.0
lock k32w061+release (read/write) 724200 724184 -16 -0.0
.text 645556 645540 -16 -0.0
linux all-clusters-app debug .rodata 261661 261597 -64 -0.0
all-clusters-minimal-app debug .rodata 263261 263229 -32 -0.0
bridge-app debug+rpc .rodata 196448 196416 -32 -0.0
chip-tool debug .rodata 508693 508645 -48 -0.0
chip-tool-no-interactive-ipv6only arm64 .rodata 471492 471452 -40 -0.0
lighting-app debug+rpc .rodata 212200 212168 -32 -0.0
lock-app debug .rodata 225928 225864 -64 -0.0
ota-provider-app debug .rodata 202104 202072 -32 -0.0
ota-requestor-app debug .rodata 205984 205920 -64 -0.0
shell debug .rodata 231698 231666 -32 -0.0
thermostat-no-ble arm64 .rodata 164724 164684 -40 -0.0
tv-app debug .rodata 247872 247840 -32 -0.0
tv-casting-app debug .rodata 341993 341929 -64 -0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1203187 1203155 -32 -0.0
text 824232 824212 -20 -0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1150535 1150503 -32 -0.0
text 796024 796004 -20 -0.0
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 7b78861 ecb01b3 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 660451 660427 -24 -0.0
(read/write) 191620 191644 24 0.0
.bss 74964 74964 0 0.0
.data 3356 3356 0 0.0
.rodata 87011 87011 0 0.0
.text 573128 573104 -24 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 649351 649319 -32 -0.0
(read/write) 158524 158524 0 0.0
.bss 74252 74252 0 0.0
.data 3356 3356 0 0.0
.rodata 90623 90623 0 0.0
.text 558408 558376 -32 -0.0
lock-ftd LP_CC2652R7 (read only) 692099 692067 -32 -0.0
(read/write) 150084 150116 32 0.0
.bss 71964 71964 0 0.0
.data 3280 3280 0 0.0
.rodata 99467 99467 0 0.0
.text 592148 592116 -32 -0.0
lock-mtd LP_CC2652R7 (read only) 641515 641491 -24 -0.0
(read/write) 145096 145096 0 0.0
.bss 67700 67700 0 0.0
.data 3280 3280 0 0.0
.rodata 99355 99355 0 0.0
.text 541668 541644 -24 -0.0
pump-app LP_CC2652R7 (read only) 673343 673319 -24 -0.0
(read/write) 169736 169760 24 0.0
.bss 72092 72092 0 0.0
.data 3280 3280 0 0.0
.rodata 87783 87783 0 0.0
.text 585076 585052 -24 -0.0
pump-controller-app LP_CC2652R7 (read only) 659223 659191 -32 -0.0
(read/write) 183968 184000 32 0.0
.bss 72204 72204 0 0.0
.data 3276 3276 0 0.0
.rodata 83623 83623 0 0.0
.text 575120 575088 -32 -0.0
shell LP_CC2652R7 (read only) 690290 690258 -32 -0.0
(read/write) 157284 157316 32 0.0
.bss 77268 77268 0 0.0
.data 3360 3360 0 0.0
.rodata 109866 109866 0 0.0
.text 580112 580080 -32 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 592626 592610 -16 -0.0
.app_xip_area 462740 462724 -16 -0.0
.bss 72848 72848 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 590010 589994 -16 -0.0
.app_xip_area 459948 459932 -16 -0.0
.bss 73024 73024 0 0.0
.data 720 720 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 595142 595126 -16 -0.0
.app_xip_area 466128 466112 -16 -0.0
.bss 72032 72032 0 0.0
.data 660 660 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1071784 1071824 40 0.0
.bss 133716 133716 0 0.0
.data 2048 2048 0 0.0
.text 936000 936040 40 0.0
BRD4161A+rpc (read/write) 1125852 1125892 40 0.0
.bss 150404 150404 0 0.0
.data 2260 2260 0 0.0
.text 973164 973204 40 0.0
BRD4161A+rs911x (read/write) 936456 936512 56 0.0
.bss 140088 140088 0 0.0
.data 2048 2048 0 0.0
.text 794300 794356 56 0.0
lock-app BRD4161A+wf200 (read/write) 1112156 1112124 -32 -0.0
.bss 140504 140504 0 0.0
.data 2056 2056 0 0.0
.text 969576 969544 -32 -0.0
window-app BRD4161A (read/write) 1057036 1057076 40 0.0
.bss 133788 133788 0 0.0
.data 2076 2076 0 0.0
.text 921152 921192 40 0.0
esp32 all-clusters-app c3devkit (read only) 1014146 1014218 72 0.0
(read/write) 1485666 1485666 0 0.0
.dram0.bss 71424 71424 0 0.0
.dram0.data 14632 14632 0 0.0
.flash.rodata 214176 214176 0 0.0
.flash.text 1014146 1014218 72 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1068475 1068515 40 0.0
(read/write) 487752 487752 0 0.0
.dram0.bss 76936 76936 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 244676 244676 0 0.0
.flash.text 1063091 1063131 40 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 662204 662188 -16 -0.0
.bss 70396 70396 0 0.0
.data 1992 1992 0 0.0
.text 584016 584000 -16 -0.0
lock k32w061+release (read/write) 724200 724184 -16 -0.0
.bss 70844 70844 0 0.0
.data 2000 2000 0 0.0
.text 645556 645540 -16 -0.0
linux all-clusters-app debug (read only) 2939145 2939161 16 0.0
(read/write) 177136 177136 0 0.0
.bss 84192 84192 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 84696 84696 0 0.0
.dynamic 608 608 0 0.0
.got 4536 4536 0 0.0
.init 27 27 0 0.0
.init_array 1032 1032 0 0.0
.rodata 261661 261597 -64 -0.0
.text 2500994 2501074 80 0.0
all-clusters-minimal-app debug (read only) 2792921 2792969 48 0.0
(read/write) 169040 169040 0 0.0
.bss 83520 83520 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 77336 77336 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1032 1032 0 0.0
.rodata 263261 263229 -32 -0.0
.text 2355346 2355426 80 0.0
bridge-app debug+rpc (read only) 2296905 2296953 48 0.0
(read/write) 147936 147936 0 0.0
.bss 71616 71616 0 0.0
.data 3792 3792 0 0.0
.data.rel.ro 66760 66760 0 0.0
.dynamic 608 608 0 0.0
.got 4400 4400 0 0.0
.init 27 27 0 0.0
.init_array 728 728 0 0.0
.rodata 196448 196416 -32 -0.0
.text 1939202 1939282 80 0.0
chip-tool debug (read only) 10168893 10168973 80 0.0
(read/write) 610600 610600 0 0.0
.bss 24416 24416 0 0.0
.data 1088 1088 0 0.0
.data.rel.ro 578800 578800 0 0.0
.dynamic 624 624 0 0.0
.got 5008 5008 0 0.0
.init 27 27 0 0.0
.init_array 640 640 0 0.0
.rodata 508693 508645 -48 -0.0
.text 8252165 8252293 128 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9905060 9905148 88 0.0
(read/write) 675281 675281 0 0.0
.bss 42641 42641 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 614200 614200 0 0.0
.dynamic 528 528 0 0.0
.got 13472 13472 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 471492 471452 -40 -0.0
.text 7894660 7894788 128 0.0
lighting-app debug+rpc (read only) 2533673 2533737 64 0.0
(read/write) 151928 151928 0 0.0
.bss 72064 72064 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 71912 71912 0 0.0
.dynamic 608 608 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 816 816 0 0.0
.rodata 212200 212168 -32 -0.0
.text 2151634 2151730 96 0.0
lock-app debug (read only) 2466977 2467009 32 0.0
(read/write) 146672 146672 0 0.0
.bss 70496 70496 0 0.0
.data 1680 1680 0 0.0
.data.rel.ro 68632 68632 0 0.0
.dynamic 608 608 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 776 776 0 0.0
.rodata 225928 225864 -64 -0.0
.text 2076610 2076706 96 0.0
ota-provider-app debug (read only) 2304817 2304865 48 0.0
(read/write) 140776 140776 0 0.0
.bss 70176 70176 0 0.0
.data 1912 1912 0 0.0
.data.rel.ro 62872 62872 0 0.0
.dynamic 608 608 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 672 672 0 0.0
.rodata 202104 202072 -32 -0.0
.text 1940418 1940498 80 0.0
ota-requestor-app debug (read only) 2421921 2421937 16 0.0
(read/write) 147648 147648 0 0.0
.bss 72544 72544 0 0.0
.data 2200 2200 0 0.0
.data.rel.ro 67048 67048 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 728 728 0 0.0
.rodata 205984 205920 -64 -0.0
.text 2045266 2045346 80 0.0
shell debug (read only) 2612633 2612889 256 0.0
(read/write) 196120 196120 0 0.0
.bss 111304 111304 0 0.0
.data 1232 1232 0 0.0
.data.rel.ro 77832 77832 0 0.0
.dynamic 608 608 0 0.0
.got 4168 4168 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 231698 231666 -32 -0.0
.text 2221154 2221442 288 0.0
thermostat-no-ble arm64 (read only) 2579044 2579116 72 0.0
(read/write) 180673 180673 0 0.0
.bss 87905 87905 0 0.0
.data 1704 1704 0 0.0
.data.rel.ro 82952 82952 0 0.0
.dynamic 528 528 0 0.0
.got 5088 5088 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 164724 164684 -40 -0.0
.text 2175104 2175216 112 0.0
tv-app debug (read only) 3082929 3082945 16 0.0
(read/write) 277864 277864 0 0.0
.bss 187720 187720 0 0.0
.data 4656 4656 0 0.0
.data.rel.ro 79048 79048 0 0.0
.dynamic 608 608 0 0.0
.got 4840 4840 0 0.0
.init 27 27 0 0.0
.init_array 952 952 0 0.0
.rodata 247872 247840 -32 -0.0
.text 2648098 2648146 48 0.0
tv-casting-app debug (read only) 5573553 5573585 32 0.0
(read/write) 184144 184144 0 0.0
.bss 72872 72872 0 0.0
.data 2448 2448 0 0.0
.data.rel.ro 102608 102608 0 0.0
.dynamic 608 608 0 0.0
.got 4712 4712 0 0.0
.init 27 27 0 0.0
.init_array 864 864 0 0.0
.rodata 341993 341929 -64 -0.0
.text 4955234 4955330 96 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2439536 2439536 0 0.0
.bss 214692 214692 0 0.0
.data 5864 5864 0 0.0
.text 1402180 1402180 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1203187 1203155 -32 -0.0
bss 143646 143646 0 0.0
rodata 156388 156388 0 0.0
text 824232 824212 -20 -0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1150535 1150503 -32 -0.0
bss 142882 142882 0 0.0
rodata 132720 132720 0 0.0
text 796024 796004 -20 -0.0
p6 all-clusters-app default (read/write) 2555952 2555992 40 0.0
.bss 149872 149872 0 0.0
.data 2776 2776 0 0.0
.text 1514216 1514256 40 0.0
all-clusters-minimal-app default (read/write) 2501800 2501840 40 0.0
.bss 149152 149152 0 0.0
.data 2776 2776 0 0.0
.text 1460064 1460104 40 0.0
light-app default (read/write) 2432552 2432592 40 0.0
.bss 141232 141232 0 0.0
.data 2592 2592 0 0.0
.text 1390816 1390856 40 0.0
lock-app default (read/write) 2453280 2453320 40 0.0
.bss 141056 141056 0 0.0
.data 2600 2600 0 0.0
.text 1411544 1411584 40 0.0
telink light-switch-app tlsr9518adk80d (read/write) 793000 793040 40 0.0
bss 71924 71924 0 0.0
noinit 40416 40416 0 0.0
text 561086 561124 38 0.0
lighting-app tlsr9518adk80d (read/write) 812712 812752 40 0.0
bss 72172 72172 0 0.0
noinit 40416 40416 0 0.0
text 577546 577586 40 0.0

@bzbarsky-apple
Copy link
Contributor

Looks like this fixes the Linux part of #16958 ?

@andy31415 andy31415 merged commit bb47380 into project-chip:master Jun 23, 2022
@tehampson tehampson deleted the fix-linux-kvs-impl-get branch June 23, 2022 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

@arkq arkq Awaiting requested review from arkq

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

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

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@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

@hawk248 hawk248 Awaiting requested review from hawk248

@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

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@kghost kghost Awaiting requested review from kghost

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

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

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@rgoliver rgoliver Awaiting requested review from rgoliver

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@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

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

Assignees
No one assigned
Labels
fast track linux platform review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tehampson @bzbarsky-apple @tcarmelveilleux @woody-apple @andy31415