CI: Reject warnings; fix warnings

Merges: #106

.github/workflows/test.yml

@@ -1,6 +1,11 @@
 # Ideally this should be replaced with a call out to Murdock; until that is
 # practical, building representative examples.
 
+# Ad RUSTFLAGS=-Dwarnings: While this is generally discouraged (for it means
+# that new Rust versions break CI), in this case we don't get new Rust versions
+# all the time but only on riotdocker updates, and that's a good time to
+# reflect on whether the new warnings make sense or not.
+
 name: test
 
 on:
@@ -58,7 +63,7 @@ jobs:
 
     - name: Build the example
       run: |
-        make all BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }}
+        RUSTFLAGS=-Dwarnings make all BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }}
 
   enumerate-wrappers-tests:
     runs-on: ubuntu-latest
@@ -128,7 +133,7 @@ jobs:
         cd ${{ matrix.testdir }}
         if BOARDS=${{ matrix.board }} make info-boards-supported | grep -q .
         then
-          BOARD=${{ matrix.board }} make all
+          BOARD=${{ matrix.board }} RUSTFLAGS=-Dwarnings make all
 
           if [ "native" = "${{ matrix.board }}" ] && make test/available BOARD=native
           then

src/helpers.rs

@@ -61,49 +61,55 @@ impl PointerToCStr for *const i8 {
     }
 }
 
-/// Trait that eases conversions from a char slice (no matter the signedness, they are used
-/// inconsistently in RIOT) to a CStr. The result is often used with `?.to_str().ok()?`.
-pub(crate) trait SliceToCStr {
-    /// Cast self around until it is suitable input to [`core::ffi::CStr::from_bytes_until_nul()`],
-    /// and run that function.
-    ///
-    /// Note that while "the slice until any null byte" could be safely used in Rust (as a slice or
-    /// even a str), its presence in C practically always indicates an error, also because that
-    /// data wouldn't be usable by other C code using its string conventions.
-    ///
-    /// It is using a local error type because while the semantics of `from_bytes_until_nul` are
-    /// the right ones considering how this is used on C fields that are treated with `strlen()`
-    /// etc., that function is not stable yet and emulated.
-    fn to_cstr(&self) -> Result<&core::ffi::CStr, FromBytesUntilNulError>;
-}
+// Gated because it is only used there -- expand as needed.
+#[cfg(riot_module_vfs)]
+mod slice_to_cstr {
+    /// Trait that eases conversions from a char slice (no matter the signedness, they are used
+    /// inconsistently in RIOT) to a CStr. The result is often used with `?.to_str().ok()?`.
+    pub(crate) trait SliceToCStr {
+        /// Cast self around until it is suitable input to [`core::ffi::CStr::from_bytes_until_nul()`],
+        /// and run that function.
+        ///
+        /// Note that while "the slice until any null byte" could be safely used in Rust (as a slice or
+        /// even a str), its presence in C practically always indicates an error, also because that
+        /// data wouldn't be usable by other C code using its string conventions.
+        ///
+        /// It is using a local error type because while the semantics of `from_bytes_until_nul` are
+        /// the right ones considering how this is used on C fields that are treated with `strlen()`
+        /// etc., that function is not stable yet and emulated.
+        fn to_cstr(&self) -> Result<&core::ffi::CStr, FromBytesUntilNulError>;
+    }
 
-// Unlike in the from_ptr case, this is consistently taking u8, so only the i8 case gets casting.
+    // Unlike in the from_ptr case, this is consistently taking u8, so only the i8 case gets casting.
 
-impl SliceToCStr for [u8] {
-    fn to_cstr(&self) -> Result<&core::ffi::CStr, FromBytesUntilNulError> {
-        // Emulate from_bytes_until_null
-        let index = self
-            .iter()
-            .position(|&c| c == 0)
-            .ok_or(FromBytesUntilNulError {})?;
+    impl SliceToCStr for [u8] {
+        fn to_cstr(&self) -> Result<&core::ffi::CStr, FromBytesUntilNulError> {
+            // Emulate from_bytes_until_null
+            let index = self
+                .iter()
+                .position(|&c| c == 0)
+                .ok_or(FromBytesUntilNulError {})?;
 
-        core::ffi::CStr::from_bytes_with_nul(&self[..index + 1])
-            // Actually the error is unreachable
-            .map_err(|_| FromBytesUntilNulError {})
+            core::ffi::CStr::from_bytes_with_nul(&self[..index + 1])
+                // Actually the error is unreachable
+                .map_err(|_| FromBytesUntilNulError {})
+        }
     }
-}
 
-impl SliceToCStr for [i8] {
-    fn to_cstr(&self) -> Result<&core::ffi::CStr, FromBytesUntilNulError> {
-        let s: &[u8] = unsafe { core::mem::transmute(self) };
-        s.to_cstr()
+    impl SliceToCStr for [i8] {
+        fn to_cstr(&self) -> Result<&core::ffi::CStr, FromBytesUntilNulError> {
+            let s: &[u8] = unsafe { core::mem::transmute(self) };
+            s.to_cstr()
+        }
     }
-}
 
-/// Error from [SliceToCStr::to_cstr].
-///
-/// This will become [core::ffi:FromBytesUntilNulError] once that's stable, and may be changed
-/// without a breaking release even though it's technically a breaking change. (At this point, that
-/// type will be `pub use`d here and deprecated).
-#[derive(Debug)]
-pub struct FromBytesUntilNulError {}
+    /// Error from [SliceToCStr::to_cstr].
+    ///
+    /// This will become [core::ffi:FromBytesUntilNulError] once that's stable, and may be changed
+    /// without a breaking release even though it's technically a breaking change. (At this point, that
+    /// type will be `pub use`d here and deprecated).
+    #[derive(Debug)]
+    pub struct FromBytesUntilNulError {}
+}
+#[cfg(riot_module_vfs)]
+pub use slice_to_cstr::*;

src/impl_critical_section.rs

@@ -3,7 +3,7 @@
 
 use critical_section::RawRestoreState;
 
-struct CriticalSection(usize);
+struct CriticalSection;
 critical_section::set_impl!(CriticalSection);
 
 unsafe impl critical_section::Impl for CriticalSection {

src/lib.rs

@@ -184,4 +184,10 @@ pub mod led;
 #[cfg(riot_module_auto_init)]
 pub mod auto_init;
 
+// Gated like socket_embedded_nal_async_udp as it is only used there -- expand as needed.
+#[cfg(all(
+    riot_module_sock_udp,
+    riot_module_sock_aux_local,
+    feature = "with_embedded_nal_async"
+))]
 mod async_helpers;

