core: add support to see if there are messages available for the curr…#3997
core: add support to see if there are messages available for the curr…#3997OlegHahm merged 1 commit intoRIOT-OS:masterfrom
Conversation
|
Could you add a simple test case for the new function? I suggest changing the name to |
1b508e2 to
b3440ad
Compare
|
Addressed name change and unit test. |
tests/thread_msg_avail/Makefile
Outdated
There was a problem hiding this comment.
Are you sure? This application just uses core. If stm32f0discovery does not fit core we really reevaluate support for this platform ;-)
There was a problem hiding this comment.
I copied it from thread_msg but I guess it has insufficient memory there because of 3 threads with printf support, sec ;)
b3440ad to
9c5a42a
Compare
core/msg.c
Outdated
There was a problem hiding this comment.
Replace sched_threads[sched_active_pid] with sched_active_thread.
There was a problem hiding this comment.
I copy/pasted the code from _msg_receive should it get changed there aswell?
There was a problem hiding this comment.
Yes, please, and in msg_send_receive, too. But that should go into a new commit. Could be that I am missing a use case where sched_threads[sched_active_pid] != sched_active_thread, but I think that these two need to match in all cases but for a short moment then switching threads.
There was a problem hiding this comment.
The sched_threads[sched_active_pid] changes should go in a separate PR. The reviewer needs to look up all write occurences of sched_active_thread and sched_active_pid to ensure that we are not introducing a bug (possibly a race while switching threads) in the message passing system.
There was a problem hiding this comment.
@gebart So for this PR I keep using sched_threads[sched_active_pid] to be on the save side?
There was a problem hiding this comment.
I was primarily talking about the existing functions which use sched_threads[sched_active_pid], since it depends on where the function is used if there will be any possible races or not. I guess @Kijewski might have an opinion here?
There was a problem hiding this comment.
In here you can use sched_active_thread. For the other functions we need to look if they [are/may be] used in interrupt handlers. I am quite sure they must not be called by an interrupt handler, because it would simply make no sense if I am not mistaken.
|
@DipSwitch, can you address @Kijewski's latest small comment? |
|
Kijewski: Replace sched_threads[sched_active_pid] with sched_active_thread. Me: I copy/pasted the code from _msg_receive should it get changed there aswell? Kijewski: Yes, please, and in msg_send_receive, too. But that should go into a new commit. Could be that I am missing a use case where sched_threads[sched_active_pid] != sched_active_thread, but I think that these two need to match in all cases but for a short moment then switching threads. Do I make a pull request for the other lines as well? I'll squash if you validated the last commit addressing @Kijewski's points. |
|
Validated. Please squash. |
e89d5a6 to
1c8d81f
Compare
|
squashed |
core/include/msg.h
Outdated
There was a problem hiding this comment.
@return -1 if the caller's message queue was not initialized.
|
Please add the line of documentation about the error case. You can squash immediately. Other than that: ACK. Needs a seconds ACK, because it's a core change. |
1c8d81f to
15e8f4e
Compare
|
Done |
|
Travis failed on |
|
Travis is happy |
|
ACK and go! |
core: add support to see if there are messages available for the curr…
Add support to check if there are messages queued for the current thread.
This will later be used for
-EWOULDBLOCKerror code.