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

[Inet] Explicit error return from FromSockAddr #13049

Merged
merged 1 commit into from
Dec 16, 2021
Merged

[Inet] Explicit error return from FromSockAddr #13049

merged 1 commit into from
Dec 16, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

A review raised questions about failure cases of IPAddress::FromSockAddr
(see link in the issue for details).

Fixes #10800 IPAddress::FromSockAddr falls through to return Any — is this correct?

Change overview

  • Replace FromSockAddr(SockAddr) with GetIPAddressFromSockAddr()
    with an explicit error return.
  • Change its main use, InterfaceIterator::GetAddress(), to also
    have an explicit error return.

Testing

CI (GetAddress is unit tested).

@kpschoedel
[Inet] Explicit error return from FromSockAddr
f74c278 
#### Problem

A review raised questions about failure cases of `IPAddress::FromSockAddr`
(see link in the issue for details).

Fixes  project-chip#10800 _IPAddress::FromSockAddr falls through to return Any — is this correct?_

#### Change overview

- Replace `FromSockAddr(SockAddr)` with `GetIPAddressFromSockAddr()`
  with an explicit error return.
- Change its main use, `InterfaceIterator::GetAddress()`, to also
  have an explicit error return.

#### Testing

CI (`GetAddress` is unit tested).
@kpschoedel kpschoedel mentioned this pull request Dec 15, 2021
Merged
@github-actions
Copy link

github-actions bot commented Dec 15, 2021 •
edited
Loading

PR #13049: Size comparison from 89aff25 to f74c278

