pkg/lwip / pkg/tinydtls: use new mbox_avail() / mbox_unset() function.#15484
pkg/lwip / pkg/tinydtls: use new mbox_avail() / mbox_unset() function.#15484miri64 merged 2 commits intoRIOT-OS:masterfrom
Conversation
miri64
added
Area: network
miri64
changed the title
kaspar030
left a comment
kaspar030
left a comment
There was a problem hiding this comment.
ACK. Straight forward replace of cib_avail(&mbox->cib) with mbox_avail(mbox)
miri64
changed the title
miri64
dismissed
kaspar030’s stale review
Sorry, decided last minute to also create and use mbox_unset()
benpicco
added
the
CI: ready for build
core/include/mbox.h
Outdated
| } | ||
|
|
||
| /** | ||
| * @brief Unset's the mbox, effectively deinitializing invalidating it. |
There was a problem hiding this comment.
both deinitializing and invalidating? Comma missing? :)
This leaks a possibly supplied queue. Maybe better call it mbox_release_queue?
The mbox actually works without queue, it'd just be synchronous from that point on.
This is also a bit dangerous, if this is called with messages in the queue, they get lost. If there's a sender that was previously waiting, it would be stuck there until the queue transitions from full to empty again.
Why is this needed?
There was a problem hiding this comment.
(I suggest splitting, the other changes were already ACKed...)
There was a problem hiding this comment.
Why is this needed?
Scroll the code and you see, that lwIP requires a way to invalidate their mboxes and check the validiy.
RIOT/pkg/lwip/include/arch/sys_arch.h
Lines 101 to 111 in dd1de91
I agree that the current approach is not the correct one, maybe. Any idea how we should proceed with that?
(I suggest splitting, the other changes were already ACKed...)
Can do.
miri64
force-pushed
the
mbox/enh/use-new-api
branch
from
22ec43a to
ef5e034
Compare
|
Reverted to ACK'd state, as suggested in #15484 (comment) |
3 participants
Contribution description
Follow-up on #15478
Testing procedure
The following tests should still compile and run successfully:
tests/lwiptests/lwip_sock_ip(both withLWIP_IPV4=1andLWIP_IPV6=1and their combination)tests/lwip_sock_tcp(both withLWIP_IPV4=1andLWIP_IPV6=1and their combination)tests/lwip_sock_udp(both withLWIP_IPV4=1andLWIP_IPV6=1and their combination)tests/pkg_tinydtls_sock_async/Issues/PRs references
Follow-up on #15478