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

[Python] Drop unnecessary null termination #33915

Merged

Conversation

agners
Copy link
Contributor

@agners agners commented Jun 13, 2024

The ctypes data type c_char_p takes care of null-terminating the byte array provided to it. The additional null termination doesn't hurt in practice, but it's unnecessary.

The ctypes data type `c_char_p` takes care of null-terminating the byte
array provided to it. The additional null termination doesn't hurt in
practice, but it's unnecessary.
@agners
Copy link
Contributor Author

agners commented Jun 13, 2024

This StackOverflow comment mentions that the implicitly called z_set function calls PyBytes_AsString which takes care of null termination, from the PyBytes_AsString docs:

Return a pointer to the contents of o. The pointer refers to the internal buffer of o, which consists of len(o) + 1 bytes. The last byte in the buffer is always null, regardless of whether there are any other null bytes.

I've verified this to be the case by checking what is present at memory address strlen(), +1/+2.

Additionally, we do have places where "manual" null-termination hasn't been applied, and it seemd to have worked too.

Copy link

github-actions bot commented Jun 13, 2024

PR #33915: Size comparison from a3bb9c3 to b2f627a

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 a3bb9c3 b2f627a9 change % change
bl602 lighting-app bl602 FLASH 1268236 1268236 0 0.0
RAM 95328 95328 0 0.0
bl602+mfd FLASH 1282514 1282514 0 0.0
RAM 95480 95480 0 0.0
bl602+rpc FLASH 1307194 1307194 0 0.0
RAM 103760 103760 0 0.0
bl702 lighting-app bl702 FLASH 1088876 1088876 0 0.0
RAM 14897 14897 0 0.0
bl702+mfd FLASH 1099826 1099826 0 0.0
RAM 15049 15049 0 0.0
bl702+rpc FLASH 1178956 1178956 0 0.0
RAM 23925 23925 0 0.0
bl706-eth FLASH 872476 872476 0 0.0
RAM 27016 27016 0 0.0
bl706-wifi FLASH 1124174 1124174 0 0.0
RAM 14349 14349 0 0.0
bl702l lighting-app bl702l FLASH 1076018 1076018 0 0.0
RAM 21468 21468 0 0.0
bl702l+mfd FLASH 1087280 1087280 0 0.0
RAM 21628 21628 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 797864 797864 0 0.0
RAM 103088 103088 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 813668 813668 0 0.0
RAM 113568 113568 0 0.0
lock-mtd LP_EM_CC1354P10_6 FLASH 803056 803056 0 0.0
RAM 107688 107688 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 755552 755552 0 0.0
RAM 101788 101788 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 741184 741184 0 0.0
RAM 102036 102036 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 605378 605378 0 0.0
RAM 204512 204512 0 0.0
lock CC3235SF_LAUNCHXL FLASH 651166 651166 0 0.0
RAM 204772 204772 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 661413 661413 0 0.0
RAM 74512 74512 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 681265 681265 0 0.0
RAM 77144 77144 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 681265 681265 0 0.0
RAM 77144 77144 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 638201 638201 0 0.0
RAM 69580 69580 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 603589 603589 0 0.0
RAM 70216 70216 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 623233 623233 0 0.0
RAM 72768 72768 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 623233 623233 0 0.0
RAM 72768 72768 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 619373 619373 0 0.0
RAM 73232 73232 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 639097 639097 0 0.0
RAM 75784 75784 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 639097 639097 0 0.0
RAM 75784 75784 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 587629 587629 0 0.0
RAM 67200 67200 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 607473 607473 0 0.0
RAM 69832 69832 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 607473 607473 0 0.0
RAM 69832 69832 0 0.0
efr32 lighting-app BRD4187C FLASH 888024 888024 0 0.0
RAM 183424 183424 0 0.0
lock-app BRD4338a FLASH 699912 699912 0 0.0
RAM 242228 242228 0 0.0
window-app BRD4187C FLASH 969084 969084 0 0.0
RAM 167856 167856 0 0.0
esp32 all-clusters-app c3devkit DRAM 88324 88324 0 0.0
FLASH 1469096 1469096 0 0.0
IRAM 75570 75570 0 0.0
m5stack DRAM 114796 114796 0 0.0
FLASH 1538019 1538019 0 0.0
IRAM 125403 125403 0 0.0
linux air-purifier-app debug unknown 4568 4568 0 0.0
FLASH 2533808 2533808 0 0.0
RAM 129072 129072 0 0.0
all-clusters-app debug unknown 5344 5344 0 0.0
FLASH 5583182 5583182 0 0.0
RAM 483208 483208 0 0.0
all-clusters-minimal-app debug unknown 5264 5264 0 0.0
FLASH 5054184 5054184 0 0.0
RAM 236792 236792 0 0.0
bridge-app debug unknown 5232 5232 0 0.0
FLASH 4479496 4479496 0 0.0
RAM 216752 216752 0 0.0
chip-tool debug unknown 5744 5744 0 0.0
FLASH 11520423 11520423 0 0.0
RAM 535394 535394 0 0.0
chip-tool-ipv6only arm64 unknown 19816 19816 0 0.0
FLASH 10657972 10657972 0 0.0
RAM 583416 583416 0 0.0
fabric-admin debug unknown 5600 5600 0 0.0
FLASH 11290599 11290599 0 0.0
RAM 528978 528978 0 0.0
fabric-bridge-app debug unknown 5240 5240 0 0.0
FLASH 4349096 4349096 0 0.0
RAM 208800 208800 0 0.0
lighting-app debug+rpc+ui unknown 5864 5864 0 0.0
FLASH 5367554 5367554 0 0.0
RAM 225392 225392 0 0.0
lock-app debug unknown 5184 5184 0 0.0
FLASH 4544872 4544872 0 0.0
RAM 204208 204208 0 0.0
ota-provider-app debug unknown 4552 4552 0 0.0
FLASH 4179352 4179352 0 0.0
RAM 193200 193200 0 0.0
ota-requestor-app debug unknown 4488 4488 0 0.0
FLASH 4303768 4303768 0 0.0
RAM 197840 197840 0 0.0
shell debug unknown 4112 4112 0 0.0
FLASH 2802765 2802765 0 0.0
RAM 154392 154392 0 0.0
thermostat-no-ble arm64 unknown 9184 9184 0 0.0
FLASH 4151860 4151860 0 0.0
RAM 234584 234584 0 0.0
tv-app debug unknown 5472 5472 0 0.0
FLASH 5589304 5589304 0 0.0
RAM 345640 345640 0 0.0
tv-casting-app debug unknown 5104 5104 0 0.0
FLASH 9568102 9568102 0 0.0
RAM 352800 352800 0 0.0
mbed lock-app-release cy8cproto_062_4343w FLASH 1497660 1497660 0 0.0
RAM 226072 226072 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 876604 876604 0 0.0
RAM 139129 139129 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 950224 950224 0 0.0
RAM 137557 137557 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 822156 822156 0 0.0
RAM 138027 138027 0 0.0
nxp contact k32w0+release FLASH 575436 575436 0 0.0
RAM 70024 70024 0 0.0
k32w1+release FLASH 590552 590552 0 0.0
RAM 74056 74056 0 0.0
light k32w0+release FLASH 609384 609384 0 0.0
RAM 69500 69500 0 0.0
k32w1+release FLASH 673744 673744 0 0.0
RAM 82808 82808 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1613772 1613772 0 0.0
RAM 207132 207132 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1534716 1534716 0 0.0
RAM 204036 204036 0 0.0
light cy8ckit_062s2_43012 FLASH 1460988 1460988 0 0.0
RAM 197316 197316 0 0.0
lock cy8ckit_062s2_43012 FLASH 1463300 1463300 0 0.0
RAM 224380 224380 0 0.0
qpg lighting-app qpg6105+debug FLASH 650352 650352 0 0.0
RAM 104556 104556 0 0.0
lock-app qpg6105+debug FLASH 610404 610404 0 0.0
RAM 99232 99232 0 0.0
stm32 light STM32WB5MM-DK FLASH 472260 472260 0 0.0
RAM 141652 141652 0 0.0
telink air-quality-sensor-app tlsr9528a_retention FLASH 625418 625418 0 0.0
RAM 49904 49904 0 0.0
all-clusters-app tlsr9118bdk40d FLASH 602942 602942 0 0.0
RAM 130288 130288 0 0.0
all-clusters-minimal-app tlsr9528a FLASH 770624 770624 0 0.0
RAM 110052 110052 0 0.0
bridge-app tlsr9258a FLASH 668544 668544 0 0.0
RAM 94672 94672 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 627170 627170 0 0.0
RAM 49948 49948 0 0.0
light-switch-app-ota-shell-factory-data tlsr9528a FLASH 713008 713008 0 0.0
RAM 76540 76540 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 558150 558150 0 0.0
RAM 126544 126544 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 793212 793212 0 0.0
RAM 99900 99900 0 0.0
lock-app-dfu tlsr9528a FLASH 660474 660474 0 0.0
RAM 69228 69228 0 0.0
ota-requestor-app tlsr9258a FLASH 686880 686880 0 0.0
RAM 94396 94396 0 0.0
pump-app tlsr9518adk80d FLASH 609370 609370 0 0.0
RAM 56328 56328 0 0.0
pump-controller-app tlsr9518adk80d FLASH 599718 599718 0 0.0
RAM 56128 56128 0 0.0
shell tlsr9518adk80d FLASH 462614 462614 0 0.0
RAM 71852 71852 0 0.0
smoke_co_alarm-app tlsr9528a_retention FLASH 633552 633552 0 0.0
RAM 51576 51576 0 0.0
temperature-measurement-app-mars-ota tlsr9518adk80d FLASH 642556 642556 0 0.0
RAM 59764 59764 0 0.0
thermostat tlsr9518adk80d FLASH 618620 618620 0 0.0
RAM 56452 56452 0 0.0
window-covering tlsr9118bdk40d FLASH 464934 464934 0 0.0
RAM 82224 82224 0 0.0
tizen all-clusters-app arm unknown 1548 1548 0 0.0
FLASH 1622328 1622328 0 0.0
RAM 45412 45412 0 0.0
chip-tool-ubsan arm unknown 2360 2360 0 0.0
FLASH 15753258 15753258 0 0.0
RAM 6912572 6912572 0 0.0

@andy31415 andy31415 merged commit 6842912 into project-chip:master Jun 14, 2024
68 of 69 checks passed
agners added a commit to home-assistant-libs/chip-wheels that referenced this pull request Jun 17, 2024
This adds more cleanups from the master branch to keep our 1.3 based
branch close to upstream. Most noteworthy here are a patch which stops
mDNS discovery when using the on-network commissioning API, fixes when
commissioning using WiFi/Thread setup through BLE directly (the Python
Matter Server isn't using this APIs currently), dropping unnecessary
code and and general messaging cleanup.

Specifically, this integrates changes from the following PRs
- project-chip/connectedhomeip#33882
- project-chip/connectedhomeip#33896
- project-chip/connectedhomeip#33891
- project-chip/connectedhomeip#33880
- project-chip/connectedhomeip#33914
- project-chip/connectedhomeip#33915
- project-chip/connectedhomeip#33933
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.

None yet

5 participants