diff --git a/samples/philosophers/src/channel.rs b/samples/philosophers/src/channel.rs index 82781201..f76964f2 100644 --- a/samples/philosophers/src/channel.rs +++ b/samples/philosophers/src/channel.rs @@ -32,8 +32,10 @@ enum Command { } /// This implements a single Fork on the server side for the ChannelSync. +#[derive(Default)] enum ChannelFork { /// The fork is free, + #[default] Free, /// The work is in use, nobody is waiting. InUse, @@ -41,12 +43,6 @@ enum ChannelFork { InUseWait(Sender<()>), } -impl Default for ChannelFork { - fn default() -> Self { - ChannelFork::Free - } -} - impl ChannelFork { /// Attempt to aquire the work. If it is free, reply to the sender, otherwise, track them to /// reply to them when the fork is freed up. diff --git a/samples/philosophers/src/dynsemsync.rs b/samples/philosophers/src/dynsemsync.rs index 74b6f1ee..9f43c0b6 100644 --- a/samples/philosophers/src/dynsemsync.rs +++ b/samples/philosophers/src/dynsemsync.rs @@ -37,7 +37,7 @@ pub fn dyn_semaphore_sync() -> Vec> { .each_ref() .map(|()| Arc::new(Semaphore::new(1, 1).unwrap())); - let syncers = (0..NUM_PHIL) + (0..NUM_PHIL) .map(|_| { let syncer = SemSync { forks: forks.clone(), @@ -45,7 +45,5 @@ pub fn dyn_semaphore_sync() -> Vec> { let item = Box::new(syncer) as Box; Arc::from(item) }) - .collect(); - - syncers + .collect() } diff --git a/samples/philosophers/src/semsync.rs b/samples/philosophers/src/semsync.rs index 0cf901df..c8896fe1 100644 --- a/samples/philosophers/src/semsync.rs +++ b/samples/philosophers/src/semsync.rs @@ -38,7 +38,7 @@ pub fn semaphore_sync() -> Vec> { Arc::new(m.init_once((1, 1)).unwrap()) }); - let syncers = (0..NUM_PHIL) + (0..NUM_PHIL) .map(|_| { let syncer = SemSync { forks: forks.clone(), @@ -46,9 +46,7 @@ pub fn semaphore_sync() -> Vec> { let item = Box::new(syncer) as Box; Arc::from(item) }) - .collect(); - - syncers + .collect() } kobj_define! { diff --git a/zephyr-build/src/devicetree.rs b/zephyr-build/src/devicetree.rs index e3200933..45b68b37 100644 --- a/zephyr-build/src/devicetree.rs +++ b/zephyr-build/src/devicetree.rs @@ -5,13 +5,13 @@ //! different ways: //! //! - Canonical DTS. There is a single DTS file (`build/zephyr/zephyr.dts`) that contains the final -//! tree, but still in DTS format (the DTB file would have information discarded). +//! tree, but still in DTS format (the DTB file would have information discarded). //! //! - Generated. The C header `devicetree_generated.h` contains all of the definitions. This isn't -//! a particularly friendly file to read or parse, but it does have one piece of information that is -//! not represented anywhere else: the mapping between devicetree nodes and their "ORD" index. The -//! device nodes in the system are indexed by this number, and we need this in order to be able to -//! reference the nodes from Rust. +//! a particularly friendly file to read or parse, but it does have one piece of information that is +//! not represented anywhere else: the mapping between devicetree nodes and their "ORD" index. The +//! device nodes in the system are indexed by this number, and we need this in order to be able to +//! reference the nodes from Rust. //! //! Beyond the ORD field, it seems easier to deal with the DTS file itself. Parsing is fairly //! straightforward, as it is a subset of the DTS format, and we only have to be able to deal with diff --git a/zephyr-build/src/devicetree/augment.rs b/zephyr-build/src/devicetree/augment.rs index 156a737b..55f150db 100644 --- a/zephyr-build/src/devicetree/augment.rs +++ b/zephyr-build/src/devicetree/augment.rs @@ -112,12 +112,10 @@ fn parent_compatible(node: &Node, names: &[String], level: usize) -> bool { // runs on the host, so the stack is easier. if level == 0 { names.iter().any(|n| node.is_compatible(n)) + } else if let Some(parent) = node.parent.borrow().as_ref() { + parent_compatible(parent, names, level - 1) } else { - if let Some(parent) = node.parent.borrow().as_ref() { - parent_compatible(parent, names, level - 1) - } else { - false - } + false } } diff --git a/zephyr-build/src/devicetree/output.rs b/zephyr-build/src/devicetree/output.rs index bedb28f0..bf132856 100644 --- a/zephyr-build/src/devicetree/output.rs +++ b/zephyr-build/src/devicetree/output.rs @@ -22,7 +22,7 @@ impl DeviceTree { // Root is a little special. Since we don't want a module for this (it will be provided // above where it is included, so it can get documentation and attributes), we use None for // the name. - self.node_walk(self.root.as_ref(), None, &augments) + self.node_walk(self.root.as_ref(), None, augments) } // Write, to the given writer, CFG lines so that Rust code can conditionalize based on the DT. @@ -98,14 +98,11 @@ impl DeviceTree { match value { Value::Words(ref words) => { if words.len() == 1 { - match &words[0] { - Word::Number(n) => { - let tag = dt_to_upper_id(&prop.name); - return quote! { - pub const #tag: u32 = #n; - }; - } - _ => (), + if let Word::Number(n) = &words[0] { + let tag = dt_to_upper_id(&prop.name); + return quote! { + pub const #tag: u32 = #n; + }; } } } @@ -173,15 +170,13 @@ impl Property { // If this property is a single top-level phandle, output that a that path is valid. It isn't a // real node, but acts like one. fn output_path(&self, write: &mut W, name: &str) -> Result<()> { - if let Some(value) = self.get_single_value() { - if let Value::Phandle(_) = value { - writeln!( - write, - "cargo:rustc-cfg=dt=\"{}::{}\"", - name, - fix_id(&self.name) - )?; - } + if let Some(Value::Phandle(_)) = self.get_single_value() { + writeln!( + write, + "cargo:rustc-cfg=dt=\"{}::{}\"", + name, + fix_id(&self.name) + )?; } Ok(()) } @@ -192,13 +187,13 @@ fn general_property(prop: &Property) -> TokenStream { let tag = format!("{}_DEBUG", prop.name); let tag = dt_to_upper_id(&tag); quote! { - pub const #tag: &'static str = #text; + pub const #tag: &str = #text; } } /// Given a DT name, return an identifier for a lower-case version. pub fn dt_to_lower_id(text: &str) -> Ident { - format_ident!("{}", fix_id(&text)) + format_ident!("{}", fix_id(text)) } pub fn dt_to_upper_id(text: &str) -> Ident { diff --git a/zephyr-build/src/devicetree/parse.rs b/zephyr-build/src/devicetree/parse.rs index ecc75502..c3d6c32a 100644 --- a/zephyr-build/src/devicetree/parse.rs +++ b/zephyr-build/src/devicetree/parse.rs @@ -60,7 +60,7 @@ impl<'a> TreeBuilder<'a> { println!("Root: {:?} {}", name, ord); */ - let mut name = LazyName::new(path, route.to_owned(), &self.ords); + let mut name = LazyName::new(path, route.to_owned(), self.ords); let mut labels = Vec::new(); let mut properties = Vec::new(); let mut children = Vec::new(); @@ -79,7 +79,7 @@ impl<'a> TreeBuilder<'a> { } Rule::node => { let child_path = name.path_ref(); - children.push(self.walk_node(pair, child_path, &name.route_ref())); + children.push(self.walk_node(pair, child_path, name.route_ref())); } r => panic!("node: {:?}", r), } @@ -140,7 +140,7 @@ fn decode_property(node: Pair<'_, Rule>) -> Property { } } -fn decode_words<'i>(node: Pair<'i, Rule>) -> Vec { +fn decode_words(node: Pair<'_, Rule>) -> Vec { let mut value = Vec::new(); for pair in node.into_inner() { match pair.as_rule() { @@ -151,7 +151,7 @@ fn decode_words<'i>(node: Pair<'i, Rule>) -> Vec { } Rule::decimal_number => { let text = pair.as_str(); - let num = u32::from_str_radix(text, 10).unwrap(); + let num = text.parse::().unwrap(); value.push(Word::Number(num)); } Rule::phandle => { @@ -165,7 +165,7 @@ fn decode_words<'i>(node: Pair<'i, Rule>) -> Vec { value } -fn decode_bytes<'i>(node: Pair<'i, Rule>) -> Vec { +fn decode_bytes(node: Pair<'_, Rule>) -> Vec { let mut value = Vec::new(); for pair in node.into_inner() { match pair.as_rule() { diff --git a/zephyr-build/src/lib.rs b/zephyr-build/src/lib.rs index 898e33a9..4c248125 100644 --- a/zephyr-build/src/lib.rs +++ b/zephyr-build/src/lib.rs @@ -72,12 +72,7 @@ pub fn build_kconfig_mod() { writeln!(&mut f, "pub const {}: isize = {};", &caps[1], &caps[2]).unwrap(); } else if let Some(caps) = config_str.captures(&line) { writeln!(&mut f, "#[allow(dead_code)]").unwrap(); - writeln!( - &mut f, - "pub const {}: &'static str = {};", - &caps[1], &caps[2] - ) - .unwrap(); + writeln!(&mut f, "pub const {}: &str = {};", &caps[1], &caps[2]).unwrap(); } } } @@ -140,10 +135,7 @@ pub fn dt_cfgs() { /// Determine if `rustfmt` is in the path, and can be excecuted. Returns false on any kind of error. pub fn has_rustfmt() -> bool { - match Command::new("rustfmt").arg("--version").status() { - Ok(st) if st.success() => true, - _ => false, - } + matches!(Command::new("rustfmt").arg("--version").status(), Ok(st) if st.success()) } /// Attempt to write the contents to a file, using rustfmt. If there is an error running rustfmt, diff --git a/zephyr-sys/src/lib.rs b/zephyr-sys/src/lib.rs index 6528b9af..e7210859 100644 --- a/zephyr-sys/src/lib.rs +++ b/zephyr-sys/src/lib.rs @@ -16,7 +16,11 @@ #![allow(improper_ctypes)] #![allow(rustdoc::broken_intra_doc_links)] #![allow(rustdoc::bare_urls)] - +// Disable various clippy warnings as they will not be fixable in the bindgen generated code. +#![allow(clippy::missing_safety_doc)] +#![allow(clippy::transmute_int_to_bool)] +#![allow(clippy::useless_transmute)] +#![allow(clippy::len_without_is_empty)] include!(concat!(env!("OUT_DIR"), "/bindings.rs")); // We have directed bindgen to not generate copy for any times. It unfortunately doesn't have an diff --git a/zephyr/src/align.rs b/zephyr/src/align.rs index 31d2c2c0..4a72432c 100644 --- a/zephyr/src/align.rs +++ b/zephyr/src/align.rs @@ -48,3 +48,12 @@ where AlignAs([]) } } + +impl Default for AlignAs +where + AlignAsStruct: AlignAsTrait, +{ + fn default() -> Self { + Self::new() + } +} diff --git a/zephyr/src/device/gpio.rs b/zephyr/src/device/gpio.rs index 4cbd4f64..14a562e0 100644 --- a/zephyr/src/device/gpio.rs +++ b/zephyr/src/device/gpio.rs @@ -24,8 +24,13 @@ pub struct GpioToken(()); static GPIO_TOKEN: Unique = Unique::new(); impl GpioToken { - /// Retrieves the gpio token. This is unsafe because lots of code in zephyr operates on the - /// gpio drivers. + /// Retrieves the gpio token. + /// + /// # Safety + /// This is unsafe because lots of code in zephyr operates on the gpio drivers. The user of the + /// gpio subsystem, in general should either coordinate all gpio access across the system (the + /// token coordinates this only within Rust code), or verify that the particular gpio driver and + /// methods are thread safe. pub unsafe fn get_instance() -> Option { if !GPIO_TOKEN.once() { return None; @@ -112,6 +117,12 @@ impl GpioPin { } /// Configure a single pin. + /// + /// # Safety + /// + /// The `_token` enforces single threaded use of gpios from Rust code. However, many drivers + /// within Zephyr use GPIOs, and to use gpios safely, the caller must ensure that there is + /// either not simultaneous use, or the gpio driver in question is thread safe. pub unsafe fn configure(&mut self, _token: &mut GpioToken, extra_flags: raw::gpio_flags_t) { // TODO: Error? unsafe { @@ -124,6 +135,12 @@ impl GpioPin { } /// Toggle pin level. + /// + /// # Safety + /// + /// The `_token` enforces single threaded use of gpios from Rust code. However, many drivers + /// within Zephyr use GPIOs, and to use gpios safely, the caller must ensure that there is + /// either not simultaneous use, or the gpio driver in question is thread safe. pub unsafe fn toggle_pin(&mut self, _token: &mut GpioToken) { // TODO: Error? unsafe { diff --git a/zephyr/src/logging/impl_printk.rs b/zephyr/src/logging/impl_printk.rs index 72a1d5c2..c41ce3b5 100644 --- a/zephyr/src/logging/impl_printk.rs +++ b/zephyr/src/logging/impl_printk.rs @@ -35,8 +35,10 @@ static PRINTK_LOGGER: PrintkLogger = PrintkLogger; /// Set the log handler to log messages through printk in Zephyr. /// +/// # Safety +/// /// This is unsafe due to racy issues in the log framework on targets that do not support atomic -/// pointers. +/// pointers. As long as this is called ever by a single thread, it is safe to use. pub unsafe fn set_logger() -> Result<(), SetLoggerError> { super::set_logger_internal(&PRINTK_LOGGER) } diff --git a/zephyr/src/object.rs b/zephyr/src/object.rs index aab7be78..6f40ea09 100644 --- a/zephyr/src/object.rs +++ b/zephyr/src/object.rs @@ -167,9 +167,13 @@ impl StaticKernelObject where StaticKernelObject: Wrapped, { - /// Construct an empty of these objects, with the zephyr data zero-filled. This is safe in the - /// sense that Zephyr we track the initialization, they start in the uninitialized state, and - /// the zero value of the initialize atomic indicates that it is uninitialized. + /// Construct an empty of these objects, with the zephyr data zero-filled. + /// + /// # Safety + /// + /// This is safe in the sense that Zephyr we track the initialization, they start in the + /// uninitialized state, and the zero value of the initialize atomic indicates that it is + /// uninitialized. pub const unsafe fn new() -> StaticKernelObject { StaticKernelObject { value: unsafe { mem::zeroed() }, @@ -184,12 +188,16 @@ where /// /// If it is called an additional time, it will return None. pub fn init_once(&self, args: ::I) -> Option<::T> { - if let Err(_) = self.init.compare_exchange( - KOBJ_UNINITIALIZED, - KOBJ_INITING, - Ordering::AcqRel, - Ordering::Acquire, - ) { + if self + .init + .compare_exchange( + KOBJ_UNINITIALIZED, + KOBJ_INITING, + Ordering::AcqRel, + Ordering::Acquire, + ) + .is_err() + { return None; } let result = self.get_wrapped(args); diff --git a/zephyr/src/sync/mutex.rs b/zephyr/src/sync/mutex.rs index c8431c73..2e5b2886 100644 --- a/zephyr/src/sync/mutex.rs +++ b/zephyr/src/sync/mutex.rs @@ -6,6 +6,7 @@ use core::{ cell::UnsafeCell, + convert::Infallible, fmt, marker::PhantomData, ops::{Deref, DerefMut}, @@ -15,7 +16,7 @@ use crate::sys::sync as sys; use crate::time::{Forever, NoWait}; /// Until poisoning is implemented, mutexes never return an error, and we just get back the guard. -pub type LockResult = Result; +pub type LockResult = Result; /// The return type from [`Mutex::try_lock`]. /// @@ -238,6 +239,13 @@ impl Condvar { } } +#[cfg(CONFIG_RUST_ALLOC)] +impl Default for Condvar { + fn default() -> Self { + Self::new() + } +} + impl fmt::Debug for Condvar { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Condvar {:?}", self.inner) diff --git a/zephyr/src/sys/queue.rs b/zephyr/src/sys/queue.rs index f52fa57e..5207ef39 100644 --- a/zephyr/src/sys/queue.rs +++ b/zephyr/src/sys/queue.rs @@ -50,6 +50,16 @@ impl Queue { /// safely. /// /// [`Message`]: crate::sync::channel::Message + /// + /// # Safety + /// + /// Zephyr has specific requirements on the memory given in data, which can be summarized as: + /// - The memory must remain valid until after the data is returned, via recv. + /// - The first `usize` in the pointed data will be mutated by Zephyr to manage structures. + /// - This first field must not be modified by any code while the message is enqueued. + /// + /// These are easiest to satisfy by ensuring the message is Boxed, and owned by the queue + /// system. pub unsafe fn send(&self, data: *mut c_void) { k_queue_append(self.item.get(), data) } @@ -64,6 +74,11 @@ impl Queue { /// [`Forever`]: crate::time::Forever /// [`NoWait`]: crate::time::NoWait /// [`Duration`]: crate::time::Duration + /// + /// # Safety + /// + /// Once an item is received from a queue, ownership is returned to the caller, and Zephyr no + /// longer depends on it not being freed, or the first `usize` field being for its use. pub unsafe fn recv(&self, timeout: T) -> *mut c_void where T: Into, diff --git a/zephyr/src/sys/thread.rs b/zephyr/src/sys/thread.rs index 92999ff0..f2a8eeac 100644 --- a/zephyr/src/sys/thread.rs +++ b/zephyr/src/sys/thread.rs @@ -227,6 +227,12 @@ impl Thread { /// Simple thread spawn. This is unsafe because of the raw values being used. This can be /// useful in systems without an allocator defined. + /// + /// # Safety + /// + /// Safe use follows similar requirements to using this safely from within C code. Passing Rust + /// values through this interface is difficult to get right, and it is generally recommended to + /// use [`spawn`]. pub unsafe fn simple_spawn( mut self, child: k_thread_entry_t, @@ -501,7 +507,7 @@ mod closure { pub unsafe extern "C" fn child(child: *mut c_void, _p2: *mut c_void, _p3: *mut c_void) { let thread_data: Box = unsafe { Box::from_raw(child as *mut ThreadData) }; - let closure = (*thread_data).closure; + let closure = thread_data.closure; closure(); } } diff --git a/zephyr/src/timer.rs b/zephyr/src/timer.rs index a8606a35..bfa224ca 100644 --- a/zephyr/src/timer.rs +++ b/zephyr/src/timer.rs @@ -136,6 +136,12 @@ impl fmt::Debug for StoppedTimer { } } +impl Default for StoppedTimer { + fn default() -> Self { + Self::new() + } +} + /// A statically allocated `k_timer` (StoppedTimer). /// /// This is intended to be used from within the `kobj_define!` macro. It declares a static