-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added semaphore support for Linux platform #83
base: master
Are you sure you want to change the base?
Added semaphore support for Linux platform #83
Conversation
CristianDragu
commented
May 2, 2019
- most of the implementation added
718b61b
to
b2edc44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The patch seems to be missing your Posix interface
[ct] if platformName == "Linux" | ||
datatype NativeThreadHandle = ULong | ||
else | ||
datatype NativeThreadHandle = Byte Ptr // Opaque type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor, not introduced in this patch]
Change to UntypedPtr
import std.ptr | ||
|
||
datatype NativeThreadHandle = Byte Ptr // Opaque type | ||
[ct] if platformName == "Linux" | ||
datatype NativeThreadHandle = ULong |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you also need to add the >>
operator for the new type (if I remember correctly it will not be autogenerated, so you can't print a NativeThreadHandle)
|
||
using InvalidThreadHandle = NativeThreadHandle() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't define InvalidThreadHandle
for Linux
[native("sysconf")] fun sysconf(name: Int): Long | ||
[ct] if platformName == "Darwin" | ||
using _SC_NPROCESSORS_ONLN = 58 | ||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also test for Linux on the else branch. Just to be sure
import config | ||
|
||
datatype TimeSpecT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be public to this file
[ct] if platformName == "Darwin" | ||
var res = _Impl.semaphore_create(_Impl.mach_task_self(), _handle, _Impl.SYNC_POLICY_FIFO, Int(startValue)) | ||
else | ||
var res = _Impl.sem_init(_handle, Int(0), UInt(startValue)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't you implement this in a Posix header?