Change version to 0.9, merging breaking changes

Merges: #109

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "riot-wrappers"
-version = "0.8.999" # really 0.9.0-alpha.1, but we also try hard to not break realistic 0.8 users who fixed all deprecation warnings and don't hold things wrong.
+version = "0.9.0"
 authors = ["Christian Amsüss <[email protected]>"]
 edition = "2021"
 rust-version = "1.77.0"

NEWS.md

@@ -0,0 +1,64 @@
+# Changes in 0.9.0
+
+## Breaking changes
+
+*These items will require code changes for all who use them.*
+
+* VFS: Opening directories now takes a pinned slot to store the open directory in.
+
+* ZTimer: Sleep functions were renamed for consistency
+
+  * `sleep()` became `sleep_extended()`
+
+  * `sleep_ticks()` became `sleep()` and takes `Ticks(u32)` instead of a plain `u32`
+
+* Support for all outdated traits was removed:
+
+  * `embedded-hal` 0.2 (except for ADC and SPI)
+
+  * `coap-handler` 0.1 and `coap-message` 0.2
+
+  * `embedded-nal-async` 0.6
+
+## Subtle breakage
+
+*These items will only require code changes for users who had unresolved deprecation warnings,
+or who explicitly named clock or error types.*
+
+* All deprecated items were removed.
+
+* gcoap: `PacketBuffer` now has a lifetime.
+
+* saul: `Unit` is non-exhaustive.
+
+* shell: `.and()` returns an `impl CommandList` instead of a `Command`.
+  This drastically enhances usability because chaining is now possible without extra effort going into type annotations.
+
+* ztimer: Clocks are now generated as `ValueInThread<Clock<HZ>>`
+  and require to stay in a thread to allow using the `sleep{,_*}` methods.
+
+* Traits that previously used `!` as associated error type
+  now use `Infallible`.
+
+* Dependencies: heapless is now used in version 0.8.
+  This is a public dependency because `saul::Unit::name_owned` returns a buffer.
+
+## Enhancements
+
+* gpio: `.toggle()` was added.
+
+* shell: `.run_forever()` and `.run_once()` are now available with provided buffers;
+  the previous `…_providing_buf()` variants are now deprecated aliases.
+
+* ztimer: Timers can be `.acquire()`'d and then provide a `.now()` method,
+  as provide a `.time()` method to measure the execution time of a closure.
+
+## Bug fixes
+
+* VFS: Fixed out-of-bounds read for file names that were not nul-terminated.
+
+## Internal changes
+
+* Tests were added for shell and ztimer.
+
+* Rust shows no more warnings for this crate.

src/vfs.rs

@@ -14,6 +14,9 @@
 
 use core::marker::PhantomData;
 use core::mem::MaybeUninit;
+use core::pin::Pin;
+
+use pin_project::{pin_project, pinned_drop};
 
 use riot_sys::libc;
 
@@ -141,39 +144,106 @@ impl Drop for File {
     }
 }
 
