Skip to content

sys/net/nanocoap: fix coap_get_total_hdr_len()#20946

Merged
maribu merged 2 commits intoRIOT-OS:masterfrom
maribu:sys/nanocoap/fix-header-size-computation
Nov 1, 2024
Merged

sys/net/nanocoap: fix coap_get_total_hdr_len()#20946
maribu merged 2 commits intoRIOT-OS:masterfrom
maribu:sys/nanocoap/fix-header-size-computation

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Nov 1, 2024

Contribution description

Before coap_get_total_hdr_len() did not take the extended TKL field (RFC 8974) into account. This fixes the issue.

Testing procedure

Unit tests in master (cherry-picking the second commit into master):

$ make BOARD=native -C tests/unittests all test
[...]

main(): This is RIOT! (Version: 2024.10-devel-370-g00e25)
Help: Press s to start test, r to print it is ready
READY
s
START
.......................==83870==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
.....................................................................................................................................................................................................................==83870==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0xffcd4000; bottom 0xedf02000; size: 0x11dd2000 (299704320)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
.................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
nanocoap_tests.test_nanocoap__token_length_ext_16 (tests/unittests/tests-nanocoap/tests-nanocoap.c 1109) exp 21 was 20
.
nanocoap_tests.test_nanocoap__token_length_ext_269 (tests/unittests/tests-nanocoap/tests-nanocoap.c 1138) exp 275 was 273
...........................................................................................................................................................................................................................................................................................................................................................
run 1209 failures 2

Unit tests in this PR

$ make BOARD=native -C tests/unittests all test
[...]
main(): This is RIOT! (Version: 2024.10-devel-372-g2b3da-sys/nanocoap/fix-header-size-computation)
Help: Press s to start test, r to print it is ready
READY
s
START
.......................==81436==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
.....................................................................................................................................................................................................................==81436==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0xfff08000; bottom 0xed502000; size: 0x12a06000 (312500224)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
.............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
OK (1209 tests)

Issues/PRs references

None

maribu added 2 commits November 1, 2024 13:58
@maribu
sys/net/nanocoap: fix coap_get_total_hdr_len()
469edf4 
Before `coap_get_total_hdr_len()` did not take the extended TKL field
(RFC 8974) into account. This fixes the issue.
@maribu
tests/unittests: increase coverage for coap_get_total_hdr_len()
2b3da39 
Previously the corner case when RFC 8974 extended TKL fields are used
the result of coap_get_totel_hdr_len() was not tested, resulting in a
bug slipping through the test. This increase the test coverage.
@maribu maribu requested a review from miri64 as a code owner November 1, 2024 13:02
@maribu maribu requested review from benpicco and removed request for miri64 November 1, 2024 13:02
@github-actions github-actions bot added Area: network Area: Networking Area: tests Area: tests and testing framework Area: sys Area: System labels Nov 1, 2024
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: network Area: Networking Area: tests Area: tests and testing framework Area: sys Area: System labels Nov 1, 2024
@riot-ci
Copy link
Copy Markdown

riot-ci commented Nov 1, 2024

Murdock results

✔️ PASSED

2b3da39 tests/unittests: increase coverage for coap_get_total_hdr_len()

Success Failures Total Runtime
10215 0 10215 14m:48s

Artifacts

@maribu maribu added this pull request to the merge queue Nov 1, 2024
Merged via the queue into RIOT-OS:master with commit 5ae4c96 Nov 1, 2024
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 1, 2024

Thx! :)

@maribu maribu deleted the sys/nanocoap/fix-header-size-computation branch November 1, 2024 20:13
@MrKevinWeiss MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabian18 fabian18 fabian18 approved these changes

@benpicco benpicco Awaiting requested review from benpicco

Assignees

No one assigned

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2025.01

Development

Successfully merging this pull request may close these issues.

4 participants

@maribu @riot-ci @fabian18 @MrKevinWeiss