gnrc_sixlowpan_frag_vrb: fix issues with interval marker inherited from base#12848
Merged
aabadie merged 3 commits intoRIOT-OS:masterfrom Jan 15, 2020
Merged
Conversation
`gnrc_sixlowpan_frag_rb_base_rm()` cleans up the intervals which is part of `gnrc_sixlowpan_frag_rb`, not `gnrc_sixlowpan_frag`, so when the `gnrc_sixlowpan_frag` is not compiled in, but `gnrc_sixlowpan_frag_rb`, the intervals allocated in the reassembly buffer and inherited by the virtual reassembly buffer are never released.
miri64
added
Type: bug
benpicco
added
the
CI: ready for build
Member
Author
|
From #12648 (comment):
After two days of bug hunting I finally found that leak and provided a fix (a4e41b8) here, as it was an issue within the VRB, as resends are a feature of SFR, I added the test case |
miri64
changed the title
Contributor
|
I'm getting a segmention fault on this one (or hard fault if none native). |
Member
Author
|
The latest fixup should fix that. It unrolls the definition of |
fjmolinas
added
CI: run tests
Contributor
|
Shouldn;t it be; |
Contributor
|
I'm getting |
Member
Author
|
Did you patch in the patch above and added the |
Contributor
My bad I though those where two test cases:
sorry |
Member
Author
There are two test cases, but they are
;-) |
Contributor
|
EDIT apparently I hadn't cleaned properly. |
Contributor
|
Sorry for all the noise, I was compiling with
Details
Details |
Member
Author
|
Squashed. |
miri64
force-pushed
the
gnrc_sixlowpan_frag_rb/fix/interval-fixes
branch
from
749f534 to
05d0c48
Compare
fjmolinas
removed
the
CI: ready for build
Contributor
|
Travis complained: Please re-schedule build when addressed. |
Member
Author
|
Is |
miri64
force-pushed
the
gnrc_sixlowpan_frag_rb/fix/interval-fixes
branch
from
05d0c48 to
98db58c
Compare
Member
Author
|
I added a suppression comment. I'll have an I on the Travis output in case this wasn't enough. |
Contributor
I though the same thing hahaha... |
Member
Author
|
That did not seem to be it :-/. |
Member
Author
|
It still points to line 100 though... :-/ |
Otherwise the list in `base->ints` will get lost.
miri64
force-pushed
the
gnrc_sixlowpan_frag_rb/fix/interval-fixes
branch
from
98db58c to
d9ecc0b
Compare
Member
Author
|
I think |
miri64
added
the
CI: ready for build
Member
Author
|
It's passing now. |
5 tasks
Member
Author
Errors are unrelatedI rerun without tests. |
miri64
added
CI: ready for build
Contributor
|
My ACK stil stand, feel free to merge when it is green. |
keestux
reviewed
Aug 3, 2020
| if (tmp == base->ints) { | ||
| tmp = NULL; | ||
| } | ||
| /* cppcheck-suppress nullPointer |
Contributor
There was a problem hiding this comment.
@miri64 Can you confirm, just for my sake, why this comment is correct?
tmp can't clearly be a NULL pointer here
If that statement is true then the if right above should never be taken.
Or did you perhaps forgot to add a break?
Member
Author
There was a problem hiding this comment.
Mhh... now that you say that, it seems as clear as day ^^". Yes, there is a break missing.
Contributor
There was a problem hiding this comment.
BTW Thanks to Coverity Scan
Member
Author
There was a problem hiding this comment.
Well, cppcheck also picked it up, I was just stupid ;-P
This was referenced Aug 3, 2020
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
5 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
gnrc_sixlowpan_frag_rb_base_rm()cleans up the intervals which is part ofgnrc_sixlowpan_frag_rb, notgnrc_sixlowpan_frag, so when thegnrc_sixlowpan_fragis not compiled in, butgnrc_sixlowpan_frag_rb, the intervals allocated in the reassembly buffer and inherited by the virtual reassembly buffer are never released.This also adds an error return for the reassembly buffer, when the interval buffer is full.
Testing procedure
Should still pass.
When applying the following patch
(so the interval provided with the base can be changed and reset after each test and the removal (end == 0) is checked in the remove test).
should also succeed.
Issues/PRs references
None