src/socket.rs

@@ -64,6 +64,8 @@ impl Into<riot_sys::sock_udp_ep_t> for UdpEp {
     }
 }
 
+// Gated to its users to avoid dead code warnings
+#[cfg(any(feature = "with_embedded_nal", feature = "with_embedded_nal_async"))]
 macro_rules! implementation_no_std_net {
     ($nsn_crate:ident) => {
         use super::*;

src/ztimer/mod.rs

@@ -169,7 +169,7 @@ impl<const HZ: u32> Clock<HZ> {
         ticks: Ticks<HZ>,
         in_thread: M,
     ) -> R {
-        use core::{cell::UnsafeCell, mem::ManuallyDrop};
+        use core::cell::UnsafeCell;
 
         // This is zero-initialized, which is the more efficient mode for ztimer_t.
         let mut timer = riot_sys::ztimer_t::default();

tests/led/src/lib.rs

@@ -19,9 +19,9 @@ fn main() {
     ];
     loop {
         for i in 0..=255 {
-            for j in 0..8 {
+            for (j, led) in leds.iter_mut().enumerate() {
                 if (i ^ (i - 1)) & (1 << j) != 0 {
-                    leds[j].toggle().unwrap();
+                    led.toggle().unwrap();
                 }
             }
         }

tests/random/src/lib.rs

@@ -7,7 +7,7 @@ use riot_wrappers::riot_main;
 riot_main!(main);
 
 fn check_csrng(mut rng: impl rand_core::CryptoRng + rand_core::RngCore) {
-    use rngcheck::{helpers::*, nist::*};
+    use rngcheck::nist::*;
 
     // This is also in https://github.com/ryankurte/rngcheck/pull/3
     struct BitIter<'a, R: rand_core::RngCore> {

tests/ztimer-async/src/lib.rs

@@ -11,7 +11,9 @@ fn main() -> ! {
         static_cell::StaticCell::new();
     let executor: &'static mut _ = EXECUTOR.init(embassy_executor_riot::Executor::new());
     executor.run(|spawner| {
-        spawner.spawn(amain(spawner));
+        spawner
+            .spawn(amain(spawner))
+            .expect("Task did not get spawned before");
     })
 }
 
@@ -38,8 +40,12 @@ async fn amain(spawner: embassy_executor::Spawner) {
     drop(locked);
     println!("And now for something more complex...");
 
-    spawner.spawn(ten_tenths());
-    spawner.spawn(five_fifths());
+    spawner
+        .spawn(ten_tenths())
+        .expect("Task did not get spawned before");
+    spawner
+        .spawn(five_fifths())
+        .expect("Task did not get spawned before");
 }
 
 #[embassy_executor::task]