Fix sem_trywait and sem_wait error handling

src/header/semaphore/mod.rs

@@ -3,8 +3,15 @@
 //! See <https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/semaphore.h.html>.
 
 use crate::{
-    header::time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec},
-    platform::types::{c_char, c_int, c_long, c_uint, clockid_t},
+    error::ResultExt,
+    header::{
+        errno,
+        time::{self, timespec},
+    },
+    platform::{
+        self,
+        types::{c_char, c_int, c_long, c_uint, clockid_t},
+    },
 };
 
 /// See <https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/semaphore.h.html>.
@@ -67,9 +74,12 @@ pub unsafe extern "C" fn sem_post(sem: *mut sem_t) -> c_int {
 /// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_trywait.html>.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn sem_trywait(sem: *mut sem_t) -> c_int {
-    unsafe { get(sem) }.try_wait();
-
-    0
+    if unsafe { get(sem) }.try_wait() {
+        0
+    } else {
+        platform::ERRNO.set(errno::EAGAIN);
+        -1
+    }
 }
 
 /// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_unlink.html>.
@@ -81,9 +91,10 @@ pub unsafe extern "C" fn sem_unlink(name: *const c_char) -> c_int {
 /// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_trywait.html>.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn sem_wait(sem: *mut sem_t) -> c_int {
-    if let Ok(()) = unsafe { get(sem) }.wait(None, CLOCK_MONOTONIC) {}; // TODO handle error
-
-    0
+    unsafe { get(sem) }
+        .wait(None, time::CLOCK_MONOTONIC)
+        .map(|_| 0)
+        .or_minus_one_errno()
 }
 
 /// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_clockwait.html>.
@@ -93,18 +104,19 @@ pub unsafe extern "C" fn sem_clockwait(
     clock_id: clockid_t,
     abstime: *const timespec,
 ) -> c_int {
-    if let Ok(()) = unsafe { get(sem) }.wait(Some(&unsafe { (*abstime).clone() }), clock_id) {}; // TODO handle error
-
-    0
+    unsafe { get(sem) }
+        .wait(Some(&unsafe { (*abstime).clone() }), clock_id)
+        .map(|_| 0)
+        .or_minus_one_errno()
 }
 
 /// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_timedwait.html>.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn sem_timedwait(sem: *mut sem_t, abstime: *const timespec) -> c_int {
-    if let Ok(()) = unsafe { get(sem) }.wait(Some(&unsafe { (*abstime).clone() }), CLOCK_REALTIME) {
-    }; // TODO handle error
-
-    0
+    unsafe { get(sem) }
+        .wait(Some(&unsafe { (*abstime).clone() }), time::CLOCK_REALTIME)
+        .map(|_| 0)
+        .or_minus_one_errno()
 }
 
 unsafe fn get<'any>(sem: *mut sem_t) -> &'any RlctSempahore {

src/sync/semaphore.rs

@@ -2,8 +2,16 @@
 //TODO: improve implementation
 
 use crate::{
-    header::time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec, timespec_realtime_to_monotonic},
-    platform::types::{c_uint, clockid_t},
+    error::{Errno, Result},
+    header::{
+        errno,
+        time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec, timespec_realtime_to_monotonic},
+    },
+    platform::{
+        Pal, Sys,
+        types::{c_uint, clockid_t},
+    },
+    sync::FutexAtomicTy,
 };
 
 use core::sync::atomic::{AtomicU32, Ordering};
@@ -27,12 +35,12 @@ impl Semaphore {
         crate::sync::futex_wake(&self.count, i32::MAX);
     }
 
-    pub fn try_wait(&self) -> u32 {
+    pub fn try_wait(&self) -> bool {
         loop {
             let value = self.count.load(Ordering::SeqCst);
 
             if value == 0 {
-                return 0;
+                return false;
             }
 
             if self
@@ -41,34 +49,29 @@ impl Semaphore {
                 .is_ok()
             {
                 // Acquired
-                return value;
+                return true;
             }
             // Try again (as long as value > 0)
         }
     }
 
-    pub fn wait(&self, timeout_opt: Option<&timespec>, clock_id: clockid_t) -> Result<(), ()> {
+    pub fn wait(&self, timeout_opt: Option<&timespec>, clock_id: clockid_t) -> Result<()> {
         loop {
-            let value = self.try_wait();
-
-            if value == 0 {
+            if self.try_wait() {
                 return Ok(());
             }
-
+            // value must be zero
             if let Some(timeout) = timeout_opt {
                 let relative = match clock_id {
                     // FUTEX expect monotonic clock
                     CLOCK_MONOTONIC => timeout.clone(),
-                    CLOCK_REALTIME => match timespec_realtime_to_monotonic(timeout) {
-                        Ok(relative) => relative,
-                        Err(_) => return Err(()),
-                    },
-                    _ => return Err(()),
+                    CLOCK_REALTIME => timespec_realtime_to_monotonic(timeout)?,
+                    _ => return Err(Errno(errno::EINVAL)),
                 };
-                crate::sync::futex_wait(&self.count, value, Some(&relative));
+                unsafe { Sys::futex_wait(self.count.ptr(), 0, Some(&relative))? };
             } else {
                 // Use futex to wait for the next change, without a timeout
-                crate::sync::futex_wait(&self.count, value, None);
+                unsafe { Sys::futex_wait(self.count.ptr(), 0, None)? };
             }
         }
     }

tests/Makefile.tests.mk

@@ -70,6 +70,7 @@ EXPECT_NAMES=\
 	locale/setlocale \
 	math \
 	regex \
+	semaphore/lock \
 	setjmp \
 	sigaction \
 	sigaltstack \

tests/semaphore/lock.c

@@ -0,0 +1,29 @@
+#include <semaphore.h>
+#include <errno.h>
+
+#include "test_helpers.h"
+
+int main(void)
+{
+    sem_t sem;
+    int status;
+
+    status = sem_init(&sem, 0, 1);
+    ERROR_IF(sem_init, status, == -1);
+
+    status = sem_trywait(&sem);
+    ERROR_IF(sem_trywait, status, == -1);
+
+    status = sem_trywait(&sem);
+    UNEXP_IF(sem_trywait, status, != -1);
+    UNEXP_IF(errno, errno, != EAGAIN);
+
+    status = sem_post(&sem);
+    ERROR_IF(sem_post, status, == -1);
+
+    status = sem_wait(&sem);
+    ERROR_IF(sem_wait, status, == -1);
+
+    status = sem_destroy(&sem);
+    ERROR_IF(sem_destroy, status, == -1);
+}