sem: initial import of a lightweight semaphore layer#3549
sem: initial import of a lightweight semaphore layer#3549miri64 merged 7 commits intoRIOT-OS:masterfrom
Conversation
sys/posix/semaphore.c
Outdated
There was a problem hiding this comment.
This block could be replaced by calls to atomic_cas et. al. (atomic.h)
There was a problem hiding this comment.
Can you explain to me how?
There was a problem hiding this comment.
on second thought, it might not really give any benefits and only make the code less readable, ignore my comment for now. I did not think about the if (value == 0) case before.
d8737e4 to
de9f971
Compare
sys/sem/sem.c
Outdated
There was a problem hiding this comment.
while ((next = priority_queue_remove_head(&sem->queue)) != NULL) { or you'll have a loop.
There was a problem hiding this comment.
Well I definitely want a loop, just not an endless one. Anyway: fixed.
|
Please see miri64#15, I'd propose an extra argument so spurious messages received while waiting for the semaphore are not lost. |
|
Took it. |
Makefile.dep
Outdated
There was a problem hiding this comment.
sem needs it own section, since it depends on vtimer.
|
ACK but for the Makefile.dep comment. Squash and merge when Travis is happy. I am curious about #3551. :) |
|
Mh… actually. Since your fix |
|
m( a clean helped very much |
|
As a bonus I adapted the posix semaphores to the same hierarchy I'm trying to introduce with sockets. ( |
|
@authmillenon please squash |
4a2a006 to
d09a1c0
Compare
|
Squashed |
|
Needs rebase |
9f2d79b to
9a9a0c4
Compare
|
Rebased |
|
@Kijewski would you like to take over this assignment? I don't think I will have time to review this properly in the foreseeable future. |
|
@authmillenon, did you only move posix_sem into its own module since my last review? If yes, then ACK. |
|
I'm not sure, but think so. |
|
Oops, missed a comment. |
I deny any responsibility for my errors!! :D |
sys/include/sem.h
Outdated
|
Addressed (not squashed immediately, since this change is a little bigger). |
fc07cdc to
f42e867
Compare
sys/posix/include/semaphore.h
Outdated
There was a problem hiding this comment.
(better to comment in the PR than in the commit)
remove the leading underscore and add a trailing if you like
|
Addressed comments |
|
I realized that the posix specifications and our sem module differ in the return value. The posix documents specify that we should return -1 on error and set errno. Riot doesn't use errno except for posix compatibility functions. One solution is to modify the posix header to apply a thin wrapper on the riot functions, but there is a name collision between posix semaphore.h and riot sem. |
|
I realized that, too when I reworked the doc, but decided to keep it that
way for now. In most cases an application will just test `res < 0`, so this
shouldn't be an issue. In a later iteration we can make sure to adhere more
to the POSIX specification. The point of this PR was to get semaphores out
of POSIX again, not to fix the already faulty POSIX layer ;).
|
|
@authmillenon |
|
Done: #4109 |
|
ACK, merge when Travis agrees |
6737f78 to
f050c4e
Compare
|
Squashed |
sem: initial import of a lightweight semaphore layer
Reinstitutes semaphores as their own module and takes some POSIX dependent stuff out. Also moves it from a purely concept based scheduler magic to an IPC-based concept to allow for wait timeouts.
This is the first PR in the effort to port lwIP for RIOT.