core (largely doc): Differentiate message types from thread flags #17472
Conversation
This has been wrong since 317b013 when a third predefined flag was removed.
chrysn
added
Type: enhancement
core/include/thread_flags.h
Outdated
| * | ||
| * This bit mask is set for all thread flag bits that are predefined in RIOT. | ||
| * Flags within this set may be set on a thread by the operating system without | ||
| * thre thread soliciting them (though not all are; for example, @ref |
There was a problem hiding this comment.
| * thre thread soliciting them (though not all are; for example, @ref | |
| * the thread soliciting them (though not all are; for example, @ref |
core/include/thread_flags.h
Outdated
| * When using custom flags, asserting that they are not in this set can help | ||
| * avoid conflict with future additions to the predefined flags. | ||
| */ | ||
| #define THREAD_FLAG_PREDEFINED THREAD_FLAG_MSG_WAITING | THREAD_FLAG_TIMEOUT |
There was a problem hiding this comment.
| #define THREAD_FLAG_PREDEFINED THREAD_FLAG_MSG_WAITING | THREAD_FLAG_TIMEOUT | |
| #define THREAD_FLAG_PREDEFINED (THREAD_FLAG_MSG_WAITING | THREAD_FLAG_TIMEOUT) |
core/include/thread_flags.h
Outdated
| @@ -98,6 +101,20 @@ extern "C" { | |||
| * @see xtimer_set_timeout_flag | |||
| */ | |||
| #define THREAD_FLAG_TIMEOUT (1u << 14) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
That was originally intentional to set it off from the others -- revisiting it, given the others don't have newlines between them, added.
core/include/msg.h
Outdated
| * GNRC_NETAPI_MSG_TYPE_RCV & co are defined. These are fixed in the sense that | ||
| * registering for a particular set of messages (for the above, e.g. @ref | ||
| * gnrc_netreg_register) will use these message types. Threads that do nothing | ||
| * to receive such messages need not worry can safely use the same numbers for |
There was a problem hiding this comment.
| * to receive such messages need not worry can safely use the same numbers for | |
| * to receive such messages need can safely use the same numbers for |
|
LGTM, nice clarifications. Only the "may be considered an error", maybe needs some addition, or remove it? What do you think? |
|
All applied, including an addition on best practice to log. (Although I think I'm more on the side of "log with extreme prejudice"). I'd advocate declaring this clearly as an error (not merely "usage error") not for the behavior when receiving an unexpected message, but for how it can go wrong worse: Whichever component sends a thread messages that the recipient does not expect might also hit a message type the recipient does expect, and then the recipient might free what's behind that pointer (if the semantics are similar to what is done in GNRC), and generally we're in deep UB and potential-security-issue land there. (Say the sender thinks of the message as a pointer to some user input buffer, but the recipient thinks of it as a struct and calls through a pointer in there...) |
chrysn
removed
the
CI: ready for build
|
(CI-ready flag cleared as nothing showed up in 1/2h of running, and it'll be run again after squashing anyway). |
|
Can I squash this? |
|
@kaspar030 is this Ok to squash? |
|
Please squash! |
chrysn
force-pushed
the
doc-flags-msgs
branch
from
405bcc4
to
faba8e1
Compare
chrysn
added
the
CI: ready for build
Co-authored-by: benpicco <benpicco@googlemail.com>
chrysn
force-pushed
the
doc-flags-msgs
branch
from
361f8c7
to
b923fec
Compare
|
Thank you! Now that these semantics are clarified I can go ahead and send zero-sized types (like exclusive access to some globals) between threads on flags :-) |
Contribution description
How to make sure own thread flags or message types do not conflict with preexisting ones (especially with the lack of a comprehensive of predefined ones for the latter) is difficult from the current documentation; this change set:
(and fixes an obsolete number in the process).
Testing procedure
Issues/PRs references
This contains clean-up from #7717