add synchronization primitives for zenoh-c (similar to zenoh-pico) #250
Conversation
|
@DenisBiryukov91 If this pull request contains a bugfix or a new feature, then please consider using |
There was a problem hiding this comment.
Great job!
Only one correction: if this api will be the same for zenoh and zenoh-pico, prefix should be z_. If they are specific for zenoh-c and differs from pico ones, they should be zc_, but it's not the case here, as far as I see.
The zp_ prefix should not appear in zenoh-c, it's for zenoh-pico specific functions.
Later in zenoh-pico we can also rename them from zp_ to z_ and maybe add defines for backward compatibility.
src/platform/synchronization.rs
Outdated
| impl_guarded_transmute!(z_mutex_t, ZMutexPtr); | ||
| impl_guarded_transmute!(ZMutexPtr, z_mutex_t); | ||
|
|
||
| const EBUSY: i8 = -1; |
There was a problem hiding this comment.
As far as I see these error values doubles the ones from pthread library (https://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_mutex_init.html). This should be at least commented and maybe exposed in the documentation.
src/platform/synchronization.rs
Outdated
| impl_guarded_transmute!(ZMutexPtr, z_mutex_t); | ||
|
|
||
| // try to use the same error codes as in GNU pthreads | ||
| const EBUSY: i8 = 16; |
There was a problem hiding this comment.
This doesn't fit the agreement, that the error value should be always negative. This agreement is used here for example: https://github.com/eclipse-zenoh/zenoh-cpp/blob/main/include/zenohcxx/impl.hxx#L59-L69
Though this agreement is already violated in zenoh-pico by these functions like z_mutex_init which directly returns values from pthread. So it seems like that it's necessary to reevaluate the assumption that error value is alvays negative
There was a problem hiding this comment.
I believe error values should always be negative.
I would consider as a bug the fact that z_mutex_init directly returns values from pthread in zenoh-pico.
There was a problem hiding this comment.
made issue in zenoh-pico
eclipse-zenoh/zenoh-pico#358
Adds support for mutexes (zp_mutex_.... functions), condvars (zp_condvar_... functions) and tasks(threads) (zp_task_... functions) similar to the one provided in zenoh-pico (partially addresses #247)