tests/gnrc_ndp: don't guess size of gnrc_pktsnip_t in pktbuf#16051
Merged
miri64 merged 2 commits intoRIOT-OS:masterfrom Feb 19, 2021
Merged
tests/gnrc_ndp: don't guess size of gnrc_pktsnip_t in pktbuf#16051miri64 merged 2 commits intoRIOT-OS:masterfrom
gnrc_pktsnip_t in pktbuf#16051miri64 merged 2 commits intoRIOT-OS:masterfrom
Conversation
miri64
added
Type: bug
miri64
requested review from
PeterKietzmann,
cgundogan and
haukepetersen
as code owners
miri64
added
the
CI: ready for build
Member
Author
|
A cleaner approach could be to rewrite the whole test using a packet buffer mock to get into those error conditions, but this is the simpler approach for now ;-). |
fjmolinas
reviewed
Feb 19, 2021
Contributor
|
Test now passes on AVR: |
Contributor
|
Running the test on some more platforms... |
miri64
force-pushed
the
tests/fix/avr-gnrc_ndp
branch
from
a027d9d to
cb8afd3
Compare
Member
Author
|
My latest fixup just fixes the static tests. |
Contributor
|
Ran the tests on multiple BOARD's all good https://ci.inria.fr/ci-riot-tribe/job/build-pipeline.jk/233/artifact/, the two boards that fail is because I setup wrong udev-rules form them, something I need to address when I'm at the office. |
Contributor
|
Please squash! |
24 bytes is only the right size for `gnrc_pktsnip_t` on 32-bit systems (and only if the struct does not change), on AVR e.g. it 9 bytes. By using the internal alignment function of `pktbuf` we are able to precisely tell the internal size of `gnrc_pktsnip_t` using the `sizeof` operator.
miri64
force-pushed
the
tests/fix/avr-gnrc_ndp
branch
from
5957127 to
455a607
Compare
Member
Author
|
Squashed. |
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
From #12651
The problem is that the tests makes some assumptions that do not hold true for every platform: For testing the "packet buffer is full" error case, it fills the packet buffer with just enough data to be able to progress the function under test to a certain point and then fail. For this it assumes the size of a
gnrc_pktsnip_tto be 24 bytes. However, 24 bytes is only the right size forgnrc_pktsnip_ton 32-bit systems (and only if the struct does not change), on AVR e.g. it 9 bytes. By using the internal alignment function ofpktbufwe are able to precisely tell the internal size ofgnrc_pktsnip_tusing thesizeofoperator.Testing procedure
Run
make -C tests/gnrc_ndp -j flash teston an 8-bit platform (that fits it) and it should succeed now. Also try to run the test on several other platforms to make sure it does not introduce a regression.
Issues/PRs references
Should fix #12651 for good.