Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clippy updates #50

Merged
merged 8 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions samples/philosophers/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,17 @@ 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,
/// The fork is in use, and someone is waiting on it.
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.
Expand Down
6 changes: 2 additions & 4 deletions samples/philosophers/src/dynsemsync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,13 @@ pub fn dyn_semaphore_sync() -> Vec<Arc<dyn ForkSync>> {
.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(),
};
let item = Box::new(syncer) as Box<dyn ForkSync>;
Arc::from(item)
})
.collect();

syncers
.collect()
}
6 changes: 2 additions & 4 deletions samples/philosophers/src/semsync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,15 @@ pub fn semaphore_sync() -> Vec<Arc<dyn ForkSync>> {
Arc::new(m.init_once((1, 1)).unwrap())
});

let syncers = (0..NUM_PHIL)
(0..NUM_PHIL)
.map(|_| {
let syncer = SemSync {
forks: forks.clone(),
};
let item = Box::new(syncer) as Box<dyn ForkSync>;
Arc::from(item)
})
.collect();

syncers
.collect()
}

kobj_define! {
Expand Down
10 changes: 5 additions & 5 deletions zephyr-build/src/devicetree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 3 additions & 5 deletions zephyr-build/src/devicetree/augment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
35 changes: 15 additions & 20 deletions zephyr-build/src/devicetree/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
};
}
}
}
Expand Down Expand Up @@ -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<W: Write>(&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(())
}
Expand All @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions zephyr-build/src/devicetree/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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),
}
Expand Down Expand Up @@ -140,7 +140,7 @@ fn decode_property(node: Pair<'_, Rule>) -> Property {
}
}

fn decode_words<'i>(node: Pair<'i, Rule>) -> Vec<Word> {
fn decode_words(node: Pair<'_, Rule>) -> Vec<Word> {
let mut value = Vec::new();
for pair in node.into_inner() {
match pair.as_rule() {
Expand All @@ -151,7 +151,7 @@ fn decode_words<'i>(node: Pair<'i, Rule>) -> Vec<Word> {
}
Rule::decimal_number => {
let text = pair.as_str();
let num = u32::from_str_radix(text, 10).unwrap();
let num = text.parse::<u32>().unwrap();
value.push(Word::Number(num));
}
Rule::phandle => {
Expand All @@ -165,7 +165,7 @@ fn decode_words<'i>(node: Pair<'i, Rule>) -> Vec<Word> {
value
}

fn decode_bytes<'i>(node: Pair<'i, Rule>) -> Vec<u8> {
fn decode_bytes(node: Pair<'_, Rule>) -> Vec<u8> {
let mut value = Vec::new();
for pair in node.into_inner() {
match pair.as_rule() {
Expand Down
12 changes: 2 additions & 10 deletions zephyr-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion zephyr-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions zephyr/src/align.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,12 @@ where
AlignAs([])
}
}

impl<const N: usize> Default for AlignAs<N>
where
AlignAsStruct: AlignAsTrait<N>,
{
fn default() -> Self {
Self::new()
}
}
21 changes: 19 additions & 2 deletions zephyr/src/device/gpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GpioToken> {
if !GPIO_TOKEN.once() {
return None;
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion zephyr/src/logging/impl_printk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
26 changes: 17 additions & 9 deletions zephyr/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,13 @@ impl<T> StaticKernelObject<T>
where
StaticKernelObject<T>: 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<T> {
StaticKernelObject {
value: unsafe { mem::zeroed() },
Expand All @@ -184,12 +188,16 @@ where
///
/// If it is called an additional time, it will return None.
pub fn init_once(&self, args: <Self as Wrapped>::I) -> Option<<Self as Wrapped>::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);
Expand Down
10 changes: 9 additions & 1 deletion zephyr/src/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use core::{
cell::UnsafeCell,
convert::Infallible,
fmt,
marker::PhantomData,
ops::{Deref, DerefMut},
Expand All @@ -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<Guard> = Result<Guard, ()>;
pub type LockResult<Guard> = Result<Guard, Infallible>;

/// The return type from [`Mutex::try_lock`].
///
Expand Down Expand Up @@ -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)
Expand Down
Loading