+/// A place where a [Dir] can be stored
+///
+/// See [`Dir::open()`] for why this is necessary.
+///
+/// As for its implementation, this is built from an Option rather than a bare MaybeUninit because
+/// if the created Dir is leaked and then the DirSlot is dropped, the DirSlot needs to know whether
+/// or not to do any cleanup.
+///
+/// ## Invariants
+///
+/// This module maintains that the MaybeUninit is always initialized outside of its own functions,
+/// and that no panicing functions are called while it is uninit.
+#[derive(Default)]
+#[pin_project(PinnedDrop)]
+pub struct DirSlot(
+    #[pin] Option<MaybeUninit<riot_sys::vfs_DIR>>,
+    core::marker::PhantomPinned,
+);
+
+impl DirSlot {
+    /// Cleanly replace any Some with None.
+    fn close(mut self: Pin<&mut Self>) {
+        if let Some(mut dir) = self.as_mut().project().0.as_pin_mut() {
+            // unsafe: dir was initialized per invariants, so it's OK to call the function.
+            // The return value is ignored because we can't do anything about it.
+            unsafe { riot_sys::vfs_closedir(dir.as_mut_ptr()) };
+        }
+        // unsafe: The MaybeUninit is uninitialized now thanks to closedir, and is thus free to be
+        // replaced.
+        *{ unsafe { Pin::into_inner_unchecked(self.project().0) } } = None;
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for DirSlot {
+    fn drop(self: Pin<&mut Self>) {
+        self.close();
+    }
+}
+
 /// A directory in the file system
 ///
 /// The directory can be iterated over, producing directory entries one by one.
-#[repr(transparent)]
-pub struct Dir(riot_sys::vfs_DIR, core::marker::PhantomPinned);
-
-impl Dir {
-    pub fn open(dir: &str) -> Result<Self, NumericError> {
-        let dir = NameNullTerminated::new(dir)?;
-        let mut dirp = MaybeUninit::uninit();
-        (unsafe { riot_sys::vfs_opendir(dirp.as_mut_ptr(), dir.as_cstr()?.as_ptr() as _) })
-            .negative_to_error()?;
-        let dirp = unsafe { dirp.assume_init() };
-        Ok(Dir(dirp, core::marker::PhantomPinned))
+///
+/// ## Invariants
+///
+/// While this is active, the inner [DirSlot] always contains Some (and, in particular, per its
+/// invariants, its content is initialized).
+pub struct Dir<'a>(Pin<&'a mut DirSlot>);
+
+impl<'d> Dir<'d> {
+    /// Open a directory
+    ///
+    /// Unlike files (which are plain numeric file handles in RIOT), an open directory is a data
+    /// structure, and depending on the underlying file system may be a linked list. Therefore, we
+    /// can not return the open directory (and move it in the course of that), but need its place
+    /// to be pre-pinned. A simple `pin!(&mut Default::default())` will to to get a suitable
+    /// `slot`.
+    pub fn open<'name>(
+        name: &'name str,
+        mut slot: Pin<&'d mut DirSlot>,
+    ) -> Result<Self, NumericError> {
+        slot.as_mut().close();
+        let name = NameNullTerminated::new(name)?;
+        let dir = { unsafe { Pin::into_inner_unchecked(slot.as_mut().project().0) } }
+            .insert(MaybeUninit::uninit());
+        match (unsafe { riot_sys::vfs_opendir(dir.as_mut_ptr(), name.as_cstr()?.as_ptr() as _) })
+            .negative_to_error()
+        {
+            Ok(_) => (),
+            Err(e) => {
+                slot.0 = None;
+                return Err(e);
+            }
+        };
+        Ok(Dir(slot))
     }
 }
 
-impl Drop for Dir {
+impl Drop for Dir<'_> {
     fn drop(&mut self) {
-        unsafe { riot_sys::vfs_closedir(&mut self.0) };
+        // This is not required for soundness, but helps keep the number of open directories low on
+        // file systems where that matters.
+        self.0.as_mut().close();
     }
 }
 
-impl Iterator for Dir {
-    type Item = Dirent;
+impl<'d> Iterator for Dir<'d> {
+    type Item = Dirent<'d>;
 
-    fn next(&mut self) -> Option<Dirent> {
+    fn next(&mut self) -> Option<Dirent<'d>> {
         let mut ent = MaybeUninit::uninit();
-        let ret = (unsafe { riot_sys::vfs_readdir(&mut self.0, ent.as_mut_ptr()) })
+        let Some(mut dir) = self.0.as_mut().project().0.as_pin_mut() else {
+            unreachable!("Dir always has Some in it slot");
+        };
+        let ret = (unsafe { riot_sys::vfs_readdir(dir.as_mut_ptr(), ent.as_mut_ptr()) })
             .negative_to_error()
             .ok()?;
         if ret > 0 {
-            Some(Dirent(unsafe { ent.assume_init() }))
+            Some(Dirent(unsafe { ent.assume_init() }, PhantomData))
         } else {
             None
         }
@@ -183,9 +253,9 @@ impl Iterator for Dir {
 /// Directory entry inside a file
 ///
 /// The entry primarily indicates the file's name.
-pub struct Dirent(riot_sys::vfs_dirent_t);
+pub struct Dirent<'d>(riot_sys::vfs_dirent_t, PhantomData<&'d ()>);
 
-impl Dirent {
+impl Dirent<'_> {
     /// Name of the file
     ///
     /// This will panic if the file name is not encoded in UTF-8.

src/ztimer/mod.rs

@@ -63,8 +63,8 @@ impl<const HZ: u32> ValueInThread<Clock<HZ>> {
     ///
     /// Wraps [ztimer_sleep](https://doc.riot-os.org/group__sys__ztimer.html#gade98636e198f2d571c8acd861d29d360)
     #[doc(alias = "ztimer_sleep")]
-    pub fn sleep_ticks(&self, duration: u32) {
-        unsafe { riot_sys::ztimer_sleep(self.0, duration) };
+    pub fn sleep(&self, duration: Ticks<HZ>) {
+        unsafe { riot_sys::ztimer_sleep(self.0, duration.0) };
     }
 
     /// Keep the current thread in a busy loop until the duration of ticks in the timer's tim scale
@@ -79,11 +79,12 @@ impl<const HZ: u32> ValueInThread<Clock<HZ>> {
     /// is doable in an ISR), but it's so discouraged that the Rust wrapper takes the position that
     /// it's best done using a [ValueInThread].
     #[doc(alias = "ztimer_spin")]
-    pub fn spin_ticks(&self, duration: u32) {
-        unsafe { riot_sys::ztimer_spin(crate::inline_cast_mut(self.0), duration) };
+    pub fn spin(&self, duration: Ticks<HZ>) {
+        unsafe { riot_sys::ztimer_spin(crate::inline_cast_mut(self.0), duration.0) };
     }
 
-    /// Pause the current thread for the given duration.
+    /// Pause the current thread for the given duration, possibly exceeding values expressible in
+    /// [Ticks<HZ>].
     ///
     /// The duration is converted into ticks (rounding up), and overflows are caught by sleeping
     /// multiple times.
@@ -92,14 +93,16 @@ impl<const HZ: u32> ValueInThread<Clock<HZ>> {
     /// seconds on the microseconds timer would not overflow the timer's interface's u32, but the
     /// same multiple-sleeps trick may need to be employed by the implementation, *and* would keep
     /// the system from entering deeper sleep modes).
-    pub fn sleep(&self, duration: core::time::Duration) {
+    pub fn sleep_extended(&self, duration: core::time::Duration) {
         // Convert to ticks, rounding up as per Duration documentation
         let mut ticks = (duration * HZ - core::time::Duration::new(0, 1)).as_secs() + 1;
         while ticks > u32::MAX.into() {
-            self.sleep_ticks(u32::MAX);
+            self.sleep(Ticks(u32::MAX));
             ticks -= u64::from(u32::MAX);
         }
-        self.sleep_ticks(ticks.try_into().expect("Was just checked manually above"));
+        self.sleep(Ticks(
+            ticks.try_into().expect("Was just checked manually above"),
+        ));
     }
 
     /// Set the given callback to be executed in an interrupt some ticks in the future.
@@ -461,7 +464,7 @@ impl<const F: u32> embedded_hal::delay::DelayNs for ValueInThread<Clock<F>> {
             // ticks have some uncertainty on their own anyway.
 
             let ticks = (ns as u64) * (F as u64) / (NANOS_PER_SEC as u64);
-            self.sleep_ticks(ticks as u32);
+            self.sleep(Ticks(ticks as u32));
         }
     }
 }

tests/ztimer/src/lib.rs

@@ -12,7 +12,7 @@ fn main() {
 
     println!("Waiting 500 ticks on the msec timer before doing anything else");
     let duration = msec.time(|| {
-        msec.sleep_ticks(500);
+        msec.sleep(Ticks(500));
     });
     let duration =
         duration.expect("That should not have taken so long that the milliseconds overflowed");