Increases (7 builds for linux, mbed, p6)
platform target config section 89aff25 f74c278 change % change
linux chip-tool-ipv6only arm64 (read only) 6979836 6980332 496 0.0
.rodata 376348 376556 208 0.1
.text 5917428 5917732 304 0.0
thermostat-no-ble arm64 (read only) 2008724 2009348 624 0.0
.rodata 128068 128260 192 0.1
.text 1669120 1669552 432 0.0
mbed lighting-app CY8CPROTO_062_4343W+release (read/write) 2334320 2334384 64 0.0
.text 1296920 1296984 64 0.0
shell CY8CPROTO_062_4343W+release (read/write) 2054328 2054392 64 0.0
.text 1016928 1016992 64 0.0
p6 all-clusters-app default (read/write) 2394600 2394616 16 0.0
.text 1352864 1352880 16 0.0
light-app default (read/write) 2334912 2334928 16 0.0
.text 1293176 1293192 16 0.0
lock-app default (read/write) 2307128 2307144 16 0.0
.text 1265392 1265408 16 0.0
Decreases (3 builds for esp32, linux)
platform target config section 89aff25 f74c278 change % change
esp32 all-clusters-app c3devkit (read only) 880138 880132 -6 -0.0
.flash.text 880138 880132 -6 -0.0
m5stack (read only) 967871 967815 -56 -0.0
.flash.text 962487 962431 -56 -0.0
linux chip-tool-ipv6only arm64 .got 55520 55512 -8 -0.0
Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 89aff25 f74c278 change % change
efr32 lighting-app BRD4161A (read only) 840592 840592 0 0.0
(read/write) 128640 128640 0 0.0
.bss 126760 126760 0 0.0
.data 1876 1876 0 0.0
.text 840584 840584 0 0.0
BRD4161A+rpc (read only) 828196 828196 0 0.0
(read/write) 145312 145312 0 0.0
.bss 143336 143336 0 0.0
.data 1976 1976 0 0.0
.text 828188 828188 0 0.0
window-app BRD4161A (read only) 813968 813968 0 0.0
(read/write) 127576 127576 0 0.0
.bss 125744 125744 0 0.0
.data 1832 1832 0 0.0
.text 813960 813960 0 0.0
esp32 all-clusters-app c3devkit (read only) 880138 880132 -6 -0.0
(read/write) 1314250 1314250 0 0.0
.dram0.bss 71080 71080 0 0.0
.dram0.data 14212 14212 0 0.0
.flash.rodata 175912 175912 0 0.0
.flash.text 880138 880132 -6 -0.0
.iram0.text 62076 62076 0 0.0
m5stack (read only) 967871 967815 -56 -0.0
(read/write) 455512 455512 0 0.0
.dram0.bss 77552 77552 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 212116 212116 0 0.0
.flash.text 962487 962431 -56 -0.0
.iram0.text 123451 123451 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 691148 691148 0 0.0
.bss 79344 79344 0 0.0
.data 1908 1908 0 0.0
.text 604096 604096 0 0.0
lock-app k32w061+debug (read/write) 638712 638712 0 0.0
.bss 77496 77496 0 0.0
.data 1860 1860 0 0.0
.text 553556 553556 0 0.0
shell k32w061+debug (read/write) 644100 644100 0 0.0
.bss 79804 79804 0 0.0
.data 1840 1840 0 0.0
.text 556656 556656 0 0.0
linux chip-tool-ipv6only arm64 (read only) 6979836 6980332 496 0.0
(read/write) 324609 324609 0 0.0
.bss 55905 55905 0 0.0
.data 1096 1096 0 0.0
.data.rel.ro 208392 208392 0 0.0
.dynamic 560 560 0 0.0
.got 55520 55512 -8 -0.0
.init 24 24 0 0.0
.init_array 168 168 0 0.0
.rodata 376348 376556 208 0.1
.text 5917428 5917732 304 0.0
thermostat-no-ble arm64 (read only) 2008724 2009348 624 0.0
(read/write) 145297 145297 0 0.0
.bss 65649 65649 0 0.0
.data 832 832 0 0.0
.data.rel.ro 72080 72080 0 0.0
.dynamic 560 560 0 0.0
.got 3840 3840 0 0.0
.init 24 24 0 0.0
.init_array 288 288 0 0.0
.rodata 128068 128260 192 0.1
.text 1669120 1669552 432 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2338816 2338816 0 0.0
.bss 190332 190332 0 0.0
.data 5256 5256 0 0.0
.heap 840856 840856 0 0.0
.text 1301392 1301392 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2334320 2334384 64 0.0
.bss 182192 182192 0 0.0
.data 5544 5544 0 0.0
.heap 848712 848712 0 0.0
.text 1296920 1296984 64 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2307416 2307416 0 0.0
.bss 181232 181232 0 0.0
.data 5536 5536 0 0.0
.heap 849680 849680 0 0.0
.text 1270016 1270016 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.heap 1020320 1020320 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2054328 2054392 64 0.0
.bss 156980 156980 0 0.0
.data 4864 4864 0 0.0
.heap 874600 874600 0 0.0
.text 1016928 1016992 64 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 927055 927055 0 0.0
bss 119852 119852 0 0.0
rodata 104424 104424 0 0.0
text 627160 627160 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 890095 890095 0 0.0
bss 116204 116204 0 0.0
rodata 95720 95720 0 0.0
text 601900 601900 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 853062 853062 0 0.0
bss 121228 121228 0 0.0
rodata 99680 99680 0 0.0
text 557600 557600 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 899107 899107 0 0.0
bss 119032 119032 0 0.0
rodata 99732 99732 0 0.0
text 604928 604928 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 825366 825366 0 0.0
bss 120440 120440 0 0.0
rodata 95024 95024 0 0.0
text 535460 535460 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497447 497447 0 0.0
bss 51820 51820 0 0.0
rodata 45852 45852 0 0.0
text 339488 339488 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 904047 904047 0 0.0
bss 118944 118944 0 0.0
rodata 101088 101088 0 0.0
text 608532 608532 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 897287 897287 0 0.0
bss 118824 118824 0 0.0
rodata 99224 99224 0 0.0
text 603728 603728 0 0.0
shell nrf52840dk_nrf52840 (read/write) 782767 782767 0 0.0
bss 109624 109624 0 0.0
rodata 74396 74396 0 0.0
text 524232 524232 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 697838 697838 0 0.0
bss 110604 110604 0 0.0
rodata 69040 69040 0 0.0
text 444868 444868 0 0.0
p6 all-clusters-app default (read/write) 2394600 2394616 16 0.0
.bss 118532 118532 0 0.0
.data 2536 2536 0 0.0
.heap 912272 912272 0 0.0
.text 1352864 1352880 16 0.0
light-app default (read/write) 2334912 2334928 16 0.0
.bss 107448 107448 0 0.0
.data 2384 2384 0 0.0
.heap 923512 923512 0 0.0
.text 1293176 1293192 16 0.0
lock-app default (read/write) 2307128 2307144 16 0.0
.bss 106328 106328 0 0.0
.data 2336 2336 0 0.0
.heap 924680 924680 0 0.0
.text 1265392 1265408 16 0.0
qpg lighting-app qpg6105+debug (read only) 536144 536144 0 0.0
(read/write) 146936 146936 0 0.0
.bss 88096 88096 0 0.0
.data 1004 1004 0 0.0
.text 530824 530824 0 0.0
lock-app qpg6105+debug (read only) 507832 507832 0 0.0
(read/write) 146940 146940 0 0.0
.bss 87232 87232 0 0.0
.data 952 952 0 0.0
.text 502512 502512 0 0.0
persistent-storage-app qpg6105+debug (read only) 106448 106448 0 0.0
(read/write) 146938 146938 0 0.0
.bss 36146 36146 0 0.0
.data 288 288 0 0.0
.text 101128 101128 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 836574 836574 0 0.0
bss 88336 88336 0 0.0
noinit 37160 37160 0 0.0
text 583428 583428 0 0.0

bzbarsky-apple
bzbarsky-apple approved these changes Dec 15, 2021
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.

Rubber-stamping based on brief skim.

@andy31415 andy31415 merged commit f86f21f into project-chip:master Dec 16, 2021
@kpschoedel kpschoedel deleted the x10800-fromsockaddr branch December 16, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@msandstedt msandstedt msandstedt approved these changes

@andy31415 andy31415 andy31415 approved these changes

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

@austinh0 austinh0 Awaiting requested review from austinh0

@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

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

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@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

@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

@kghost kghost Awaiting requested review from kghost

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@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

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@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
darwin examples inet lib linux platform review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IPAddress::FromSockAddr falls through to return Any — is this correct?
4 participants
@kpschoedel @andy31415 @msandstedt @bzbarsky-apple