zephyr: sys: sync: Make Semaphore constructors const

Move Semaphores to the new `ZephyrObject` system, allowing the
constructor to be const.  Unfortunately, `Result` isn't generally useful
with const, and as such, this changes the API for `Semaphore::new()` to
just return the semaphore instead of a Result.  Instead, if a semaphore
constructor has inconsitent arguments, it will panic.  Given that
semaphore constructors are generally quite simple, this is a minor
issue.

However, it will involve removing a lot of `.unwrap()` or `.expect(...)`
calls on semaphores.

The samples have been changed for the API change, but mostly not changed
to take advantage of the easier ways to use them.  It is generally no
longer necessary to put semaphores in an Arc or Rc, but just to allocate
them in something like a StaticCell, and then pass them around by
reference.

Signed-off-by: David Brown <[email protected]>

Author: d3zd3z

samples/philosophers/src/dynsemsync.rs

@@ -35,7 +35,7 @@ impl ForkSync for SemSync {
 pub fn dyn_semaphore_sync() -> Vec<Arc<dyn ForkSync>> {
     let forks = [(); NUM_PHIL]
         .each_ref()
-        .map(|()| Arc::new(Semaphore::new(1, 1).unwrap()));
+        .map(|()| Arc::new(Semaphore::new(1, 1)));
 
     (0..NUM_PHIL)
         .map(|_| {

samples/philosophers/src/semsync.rs

@@ -10,15 +10,15 @@ extern crate alloc;
 use alloc::boxed::Box;
 use alloc::vec::Vec;
 
-use zephyr::{kobj_define, sync::Arc, sys::sync::Semaphore, time::Forever};
+use zephyr::{sync::Arc, sys::sync::Semaphore, time::Forever};
 
 use crate::{ForkSync, NUM_PHIL};
 
 #[derive(Debug)]
 pub struct SemSync {
     /// The forks for this philosopher.  This is a big excessive, as we really don't need all of
     /// them, but the ForSync code uses the index here.
-    forks: [Arc<Semaphore>; NUM_PHIL],
+    forks: &'static [Semaphore; NUM_PHIL],
 }
 
 impl ForkSync for SemSync {
@@ -33,22 +33,15 @@ impl ForkSync for SemSync {
 
 #[allow(dead_code)]
 pub fn semaphore_sync() -> Vec<Arc<dyn ForkSync>> {
-    let forks = SEMS.each_ref().map(|m| {
-        // Each fork starts as taken.
-        Arc::new(m.init_once((1, 1)).unwrap())
-    });
-
     (0..NUM_PHIL)
         .map(|_| {
-            let syncer = SemSync {
-                forks: forks.clone(),
-            };
+            let syncer = SemSync { forks: &SEMS };
             let item = Box::new(syncer) as Box<dyn ForkSync>;
             Arc::from(item)
         })
         .collect()
 }
 
-kobj_define! {
-    static SEMS: [StaticSemaphore; NUM_PHIL];
-}
+// The state of const array initialization in rust as dreadful.  There is array-init, which isn't
+// const, and there is array-const-fn-init, which, for some reason is a proc macro.
+static SEMS: [Semaphore; NUM_PHIL] = [const { Semaphore::new(1, 1) }; NUM_PHIL];

samples/work-philosophers/src/async_sem.rs

@@ -43,7 +43,7 @@ async fn local_phil() -> Stats {
 
     // One fork for each philospher.
     let forks: Vec<_> = (0..NUM_PHIL)
-        .map(|_| Rc::new(Semaphore::new(1, 1).unwrap()))
+        .map(|_| Rc::new(Semaphore::new(1, 1)))
         .collect();
 
     // Create all of the philosphers

zephyr/src/kio/sync.rs

@@ -62,7 +62,7 @@ impl<T> Mutex<T> {
     /// Construct a new Mutex.
     pub fn new(t: T) -> Mutex<T> {
         Mutex {
-            inner: Semaphore::new(1, 1).unwrap(),
+            inner: Semaphore::new(1, 1),
             data: UnsafeCell::new(t),
         }
     }

zephyr/src/sys/sync.rs

@@ -34,4 +34,4 @@ pub mod mutex;
 pub mod semaphore;
 
 pub use mutex::{Condvar, Mutex, StaticCondvar, StaticMutex};
-pub use semaphore::{Semaphore, StaticSemaphore};
+pub use semaphore::Semaphore;

zephyr/src/sys/sync/semaphore.rs

@@ -16,8 +16,6 @@ use core::fmt;
 #[cfg(CONFIG_RUST_ALLOC)]
 use core::future::Future;
 #[cfg(CONFIG_RUST_ALLOC)]
-use core::mem;
-#[cfg(CONFIG_RUST_ALLOC)]
 use core::pin::Pin;
 #[cfg(CONFIG_RUST_ALLOC)]
 use core::task::{Context, Poll};
@@ -27,22 +25,19 @@ use zephyr_sys::ETIMEDOUT;
 
 #[cfg(CONFIG_RUST_ALLOC)]
 use crate::kio::ContextExt;
+use crate::object::{ObjectInit, ZephyrObject};
 #[cfg(CONFIG_RUST_ALLOC)]
 use crate::time::NoWait;
 use crate::{
     error::{to_result_void, Result},
-    object::{Fixed, StaticKernelObject, Wrapped},
     raw::{k_sem, k_sem_count_get, k_sem_give, k_sem_init, k_sem_reset, k_sem_take},
     time::Timeout,
 };
 
 pub use crate::raw::K_SEM_MAX_LIMIT;
 
-/// A zephyr `k_sem` usable from safe Rust code.
-pub struct Semaphore {
-    /// The raw Zephyr `k_sem`.
-    pub(crate) item: Fixed<k_sem>,
-}
+/// General Zephyr Semaphores
+pub struct Semaphore(pub(crate) ZephyrObject<k_sem>);
 
 /// By nature, Semaphores are both Sync and Send.  Safety is handled by the underlying Zephyr
 /// implementation (which is why Clone is also implemented).
@@ -55,13 +50,27 @@ impl Semaphore {
     /// Create a new dynamically allocated Semaphore.  This semaphore can only be used from system
     /// threads.  The arguments are as described in [the
     /// docs](https://docs.zephyrproject.org/latest/kernel/services/synchronization/semaphores.html).
+    ///
+    /// Note that this API has changed, and it now doesn't return a Result, since the Result time
+    /// generally doesn't work (in stable rust) with const.
     #[cfg(CONFIG_RUST_ALLOC)]
-    pub fn new(initial_count: c_uint, limit: c_uint) -> Result<Semaphore> {
-        let item: Fixed<k_sem> = Fixed::new(unsafe { mem::zeroed() });
+    pub const fn new(initial_count: c_uint, limit: c_uint) -> Semaphore {
+        // Due to delayed init, we need to replicate the object checks in the C `k_sem_init`.
+
+        if limit == 0 || initial_count > limit {
+            panic!("Invalid semaphore initialization");
+        }
+
+        let this = <ZephyrObject<k_sem>>::new_raw();
+
         unsafe {
-            to_result_void(k_sem_init(item.get(), initial_count, limit))?;
+            let addr = this.get_uninit();
+            (*addr).count = initial_count;
+            (*addr).limit = limit;
         }
-        Ok(Semaphore { item })
+
+        // to_result_void(k_sem_init(item.get(), initial_count, limit))?;
+        Semaphore(this)
     }
 
     /// Take a semaphore.
@@ -74,7 +83,7 @@ impl Semaphore {
         T: Into<Timeout>,
     {
         let timeout: Timeout = timeout.into();
-        let ret = unsafe { k_sem_take(self.item.get(), timeout.0) };
+        let ret = unsafe { k_sem_take(self.0.get(), timeout.0) };
         to_result_void(ret)
     }
 
@@ -98,7 +107,7 @@ impl Semaphore {
     /// This routine gives to the semaphore, unless the semaphore is already at its maximum
     /// permitted count.
     pub fn give(&self) {
-        unsafe { k_sem_give(self.item.get()) }
+        unsafe { k_sem_give(self.0.get()) }
     }
 
     /// Resets a semaphor's count to zero.
@@ -108,14 +117,32 @@ impl Semaphore {
     ///
     /// [`take`]: Self::take
     pub fn reset(&self) {
-        unsafe { k_sem_reset(self.item.get()) }
+        unsafe { k_sem_reset(self.0.get()) }
     }
 
     /// Get a semaphore's count.
     ///
     /// Returns the current count.
     pub fn count_get(&self) -> usize {
-        unsafe { k_sem_count_get(self.item.get()) as usize }
+        unsafe { k_sem_count_get(self.0.get()) as usize }
+    }
+}
+
+impl ObjectInit<k_sem> for ZephyrObject<k_sem> {
+    fn init(item: *mut k_sem) {
+        // SAFEFY: Get the initial values used in new.  The address may have changed, but only due
+        // to a move.
+        unsafe {
+            let count = (*item).count;
+            let limit = (*item).limit;
+
+            if k_sem_init(item, count, limit) != 0 {
+                // Note that with delayed init, we cannot do anything with invalid values.  We're
+                // replicated the check in `new` above, so would only catch semantic changes in the
+                // implementation of `k_sem_init`.
+                unreachable!();
+            }
+        }
     }
 }
 
@@ -153,36 +180,6 @@ impl<'a> Future for SemTake<'a> {
     }
 }
 
-/// A static Zephyr `k_sem`.
-///
-/// This is intended to be used from within the `kobj_define!` macro.  It declares a static ksem
-/// that will be properly registered with the Zephyr kernel object system.  Call [`init_once`] to
-/// get the [`Semaphore`] that is represents.
-///
-/// [`init_once`]: StaticKernelObject::init_once
-pub type StaticSemaphore = StaticKernelObject<k_sem>;
-
-unsafe impl Sync for StaticSemaphore {}
-
-impl Wrapped for StaticKernelObject<k_sem> {
-    type T = Semaphore;
-
-    /// The initializer for Semaphores is the initial count, and the count limit (which can be
-    /// K_SEM_MAX_LIMIT, re-exported here.
-    type I = (c_uint, c_uint);
-
-    // TODO: Thoughts about how to give parameters to the initialzation.
-    fn get_wrapped(&self, arg: Self::I) -> Semaphore {
-        let ptr = self.value.get();
-        unsafe {
-            k_sem_init(ptr, arg.0, arg.1);
-        }
-        Semaphore {
-            item: Fixed::Static(ptr),
-        }
-    }
-}
-
 impl fmt::Debug for Semaphore {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "sys::Semaphore")

zephyr/src/work/futures.rs

@@ -68,7 +68,7 @@ impl<T> Answer<T> {
     pub fn new() -> Self {
         Self {
             item: Mutex::new(None),
-            wake: Semaphore::new(0, 1).expect("Initialize semaphore"),
+            wake: Semaphore::new(0, 1),
         }
     }
 
@@ -307,7 +307,7 @@ impl WakeInfo {
                 ev.get(),
                 ZR_POLL_TYPE_SEM_AVAILABLE,
                 k_poll_modes_K_POLL_MODE_NOTIFY_ONLY as i32,
-                sem.item.get() as *mut c_void,
+                sem.0.get() as *mut c_void,
             );
         }
     }