From 547ad2cf1174389dfa7704ef82c58b322a210e95 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:23:57 +0200 Subject: [PATCH 01/23] tokenparts: Rename attribute that is never read --- src/thread/tokenparts.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/thread/tokenparts.rs b/src/thread/tokenparts.rs index 6dabf227..6aa14652 100644 --- a/src/thread/tokenparts.rs +++ b/src/thread/tokenparts.rs @@ -228,7 +228,7 @@ impl InThread { pub fn promote(self, value: T) -> ValueInThread { ValueInThread { value, - in_thread: self, + _in_thread: self, } } } @@ -258,7 +258,7 @@ impl InIsr { #[cfg_attr(feature = "nightly_docs", fundamental)] pub struct ValueInThread { value: T, - in_thread: InThread, + _in_thread: InThread, } impl ValueInThread { From 78dc3346046a8433ae016756a63120e1eecb14bc Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:27:10 +0200 Subject: [PATCH 02/23] Fix unused variable warning in cfg dependent section --- src/thread/riot_c.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/thread/riot_c.rs b/src/thread/riot_c.rs index 750936b0..b2abdf90 100644 --- a/src/thread/riot_c.rs +++ b/src/thread/riot_c.rs @@ -172,16 +172,19 @@ impl KernelPID { /// This is not backed by C functions (as most of the rest of this crate is), but rather a /// practical way to access struct members that may or may not be present in a build. pub fn stack_stats(&self) -> Result { - let thread = self.thread()?; #[cfg(riot_develhelp)] - return Ok(StackStats { - // This cast is relevant because different platforms (eg. native and arm) disagree on - // whether that's an i8 or u8 pointer. Could have made it c_char, but a) don't want to - // alter the signatures and b) it's easier to use on the Rust side with a clear type. - start: unsafe { (*thread).stack_start as _ }, - size: unsafe { (*thread).stack_size as _ }, - free: unsafe { riot_sys::thread_measure_stack_free((*thread).stack_start) } as usize, - }); + { + let thread = self.thread()?; + return Ok(StackStats { + // This cast is relevant because different platforms (eg. native and arm) disagree on + // whether that's an i8 or u8 pointer. Could have made it c_char, but a) don't want to + // alter the signatures and b) it's easier to use on the Rust side with a clear type. + start: unsafe { (*thread).stack_start as _ }, + size: unsafe { (*thread).stack_size as _ }, + free: unsafe { riot_sys::thread_measure_stack_free((*thread).stack_start) } + as usize, + }); + } #[cfg(not(riot_develhelp))] return Err(StackStatsError::InformationUnavailable); } From f3a34e8669e7957c1de00ce48135951e1f93079a Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:48:45 +0200 Subject: [PATCH 03/23] tests: Fix unused import warning --- tests/mutex/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mutex/src/lib.rs b/tests/mutex/src/lib.rs index 0bda93f4..926b1374 100644 --- a/tests/mutex/src/lib.rs +++ b/tests/mutex/src/lib.rs @@ -3,7 +3,7 @@ use riot_wrappers::mutex::Mutex; use riot_wrappers::println; use riot_wrappers::riot_main; -use riot_wrappers::thread::{InThread, ValueInThread}; +use riot_wrappers::thread::InThread; riot_main!(main); From da5b6d6ef2fba9e4b2b01517d19e88b103774013 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:03:18 +0200 Subject: [PATCH 04/23] gnrc: Fix unused use warning --- src/gnrc/netreg.rs | 3 +-- src/gnrc/pktbuf.rs | 3 ++- src/never.rs | 1 + src/saul.rs | 4 ---- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/gnrc/netreg.rs b/src/gnrc/netreg.rs index f294afd8..a4d3a42f 100644 --- a/src/gnrc/netreg.rs +++ b/src/gnrc/netreg.rs @@ -1,7 +1,6 @@ +#[cfg(feature = "with_msg_v2")] use core::mem::MaybeUninit; -use riot_sys::{gnrc_netreg_entry_t, gnrc_netreg_register, gnrc_netreg_unregister, gnrc_nettype_t}; - use crate::error::NegativeErrorExt; // Transmuting the pointer into a Pktsnip does the right thing by treating it as a smart diff --git a/src/gnrc/pktbuf.rs b/src/gnrc/pktbuf.rs index 153001bd..e219ae11 100644 --- a/src/gnrc/pktbuf.rs +++ b/src/gnrc/pktbuf.rs @@ -11,7 +11,6 @@ use riot_sys::{ gnrc_pktbuf_realloc_data, gnrc_pktbuf_release_error, gnrc_pktsnip_t, - gnrc_udp_hdr_build, GNRC_NETERR_SUCCESS, }; @@ -138,6 +137,8 @@ impl Pktsnip { src: core::num::NonZeroU16, dst: core::num::NonZeroU16, ) -> Result, NotEnoughSpace> { + use riot_sys::gnrc_udp_hdr_build; + let snip = unsafe { gnrc_udp_hdr_build(self.ptr, src.into(), dst.into()) }; if snip == 0 as *mut _ { // All other errors are caught by the signature diff --git a/src/never.rs b/src/never.rs index 3c12f841..499a750b 100644 --- a/src/never.rs +++ b/src/never.rs @@ -3,6 +3,7 @@ /// /// From , adjusted for /// usability with pub interfaces by using a pub trait in a private module (sealing). +#[cfg(not(feature = "actual_never_type"))] use crate::helpers::*; #[cfg(not(feature = "actual_never_type"))] diff --git a/src/saul.rs b/src/saul.rs index 115fa924..841c87d4 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -17,12 +17,8 @@ //! //! [SAUL]: https://doc.riot-os.org/group__drivers__saul.html -use riot_sys as raw; -use riot_sys::libc; - use crate::error; use crate::helpers::PointerToCStr; -use crate::Never; use error::NegativeErrorExt; pub mod registration; From 93a4c989346f6f52b936d43e4acb8e45e501152b Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:19:38 +0200 Subject: [PATCH 05/23] gcoap: Fix unused mut --- src/gcoap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gcoap.rs b/src/gcoap.rs index 3450bee1..296a671a 100644 --- a/src/gcoap.rs +++ b/src/gcoap.rs @@ -216,7 +216,7 @@ unsafe extern "C" fn link_encoder( let h: &H = unsafe { &*((*resource).context as *const _) }; let buf = buf as *mut u8; // cast away signedness of char - let mut buf = if buf.is_null() { + let buf = if buf.is_null() { None } else { Some(core::slice::from_raw_parts_mut(buf, buf_len as _)) From 80f44432c2acce8c2df0d22fbf43265ad9915b80 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:34:32 +0200 Subject: [PATCH 06/23] embedded-nal: Rename unused arguments to _ versions --- src/socket_embedded_nal_tcp.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/socket_embedded_nal_tcp.rs b/src/socket_embedded_nal_tcp.rs index 33f8c81c..7668e5a1 100644 --- a/src/socket_embedded_nal_tcp.rs +++ b/src/socket_embedded_nal_tcp.rs @@ -107,8 +107,8 @@ impl<'a, const QUEUELEN: usize> TcpClientStack for Pin<&'a mut ListenStack Result<(), nb::Error> { panic!("A ListenStack can not connect out.") } @@ -116,7 +116,7 @@ impl<'a, const QUEUELEN: usize> TcpClientStack for Pin<&'a mut ListenStack true, + SocketImpl::Connection(_n) => true, _ => false, }) } @@ -206,13 +206,15 @@ impl<'a, const QUEUELEN: usize> TcpFullStack for Pin<&'a mut ListenStack Result<(), Self::Error> { + fn listen(&mut self, _sock: &mut Self::TcpSocket) -> Result<(), Self::Error> { // Done already in bind Ok(()) } fn accept( &mut self, - sock: &mut Self::TcpSocket, + // This is and can actually be ignored, because our stack object only serves a single + // listening socket. + _sock: &mut Self::TcpSocket, ) -> Result<(Self::TcpSocket, SocketAddr), nb::Error> { let mut sockptr = core::ptr::null_mut(); unsafe { From b65a0d64e76eba871bff6e6292881a3745683e9c Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:53:41 +0200 Subject: [PATCH 07/23] gcoap: Remove unused internal function --- src/gcoap.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/gcoap.rs b/src/gcoap.rs index 296a671a..d39fccab 100644 --- a/src/gcoap.rs +++ b/src/gcoap.rs @@ -344,7 +344,6 @@ pub trait WithLinkEncoder { } use riot_sys::{ - coap_get_total_hdr_len, coap_opt_add_opaque, coap_opt_add_uint, coap_opt_get_next, @@ -376,11 +375,6 @@ impl PacketBuffer { }) as u8 // odd return type in C } - /// Wrapper for coap_get_total_hdr_len - fn get_total_hdr_len(&self) -> usize { - (unsafe { coap_get_total_hdr_len(crate::inline_cast(self.pkt)) }) as usize - } - /// Wrapper for gcoap_resp_init /// /// As it is used and wrapped here, this makes GCOAP_RESP_OPTIONS_BUF bytes unusable, but From 280bf427d8039be402a103a614360aa2a0233e6e Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:16:29 +0200 Subject: [PATCH 08/23] tests/led: Simplify to fix warnings --- tests/led/src/lib.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/led/src/lib.rs b/tests/led/src/lib.rs index 73571e24..f0ca139c 100644 --- a/tests/led/src/lib.rs +++ b/tests/led/src/lib.rs @@ -1,28 +1,27 @@ #![no_std] -use riot_wrappers::led; +use riot_wrappers::led::LED; use riot_wrappers::riot_main; riot_main!(main); fn main() { - let mut led0 = riot_wrappers::led::LED::<0>::new(); - let mut led1 = riot_wrappers::led::LED::<1>::new(); - let mut led2 = riot_wrappers::led::LED::<2>::new(); - let mut led3 = riot_wrappers::led::LED::<3>::new(); - let mut led4 = riot_wrappers::led::LED::<4>::new(); - let mut led5 = riot_wrappers::led::LED::<5>::new(); - let mut led6 = riot_wrappers::led::LED::<6>::new(); - let mut led7 = riot_wrappers::led::LED::<7>::new(); + let mut led0 = LED::<0>::new(); + let mut led1 = LED::<1>::new(); + let mut led2 = LED::<2>::new(); + let mut led3 = LED::<3>::new(); + let mut led4 = LED::<4>::new(); + let mut led5 = LED::<5>::new(); + let mut led6 = LED::<6>::new(); + let mut led7 = LED::<7>::new(); let mut leds: [&mut dyn switch_hal::ToggleableOutputSwitch; 8] = [ &mut led0, &mut led1, &mut led2, &mut led3, &mut led4, &mut led5, &mut led6, &mut led7, ]; - use switch_hal::ToggleableOutputSwitch; loop { for i in 0..=255 { for j in 0..8 { if (i ^ (i - 1)) & (1 << j) != 0 { - leds[j].toggle(); + leds[j].toggle().unwrap(); } } } From a72a4ab39edf3530170df13f499d50b097455afe Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 19:30:56 +0200 Subject: [PATCH 09/23] Fix unused imports --- src/gnrc/netreg.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gnrc/netreg.rs b/src/gnrc/netreg.rs index a4d3a42f..573bf1b2 100644 --- a/src/gnrc/netreg.rs +++ b/src/gnrc/netreg.rs @@ -1,6 +1,7 @@ #[cfg(feature = "with_msg_v2")] use core::mem::MaybeUninit; +#[cfg(feature = "with_msg_v2")] use crate::error::NegativeErrorExt; // Transmuting the pointer into a Pktsnip does the right thing by treating it as a smart From 28758b1ae220c17d7573821dfb4c1d9a51c8759b Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:44:29 +0200 Subject: [PATCH 10/23] Silence deprecation warning from pub-use --- src/thread.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/thread.rs b/src/thread.rs index 075e1d20..a82a28b3 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -23,6 +23,7 @@ pub use riot_c::*; mod tokenparts; #[cfg(doc)] pub use tokenparts::TokenParts; +#[allow(deprecated)] pub use tokenparts::{EndToken, InIsr, InThread, StartToken, TerminationToken, ValueInThread}; mod stack_stats; From 09520f0d85ba2bd1bd6ef10dcc0a15c8afe2298a Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:16:57 +0200 Subject: [PATCH 11/23] Fix deprecated direct access --- src/bluetil.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bluetil.rs b/src/bluetil.rs index 6ac771f0..36d4da15 100644 --- a/src/bluetil.rs +++ b/src/bluetil.rs @@ -31,7 +31,7 @@ pub enum Error { impl From for Error { fn from(e: crate::error::NumericError) -> Error { - match e.number as _ { + match e.number() as _ { riot_sys::BLUETIL_AD_NOTFOUND => Error::NotFound, riot_sys::BLUETIL_AD_NOMEM => Error::NoMem, _ => panic!("Invalid bluetil error"), From 1bfe708abf0691ac9c29055211ac58e845b957ba Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:55:05 +0200 Subject: [PATCH 12/23] saul: Enum variants that were legacy shifted towards constants need naming exceptions --- src/saul.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/saul.rs b/src/saul.rs index 841c87d4..9dbcafd6 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -400,8 +400,10 @@ impl Unit { // we can just switch over before they go. #[deprecated(note = "Use the GForce variant instead")] pub const G: Self = Unit::GForce; + #[allow(non_upper_case_globals)] #[deprecated(note = "Use the Gram variant instead")] pub const Gr: Self = Unit::Gram; + #[allow(non_upper_case_globals)] #[deprecated(note = "Use the Gauss variant instead")] pub const Gs: Self = Unit::Gauss; From d99c90b5736b35dd1fa0f2e8dd8235c483a974e0 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:22:54 +0200 Subject: [PATCH 13/23] treewide/doc: Various fixes --- src/auto_init.rs | 4 ++-- src/gcoap.rs | 2 +- src/interrupt.rs | 2 +- src/main_module.rs | 13 ++++++------- src/saul/registration.rs | 7 ++++--- src/shell.rs | 8 ++++---- src/socket_embedded_nal_tcp.rs | 5 +++-- src/thread.rs | 10 +++++----- src/thread/stack_stats.rs | 2 +- src/thread/tokenparts.rs | 5 +++-- src/vfs.rs | 2 +- 11 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/auto_init.rs b/src/auto_init.rs index 31213848..40070013 100644 --- a/src/auto_init.rs +++ b/src/auto_init.rs @@ -1,6 +1,6 @@ //! Tools for declaring a function that is run during initialization //! -//! The [auto_init!] macro is this module's main product. +//! The [`auto_init!`](super::auto_init!) macro is this module's main product. /// Wrapper around [riot_sys::auto_init_module_t] /// @@ -13,7 +13,7 @@ impl AutoInitModule { /// Initializer for module auto-initialization /// /// Do not call this directly: Its result must be placed in a static in a special section in - /// memory, which is handled by the [`auto_init!`] macro. + /// memory, which is handled by the [`auto_init!`](super::auto_init!) macro. pub const fn new( init_function: extern "C" fn(), priority: u16, diff --git a/src/gcoap.rs b/src/gcoap.rs index d39fccab..66188374 100644 --- a/src/gcoap.rs +++ b/src/gcoap.rs @@ -259,7 +259,7 @@ impl<'a, H> SingleHandlerListener<'a, H> where H: 'a + Handler + WithLinkEncoder, { - /// Like [`new()`], but utilizing that the handler is also [WithLinkEncoder] and can thus influence + /// Like [`Self::new()`], but utilizing that the handler is also [WithLinkEncoder] and can thus influence /// what is reported when the default .well-known/core handler is queried. pub fn new_with_link_encoder( path: &'a core::ffi::CStr, diff --git a/src/interrupt.rs b/src/interrupt.rs index 642f684a..a458a232 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -5,7 +5,7 @@ //! * Utility functions can disable interrupts (creating critical sections), check whether //! interrupts are enabled or to determine whether code is executed in a thread or an ISR. //! -//! * Some functions (eg. [`ZTimer::set_ticks_during`](crate::ztimer::ZTimer::set_ticks_during)) +//! * Some functions (eg. [`ZTimer::set_ticks_during`](crate::ztimer::Clock::set_during)) //! take callbacks that will be called in an interrupt context. //! //! These are typechecked to be Send, as they are moved from the thread to the interrupt context. diff --git a/src/main_module.rs b/src/main_module.rs index 5e8c8e99..c0943517 100644 --- a/src/main_module.rs +++ b/src/main_module.rs @@ -1,6 +1,6 @@ //! Tools for providing a RIOT main function //! -//! The main contribution of this module is the [riot_main] macro. +//! The main contribution of this module is the [`riot_main!`](super::riot_main!) macro. //! //! The alternative to using that (other than doing it manually) is to have C code along with the //! Rust application that occupies the main function. @@ -10,6 +10,7 @@ //! C code. use crate::stdio::println; +use crate::thread::{EndToken, StartToken}; // General alternative to this module: Build the extern "C" main all the time and request that the // application implement a named function. I never got the main function to be carried to the @@ -55,9 +56,9 @@ impl T, T: Termination> UsableAsMain<[u8; 1]> for F { } } -impl crate::never::Never> Sealed<[u8; 2]> for F {} +impl crate::never::Never> Sealed<[u8; 2]> for F {} -impl crate::never::Never> UsableAsMain<[u8; 2]> for F { +impl crate::never::Never> UsableAsMain<[u8; 2]> for F { unsafe fn call_main(&self) -> i32 { // unsafe: By construction of the C main function this only happens at startup time // with a thread that hasn't done anything relevant before. @@ -67,11 +68,9 @@ impl crate::never::Never> UsableAsMain<[u8; } } -impl ((), crate::thread::EndToken)> Sealed<[u8; 3]> for F {} +impl ((), EndToken)> Sealed<[u8; 3]> for F {} -impl ((), crate::thread::EndToken)> UsableAsMain<[u8; 3]> - for F -{ +impl ((), EndToken)> UsableAsMain<[u8; 3]> for F { unsafe fn call_main(&self) -> i32 { // unsafe: By construction of the C main function this only happens at startup time // with a thread that hasn't done anything relevant before. diff --git a/src/saul/registration.rs b/src/saul/registration.rs index 29739768..74c264cd 100644 --- a/src/saul/registration.rs +++ b/src/saul/registration.rs @@ -33,8 +33,9 @@ pub trait Drivable: Sized { /// Set to true if `read` is implemented. /// /// Doing this on the type level (rather than having read and write return a more - /// differentiated error) allows the driver to point to the shared [riot_sys::saul_notsup] - /// handler rather than to monomorphize a custom erring handler for each device. + /// differentiated error) allows the driver to point to the shared [riot_sys::saul_read_notsup] + /// / [riot_sys::saul_write_notsup] handler rather than to monomorphize a custom erring handler + /// for each device. const HAS_READ: bool = false; /// Set to true if `write` is implemented. const HAS_WRITE: bool = false; @@ -50,7 +51,7 @@ pub trait Drivable: Sized { /// Set the state of an actuator, or reconfigure a sensor /// /// A &self is passed in on write because there could be concurrent access from multiple SAUL - /// users. One option of handling this is to implement Drivable for Mutex. + /// users. One option of handling this is to implement Drivable for `Mutex`. /// /// Note that due to the way SAUL is structured, the drivable can not know the number of /// entries which the user intended to set. The Drivable trait always builds the Rust Phydat diff --git a/src/shell.rs b/src/shell.rs index f5d86203..b1a768d7 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -2,8 +2,8 @@ //! //! This module can be used in two ways: //! -//! * Declare static commands using [static_command]; these only take a `fn` (not a closure) -//! because shell commands don't have an arg pointer. +//! * Declare static commands using [`static_command!`](crate::static_command!); these only take a +//! `fn` (not a closure) because shell commands don't have an arg pointer. //! //! This works even in RIOT modules that are included in a C application that starts a shell, and //! show up in shells created through Rust without explicit inclusion. @@ -20,8 +20,8 @@ //! argument. This does allow the Rust wrappers to "just so" use a closure as a command handler, //! but also needs a lot of code. //! -//! That complexity is not pulled in when only using [static_command] and running on an otherwise -//! empty command list. +//! That complexity is not pulled in when only using [`static_command!`](crate::static_command!) +//! and running on an otherwise empty command list. use crate::{mutex, stdio}; use core::ffi::CStr; diff --git a/src/socket_embedded_nal_tcp.rs b/src/socket_embedded_nal_tcp.rs index 7668e5a1..3959c94a 100644 --- a/src/socket_embedded_nal_tcp.rs +++ b/src/socket_embedded_nal_tcp.rs @@ -1,8 +1,9 @@ //! An implementation of the [embedded_nal] (Network Abstradtion Layer) TCP traits based on RIOT //! sockets //! -//! This is vastly distinct from [socket_embedded_nal] as it requires vastly different workarounds -//! (and because it was implemented when embedded-nal had already switched over to &mut stack). +//! This is vastly distinct from [the UDP version](crate::socket_embedded_nal) as it requires +//! vastly different workarounds (and because it was implemented when embedded-nal had already +//! switched over to &mut stack). //! //! ## Warning //! diff --git a/src/thread.rs b/src/thread.rs index a82a28b3..aa9e964a 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -2,11 +2,11 @@ //! //! ## Tokens //! -//! Some thread creation mechanisms (currently only [riot_main_with_tokens] and not those in here) -//! are "with tokens". With these, the zero-sized type [StartToken] is used to pass along the -//! information that the execution is currently happening in a thread, and more importantly that -//! some operations doable only once per thread (eg. setting up a message queue) have not yet -//! happed. +//! Some thread creation mechanisms (currently only +//! [`riot_main_with_tokens!`](crate::riot_main_with_tokens!) and not those in here) are "with +//! tokens". With these, the zero-sized type [StartToken] is used to pass along the information +//! that the execution is currently happening in a thread, and more importantly that some +//! operations doable only once per thread (eg. setting up a message queue) have not yet happed. //! //! When threads are created that way, they need to return an [EndToken] which ensures that //! no operations that preclude the termination of a thread have happened. diff --git a/src/thread/stack_stats.rs b/src/thread/stack_stats.rs index 65466e82..10fa655a 100644 --- a/src/thread/stack_stats.rs +++ b/src/thread/stack_stats.rs @@ -1,4 +1,4 @@ -/// Gathered information about a thread, returned by [KernelPID::stack_stats()]. +/// Gathered information about a thread, returned by [super::KernelPID::stack_stats()]. /// /// All accessors are unconditional, because the StackStats can't be obtained without develhelp in /// the first place. diff --git a/src/thread/tokenparts.rs b/src/thread/tokenparts.rs index 6aa14652..df0b7115 100644 --- a/src/thread/tokenparts.rs +++ b/src/thread/tokenparts.rs @@ -44,8 +44,9 @@ pub struct TokenParts { /// Claim that the current thread has not done anything yet that is covered by this type /// - /// Do not call yourself; this needs to be public because [riot_main_with_tokens] is a macro - /// and thus technically called from the main crate. + /// Do not call yourself; this needs to be public because + /// [`riot_main_with_tokens!`](crate::riot_main_with_tokens!) is a macro and thus technically + /// called from the main crate. pub unsafe fn new() -> Self { TokenParts { _not_send: PhantomData, diff --git a/src/vfs.rs b/src/vfs.rs index 39f40fda..0450969f 100644 --- a/src/vfs.rs +++ b/src/vfs.rs @@ -43,7 +43,7 @@ impl Stat { /// Parameter for seeking in a file /// -/// It is analogous to [std::io::SeekFrom]. +/// It is analogous to [std::io::SeekFrom](https://doc.rust-lang.org/std/io/enum.SeekFrom.html). #[derive(Debug)] pub enum SeekFrom { /// Seek to the given position from the start of the file From 5bb8849830e2a5528501cae24fc2ea557fac2cc2 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:24:22 +0200 Subject: [PATCH 14/23] thread/doc: Add comment on ValueInThread not being Send any more --- src/thread/tokenparts.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/thread/tokenparts.rs b/src/thread/tokenparts.rs index df0b7115..ccd4afc8 100644 --- a/src/thread/tokenparts.rs +++ b/src/thread/tokenparts.rs @@ -254,6 +254,8 @@ impl InIsr { /// /// This does barely implement anything on its own, but the module implementing `T` might provide /// extra methods. +/// +/// This makes the wrapped value not `Send`. // Making the type fundamental results in ValueInThread<&Mutex> being shown at Mutex's page. #[derive(Copy, Clone)] #[cfg_attr(feature = "nightly_docs", fundamental)] From 15e0b0980c9c7436a30d58081a0976310a76f962 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:37:47 +0200 Subject: [PATCH 15/23] thread: Remove intermediate module layer from constants, fixing warnings --- src/msg.rs | 2 +- src/thread/riot_c.rs | 110 ++++++++++++++++++++----------------------- 2 files changed, 52 insertions(+), 60 deletions(-) diff --git a/src/msg.rs b/src/msg.rs index c14afdb8..e99044c1 100644 --- a/src/msg.rs +++ b/src/msg.rs @@ -28,7 +28,7 @@ pub enum MsgSender { impl MsgSender { fn from_pid(pid: kernel_pid_t) -> Self { - if pid == crate::thread::pid_converted::KERNEL_PID_ISR { + if pid == crate::thread::KERNEL_PID_ISR { MsgSender::ISR } else { KernelPID::new(pid) diff --git a/src/thread/riot_c.rs b/src/thread/riot_c.rs index b2abdf90..4a93e5cb 100644 --- a/src/thread/riot_c.rs +++ b/src/thread/riot_c.rs @@ -14,41 +14,33 @@ pub use creation::{scope, spawn, CountedThread, CountingThreadScope}; #[derive(Debug, PartialEq, Copy, Clone)] pub struct KernelPID(pub(crate) raw::kernel_pid_t); -pub(crate) mod pid_converted { - //! Converting the raw constants into consistently typed ones - use riot_sys as raw; - - pub const KERNEL_PID_UNDEF: raw::kernel_pid_t = raw::KERNEL_PID_UNDEF as _; - pub const KERNEL_PID_FIRST: raw::kernel_pid_t = raw::KERNEL_PID_FIRST as _; - pub const KERNEL_PID_LAST: raw::kernel_pid_t = raw::KERNEL_PID_LAST as _; - pub const KERNEL_PID_ISR: raw::kernel_pid_t = raw::KERNEL_PID_ISR as _; -} - -mod status_converted { - //! Converting the raw constants into consistently typed ones for use in match branches. If - //! that becomes a pattern, it might make sense to introduce a macro that forces a bunch of - //! symbols (with different capitalizations) into a given type and makes an enum with a - //! from_int method out of it. - - use riot_sys as raw; - - // This is special because it is not part of the enum but a cast -1 - // unsafe: Side effect free C macros - pub const STATUS_NOT_FOUND: i32 = unsafe { raw::macro_STATUS_NOT_FOUND() as _ }; - - pub const STATUS_STOPPED: i32 = raw::thread_status_t_STATUS_STOPPED as i32; - pub const STATUS_SLEEPING: i32 = raw::thread_status_t_STATUS_SLEEPING as i32; - pub const STATUS_MUTEX_BLOCKED: i32 = raw::thread_status_t_STATUS_MUTEX_BLOCKED as i32; - pub const STATUS_RECEIVE_BLOCKED: i32 = raw::thread_status_t_STATUS_RECEIVE_BLOCKED as i32; - pub const STATUS_SEND_BLOCKED: i32 = raw::thread_status_t_STATUS_SEND_BLOCKED as i32; - pub const STATUS_REPLY_BLOCKED: i32 = raw::thread_status_t_STATUS_REPLY_BLOCKED as i32; - pub const STATUS_FLAG_BLOCKED_ANY: i32 = raw::thread_status_t_STATUS_FLAG_BLOCKED_ANY as i32; - pub const STATUS_FLAG_BLOCKED_ALL: i32 = raw::thread_status_t_STATUS_FLAG_BLOCKED_ALL as i32; - pub const STATUS_MBOX_BLOCKED: i32 = raw::thread_status_t_STATUS_MBOX_BLOCKED as i32; - pub const STATUS_RUNNING: i32 = raw::thread_status_t_STATUS_RUNNING as i32; - pub const STATUS_PENDING: i32 = raw::thread_status_t_STATUS_PENDING as i32; -} - +// Converting the raw constants into consistently typed ones + +// pub(crate) const KERNEL_PID_UNDEF: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_UNDEF as _; +const KERNEL_PID_FIRST: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_FIRST as _; +const KERNEL_PID_LAST: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_LAST as _; +pub(crate) const KERNEL_PID_ISR: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_ISR as _; + +// Converting the raw constants into consistently typed ones for use in match branches. If +// that becomes a pattern, it might make sense to introduce a macro that forces a bunch of +// symbols (with different capitalizations) into a given type and makes an enum with a +// from_int method out of it. + +// This is special because it is not part of the enum but a cast -1 +// unsafe: Side effect free C macros +const STATUS_NOT_FOUND: i32 = unsafe { riot_sys::macro_STATUS_NOT_FOUND() as _ }; + +const STATUS_STOPPED: i32 = riot_sys::thread_status_t_STATUS_STOPPED as i32; +const STATUS_SLEEPING: i32 = riot_sys::thread_status_t_STATUS_SLEEPING as i32; +const STATUS_MUTEX_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_MUTEX_BLOCKED as i32; +const STATUS_RECEIVE_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_RECEIVE_BLOCKED as i32; +const STATUS_SEND_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_SEND_BLOCKED as i32; +const STATUS_REPLY_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_REPLY_BLOCKED as i32; +const STATUS_FLAG_BLOCKED_ANY: i32 = riot_sys::thread_status_t_STATUS_FLAG_BLOCKED_ANY as i32; +const STATUS_FLAG_BLOCKED_ALL: i32 = riot_sys::thread_status_t_STATUS_FLAG_BLOCKED_ALL as i32; +const STATUS_MBOX_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_MBOX_BLOCKED as i32; +const STATUS_RUNNING: i32 = riot_sys::thread_status_t_STATUS_RUNNING as i32; +const STATUS_PENDING: i32 = riot_sys::thread_status_t_STATUS_PENDING as i32; #[derive(Debug)] #[non_exhaustive] @@ -56,17 +48,17 @@ pub enum Status { // I would not rely on any properties of the assigned values, but it might make the conversion // points easier on the generated code if it can be reasoned down to a simple check of whether // it's in range. - Stopped = status_converted::STATUS_STOPPED as isize, - Sleeping = status_converted::STATUS_SLEEPING as isize, - MutexBlocked = status_converted::STATUS_MUTEX_BLOCKED as isize, - ReceiveBlocked = status_converted::STATUS_RECEIVE_BLOCKED as isize, - SendBlocked = status_converted::STATUS_SEND_BLOCKED as isize, - ReplyBlocked = status_converted::STATUS_REPLY_BLOCKED as isize, - FlagBlockedAny = status_converted::STATUS_FLAG_BLOCKED_ANY as isize, - FlagBlockedAll = status_converted::STATUS_FLAG_BLOCKED_ALL as isize, - MboxBlocked = status_converted::STATUS_MBOX_BLOCKED as isize, - Running = status_converted::STATUS_RUNNING as isize, - Pending = status_converted::STATUS_PENDING as isize, + Stopped = STATUS_STOPPED as isize, + Sleeping = STATUS_SLEEPING as isize, + MutexBlocked = STATUS_MUTEX_BLOCKED as isize, + ReceiveBlocked = STATUS_RECEIVE_BLOCKED as isize, + SendBlocked = STATUS_SEND_BLOCKED as isize, + ReplyBlocked = STATUS_REPLY_BLOCKED as isize, + FlagBlockedAny = STATUS_FLAG_BLOCKED_ANY as isize, + FlagBlockedAll = STATUS_FLAG_BLOCKED_ALL as isize, + MboxBlocked = STATUS_MBOX_BLOCKED as isize, + Running = STATUS_RUNNING as isize, + Pending = STATUS_PENDING as isize, /// A status value not known to riot-wrappers. Don't match for this explicitly: Other values /// may, at any minor riot-wrappers update, become actual process states again. @@ -78,17 +70,17 @@ pub enum Status { impl Status { fn from_int(status: i32) -> Self { match status { - status_converted::STATUS_STOPPED => Status::Stopped, - status_converted::STATUS_SLEEPING => Status::Sleeping, - status_converted::STATUS_MUTEX_BLOCKED => Status::MutexBlocked, - status_converted::STATUS_RECEIVE_BLOCKED => Status::ReceiveBlocked, - status_converted::STATUS_SEND_BLOCKED => Status::SendBlocked, - status_converted::STATUS_REPLY_BLOCKED => Status::ReplyBlocked, - status_converted::STATUS_FLAG_BLOCKED_ANY => Status::FlagBlockedAny, - status_converted::STATUS_FLAG_BLOCKED_ALL => Status::FlagBlockedAll, - status_converted::STATUS_MBOX_BLOCKED => Status::MboxBlocked, - status_converted::STATUS_RUNNING => Status::Running, - status_converted::STATUS_PENDING => Status::Pending, + STATUS_STOPPED => Status::Stopped, + STATUS_SLEEPING => Status::Sleeping, + STATUS_MUTEX_BLOCKED => Status::MutexBlocked, + STATUS_RECEIVE_BLOCKED => Status::ReceiveBlocked, + STATUS_SEND_BLOCKED => Status::SendBlocked, + STATUS_REPLY_BLOCKED => Status::ReplyBlocked, + STATUS_FLAG_BLOCKED_ANY => Status::FlagBlockedAny, + STATUS_FLAG_BLOCKED_ALL => Status::FlagBlockedAll, + STATUS_MBOX_BLOCKED => Status::MboxBlocked, + STATUS_RUNNING => Status::Running, + STATUS_PENDING => Status::Pending, _ => Status::Other, } } @@ -109,7 +101,7 @@ impl KernelPID { // and then this function *should* be reevaluated). As pid_is_valid is static inline, the // compiler should be able to see through the calls down to there that the bounds checked // for there are the very bounds used in the construction here. - (pid_converted::KERNEL_PID_FIRST..=pid_converted::KERNEL_PID_LAST) + (KERNEL_PID_FIRST..=KERNEL_PID_LAST) .map(|i| KernelPID::new(i).expect("Should be valid by construction")) } @@ -128,7 +120,7 @@ impl KernelPID { pub fn status(&self) -> Result { // unsafe: Side effect free, always-callable C function let status = unsafe { raw::thread_getstatus(self.0) } as _; - if status == status_converted::STATUS_NOT_FOUND { + if status == STATUS_NOT_FOUND { Err(NoSuchThread) } else { Ok(Status::from_int(status)) From d6de3cbe6e8cb735abfaa8b6c5ee79437fa30b70 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:05:00 +0200 Subject: [PATCH 16/23] gnrc_pktbuf: Improve debugging experience by showing the writability generic in debug output --- src/gnrc/pktbuf.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/gnrc/pktbuf.rs b/src/gnrc/pktbuf.rs index 858a0da8..c67a8f7c 100644 --- a/src/gnrc/pktbuf.rs +++ b/src/gnrc/pktbuf.rs @@ -291,7 +291,13 @@ impl<'a> Pktsnip { impl ::core::fmt::Debug for Pktsnip { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { + let mode = core::any::type_name::(); + let mode = mode + .rsplit("::") + .next() + .expect("Type name contains :: because it is part of a module"); f.debug_struct("Pktsnip") + .field("M", &mode) .field("length", &self.len()) .field("snips", &self.count()) .finish() From afe025ccb52fe162d9eca1a67fc34ca975888942 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:39:03 +0200 Subject: [PATCH 17/23] panic: Refactor The `unreachable!` previously acted as a safeguard in case the translation of C's "no return" indicator would not work, ensuring the code path is not followed any further -- but the linter didn't like the (currently) unreachable code. Instead, we now implicitly assert that the panic function does not return by not explicitly having a return in that branch. --- src/panic.rs | 67 ++++++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/src/panic.rs b/src/panic.rs index f5cf78d9..41e33547 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -24,46 +24,45 @@ fn panic(info: &::core::panic::PanicInfo) -> ! { cstr::cstr!("RUST PANIC").as_ptr() as _, ) }; - unreachable!() - } - - // I *guess* it's OK for a panic to simply make a thread into a zombie -- this does allow other - // threads (including spawned Rust threads) to continue, but my layman's understanding of - // panicking is that that's OK because whatever we were just mutating can simply never be used - // by someone else ever again. + } else { + // I *guess* it's OK for a panic to simply make a thread into a zombie -- this does allow other + // threads (including spawned Rust threads) to continue, but my layman's understanding of + // panicking is that that's OK because whatever we were just mutating can simply never be used + // by someone else ever again. - let me = thread::get_pid(); + let me = thread::get_pid(); - if cfg!(feature = "panic_handler_format") { - use crate::stdio::println; + if cfg!(feature = "panic_handler_format") { + use crate::stdio::println; - println!( - "Error in thread {:?} ({}):", - me, - me.get_name().unwrap_or("unnamed") - ); - println!("{}", info); - } else { - let mut stdio = crate::stdio::Stdio {}; - use core::fmt::Write; - let _ = stdio.write_str("Panic in thread "); - let _ = stdio.write_str(me.get_name().unwrap_or("unnamed")); - let _ = stdio.write_str("!\n"); - } + println!( + "Error in thread {:?} ({}):", + me, + me.get_name().unwrap_or("unnamed") + ); + println!("{}", info); + } else { + let mut stdio = crate::stdio::Stdio {}; + use core::fmt::Write; + let _ = stdio.write_str("Panic in thread "); + let _ = stdio.write_str(me.get_name().unwrap_or("unnamed")); + let _ = stdio.write_str("!\n"); + } - if cfg!(feature = "panic_handler_crash") { - unsafe { - riot_sys::core_panic( - riot_sys::core_panic_t_PANIC_GENERAL_ERROR, - cstr::cstr!("RUST PANIC").as_ptr() as _, - ) + if cfg!(feature = "panic_handler_crash") { + unsafe { + riot_sys::core_panic( + riot_sys::core_panic_t_PANIC_GENERAL_ERROR, + cstr::cstr!("RUST PANIC").as_ptr() as _, + ) + } } - } - // Not trying any unwinding -- this thread is just dead, won't be re-claimed, any mutexes it - // holds are just held indefinitely rather than throwing poison errors. - loop { - thread::sleep(); + // Not trying any unwinding -- this thread is just dead, won't be re-claimed, any mutexes it + // holds are just held indefinitely rather than throwing poison errors. + loop { + thread::sleep(); + } } } From 6430de23e81ee483efbb662710fc91dceaf9f1ed Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 14 Oct 2023 18:48:09 +0200 Subject: [PATCH 18/23] pktbuf: Fix double free The `forget()` call was on the (Copy, hence a warning that is being fixed) raw pointer instead of the underlying refcounted structure, leading to a double free. --- src/gnrc/pktbuf.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/gnrc/pktbuf.rs b/src/gnrc/pktbuf.rs index e219ae11..858a0da8 100644 --- a/src/gnrc/pktbuf.rs +++ b/src/gnrc/pktbuf.rs @@ -254,13 +254,19 @@ impl<'a> Pktsnip { size: usize, nettype: gnrc_nettype_t, ) -> Result { - let next = next.map(|s| s.ptr).unwrap_or(0 as *mut _); - let snip = - unsafe { gnrc_pktbuf_add(next, data as *const _, size.try_into().unwrap(), nettype) }; + let next_ptr = next.as_ref().map(|s| s.ptr).unwrap_or(0 as *mut _); + forget(next); + let snip = unsafe { + gnrc_pktbuf_add( + next_ptr, + data as *const _, + size.try_into().unwrap(), + nettype, + ) + }; if snip == 0 as *mut _ { return Err(NotEnoughSpace); } - forget(next); Ok(unsafe { Pktsnip::::from_ptr(snip) }) } From bb73bf83d061d5ba4f30ae92df54b210dc455fb2 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:18:44 +0200 Subject: [PATCH 19/23] saul: Don't abuse negative errors with positive values --- src/saul.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/saul.rs b/src/saul.rs index 9dbcafd6..951b72f6 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -91,10 +91,9 @@ impl RegistryEntry { unsafe { riot_sys::saul_reg_write(self.0, &value.values as *const _ as *mut _) } .negative_to_error()?; if length != value.length.into() { - // FIXME is this the best way to express the error? - Err(error::NumericError { - number: length as isize, - }) + // It's not pretty to synthesize an error here, but neither would be expressing the + // partial write in the context of lengthful phydat items. + Err(error::NumericError::from_constant(riot_sys::EINVAL as _)) } else { Ok(()) } From f230e66ac6be146bbe32df4772ba1610f4e0f6b6 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 19:08:02 +0200 Subject: [PATCH 20/23] shell: Refactor to avoid lint complaining about dropping reference The lint is right in that dropping the reference does little, but it ensures that the reference is not held longer than the lock. Using an explicit scope achieves the same without any discussions about the drop. --- src/shell.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/shell.rs b/src/shell.rs index b1a768d7..06a25d34 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -108,11 +108,13 @@ pub unsafe trait CommandListInternals: Sized { let sleeve = lock .as_ref() .expect("Callback called while no shell set up as running"); - // unsafe: A suitable callback is always configured. We can make a &mut out of it for as - // long as we hold the lock. - let root = unsafe { &mut *(sleeve.0 as *mut Self) }; - let result = root.find_self_and_run(argc, argv, command_index); - drop(root); + let result; + { + // unsafe: A suitable callback is always configured. We can make a &mut out of it for as + // long as we hold the lock. + let root = unsafe { &mut *(sleeve.0 as *mut Self) }; + result = root.find_self_and_run(argc, argv, command_index); + } drop(lock); result } From 8f42354b9a033dd92ee825aa2429a27e34882ffa Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 17:05:32 +0200 Subject: [PATCH 21/23] tests: Add gnrc-pktbuf --- tests/gnrc-pktbuf/Cargo.toml | 19 +++++++++++ tests/gnrc-pktbuf/Makefile | 14 ++++++++ tests/gnrc-pktbuf/src/lib.rs | 57 +++++++++++++++++++++++++++++++ tests/gnrc-pktbuf/tests/01-run.py | 10 ++++++ 4 files changed, 100 insertions(+) create mode 100644 tests/gnrc-pktbuf/Cargo.toml create mode 100644 tests/gnrc-pktbuf/Makefile create mode 100644 tests/gnrc-pktbuf/src/lib.rs create mode 100755 tests/gnrc-pktbuf/tests/01-run.py diff --git a/tests/gnrc-pktbuf/Cargo.toml b/tests/gnrc-pktbuf/Cargo.toml new file mode 100644 index 00000000..b23236f4 --- /dev/null +++ b/tests/gnrc-pktbuf/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "riot-wrappers-test-gnrc-pktbuf" +version = "0.1.0" +authors = ["Christian Amsüss "] +edition = "2021" +publish = false + +[lib] +crate-type = ["staticlib"] + +[profile.release] +panic = "abort" + +[dependencies] +riot-wrappers = { version = "*", features = [ "set_panic_handler", "panic_handler_format" ] } +riot-sys = "*" + +[patch.crates-io] +riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" } diff --git a/tests/gnrc-pktbuf/Makefile b/tests/gnrc-pktbuf/Makefile new file mode 100644 index 00000000..d31c9f22 --- /dev/null +++ b/tests/gnrc-pktbuf/Makefile @@ -0,0 +1,14 @@ +# name of your application +APPLICATION = riot-wrappers-test-gnrc-pktbuf +APPLICATION_RUST_MODULE = riot_wrappers_test_gnrc_pktbuf +BASELIBS += $(APPLICATION_RUST_MODULE).module +FEATURES_REQUIRED += rust_target + +# FIXME: That's not a good criterion, gnrc_pktbuf can well stand alone +USEMODULE += gnrc +USEMODULE += gnrc_pktbuf + +# thus we get `gnrc_pktbuf_is_empty` and `_is_sane` +CFLAGS += -DTEST_SUITES + +include $(RIOTBASE)/Makefile.include diff --git a/tests/gnrc-pktbuf/src/lib.rs b/tests/gnrc-pktbuf/src/lib.rs new file mode 100644 index 00000000..bc4666e4 --- /dev/null +++ b/tests/gnrc-pktbuf/src/lib.rs @@ -0,0 +1,57 @@ +#![no_std] + +use riot_wrappers::println; +use riot_wrappers::riot_main; + +riot_main!(main); + +#[track_caller] +fn check() { + assert!( + unsafe { riot_sys::gnrc_pktbuf_is_sane() }, + "GNRC packet buffer in unsound state" + ); +} + +#[track_caller] +fn check_empty() { + check(); + assert!( + unsafe { riot_sys::gnrc_pktbuf_is_empty() }, + "GNRC packet buffer should be empty at this point" + ); +} + +fn main() { + check_empty(); + + let snip1 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate( + 128, + riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, + ) + .unwrap(); + println!("Allocated pktsnip {:?}", snip1); + + check(); + + let snip2 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate_from( + &[1, 2, 3, 4], + riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, + ) + .unwrap(); + println!("Allocated another pktsnip {:?}", snip2); + let snip2 = snip2 + .add(10, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF) + .unwrap(); + println!("... and prefixed it with 10 bytes into {:?}", snip2); + + check(); + + drop(snip1); + drop(snip2); + println!("Dropped it"); + + check_empty(); + + println!("Tests completed."); +} diff --git a/tests/gnrc-pktbuf/tests/01-run.py b/tests/gnrc-pktbuf/tests/01-run.py new file mode 100755 index 00000000..ac26a3bd --- /dev/null +++ b/tests/gnrc-pktbuf/tests/01-run.py @@ -0,0 +1,10 @@ +#!/usr/bin/env python3 + +import sys +from testrunner import run + +def test(child): + child.expect_exact("Tests completed.") + +if __name__ == "__main__": + sys.exit(run(test)) From 7ae33f13e33bd1a15c8a0872aaa4b949ad19d3f2 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 17:25:21 +0200 Subject: [PATCH 22/23] Move gnrc::pktbuf to gnrc_pktbuf The module can exist and be tested without all of GNRC --- src/gnrc.rs | 3 ++- src/{gnrc/pktbuf.rs => gnrc_pktbuf.rs} | 0 src/lib.rs | 3 +++ tests/gnrc-pktbuf/Makefile | 2 -- tests/gnrc-pktbuf/src/lib.rs | 4 ++-- 5 files changed, 7 insertions(+), 5 deletions(-) rename src/{gnrc/pktbuf.rs => gnrc_pktbuf.rs} (100%) diff --git a/src/gnrc.rs b/src/gnrc.rs index 5c0bca52..53839700 100644 --- a/src/gnrc.rs +++ b/src/gnrc.rs @@ -5,7 +5,8 @@ pub mod ipv6; pub mod netapi; pub mod netreg; -pub mod pktbuf; +#[deprecated(note = "moved to gnrc_pktbuf toplevel module")] +pub use crate::gnrc_pktbuf as pktbuf; use riot_sys::{gnrc_netif_iter, gnrc_netif_t}; diff --git a/src/gnrc/pktbuf.rs b/src/gnrc_pktbuf.rs similarity index 100% rename from src/gnrc/pktbuf.rs rename to src/gnrc_pktbuf.rs diff --git a/src/lib.rs b/src/lib.rs index 8c976e6a..5b2c745c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,6 +105,9 @@ pub mod thread; pub mod gcoap; #[cfg(riot_module_gnrc)] pub mod gnrc; +// Note that this can also exist without gnrc +#[cfg(riot_module_gnrc_pktbuf)] +pub mod gnrc_pktbuf; #[cfg(riot_module_gnrc)] pub mod gnrc_util; #[cfg(riot_module_periph_i2c)] diff --git a/tests/gnrc-pktbuf/Makefile b/tests/gnrc-pktbuf/Makefile index d31c9f22..36095a97 100644 --- a/tests/gnrc-pktbuf/Makefile +++ b/tests/gnrc-pktbuf/Makefile @@ -4,8 +4,6 @@ APPLICATION_RUST_MODULE = riot_wrappers_test_gnrc_pktbuf BASELIBS += $(APPLICATION_RUST_MODULE).module FEATURES_REQUIRED += rust_target -# FIXME: That's not a good criterion, gnrc_pktbuf can well stand alone -USEMODULE += gnrc USEMODULE += gnrc_pktbuf # thus we get `gnrc_pktbuf_is_empty` and `_is_sane` diff --git a/tests/gnrc-pktbuf/src/lib.rs b/tests/gnrc-pktbuf/src/lib.rs index bc4666e4..d2a81e3f 100644 --- a/tests/gnrc-pktbuf/src/lib.rs +++ b/tests/gnrc-pktbuf/src/lib.rs @@ -25,7 +25,7 @@ fn check_empty() { fn main() { check_empty(); - let snip1 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate( + let snip1 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate( 128, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, ) @@ -34,7 +34,7 @@ fn main() { check(); - let snip2 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate_from( + let snip2 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate_from( &[1, 2, 3, 4], riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, ) From dfd7a89412de98015bc635545425511ca114b409 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:05:43 +0200 Subject: [PATCH 23/23] tests/gnrc_pktbuf: Try switching between shared and writable --- tests/gnrc-pktbuf/src/lib.rs | 41 ++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/tests/gnrc-pktbuf/src/lib.rs b/tests/gnrc-pktbuf/src/lib.rs index d2a81e3f..64ad97b5 100644 --- a/tests/gnrc-pktbuf/src/lib.rs +++ b/tests/gnrc-pktbuf/src/lib.rs @@ -3,6 +3,8 @@ use riot_wrappers::println; use riot_wrappers::riot_main; +use riot_wrappers::gnrc_pktbuf::{Pktsnip, Shared}; + riot_main!(main); #[track_caller] @@ -25,20 +27,13 @@ fn check_empty() { fn main() { check_empty(); - let snip1 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate( - 128, - riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, - ) - .unwrap(); + let snip1 = Pktsnip::allocate(128, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF).unwrap(); println!("Allocated pktsnip {:?}", snip1); check(); - let snip2 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate_from( - &[1, 2, 3, 4], - riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, - ) - .unwrap(); + let snip2 = + Pktsnip::allocate_from(&[1, 2, 3, 4], riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF).unwrap(); println!("Allocated another pktsnip {:?}", snip2); let snip2 = snip2 .add(10, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF) @@ -47,8 +42,32 @@ fn main() { check(); + let snip2a: Pktsnip = snip2.into(); + let snip2b = snip2a.clone(); + let snip2b = snip2b + .add(10, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF) + .unwrap(); + println!( + "We have two of them now, the second one with an extension: {:?}, {:?}", + snip2a, snip2b + ); + + println!( + "Making the second writable should not change anything about memory usage (not tested)..." + ); + let snip2b: Pktsnip = snip2b.into(); // and the method isn't even available unless we + // make it shared in the first place + let snip2b = snip2b.start_write().unwrap(); + println!("... whereas making the first writable entails copying"); + let snip2a = snip2a.start_write().unwrap(); + println!( + "In the end, they still look the same: {:?}, {:?}", + snip2a, snip2b + ); + drop(snip1); - drop(snip2); + drop(snip2a); + drop(snip2b); println!("Dropped it"); check_empty();