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

Fix clippy warnings and enforce -Dwarnings #224

Merged
merged 10 commits into from
Aug 11, 2023
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
25 changes: 18 additions & 7 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,17 @@ _rustflags := env_var_or_default("RUSTFLAGS", "")

# If we're running in Github Actions and cargo-action-fmt is installed, then add
# a command suffix that formats errors.
_fmt := if env_var_or_default("GITHUB_ACTIONS", "") != "true" { "" } else {
#
# Clippy version also gets -Dwarnings.
_fmt_clippy := if env_var_or_default("GITHUB_ACTIONS", "") != "true" { "-- -Dwarnings" } else {
```
if command -v cargo-action-fmt >/dev/null 2>&1; then
echo "--message-format=json -- -Dwarnings | cargo-action-fmt"
fi
```
}
jamesmunns marked this conversation as resolved.
Show resolved Hide resolved

_fmt_check_doc := if env_var_or_default("GITHUB_ACTIONS", "") != "true" { "" } else {
```
if command -v cargo-action-fmt >/dev/null 2>&1; then
echo "--message-format=json | cargo-action-fmt"
Expand Down Expand Up @@ -47,27 +57,28 @@ default:
check: && (check-crate _d1_pkg) (check-crate _espbuddy_pkg) (check-crate _x86_bootloader_pkg)
{{ _cargo }} check \
--lib --bins --examples --tests --benches \
{{ _fmt }}
{{ _fmt_check_doc }}

# check a crate.
check-crate crate:
{{ _cargo }} check \
--lib --bins --examples --tests --benches --all-features \
--package {{ crate }} \
{{ _fmt }}
{{ _fmt_check_doc }}

# run Clippy checks for all crates, across workspaces.
clippy: && (clippy-crate _d1_pkg) (clippy-crate _espbuddy_pkg) (clippy-crate _x86_bootloader_pkg)
{{ _cargo }} clippy \
--lib --bins --examples --tests --benches --all-features \
{{ _fmt }}
{{ _fmt_clippy }}

# run clippy checks for a crate.
# NOTE: -Dwarnings is added by _fmt because reasons
clippy-crate crate:
{{ _cargo }} clippy \
--lib --bins --examples --tests --benches \
--package {{ crate }} \
{{ _fmt }}
{{ _fmt_clippy }}

# test all packages, across workspaces
test: (_get-cargo-command "nextest" "cargo-nextest" no-nextest)
Expand Down Expand Up @@ -138,7 +149,7 @@ docs *FLAGS:
{{ _cargo }} doc \
--all-features \
{{ FLAGS }} \
{{ _fmt }}
{{ _fmt_check_doc }}

_get-cargo-command name pkg skip='':
#!/usr/bin/env bash
Expand All @@ -158,4 +169,4 @@ _get-cargo-command name pkg skip='':
err "missing cargo-{{ name }} executable"
if confirm " install it?"; then
cargo install {{ pkg }}
fi
fi
2 changes: 1 addition & 1 deletion platforms/allwinner-d1/boards/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::path::Path;
fn main() {
let out_dir = env::var("OUT_DIR").expect("No out dir");
let dest_path = Path::new(&out_dir);
let mut f = File::create(&dest_path.join("memory.x")).expect("Could not create file");
let mut f = File::create(dest_path.join("memory.x")).expect("Could not create file");

f.write_all(include_bytes!("memory.x"))
.expect("Could not write file");
Expand Down
2 changes: 2 additions & 0 deletions platforms/allwinner-d1/boards/src/bin/mq-pro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ fn main() -> ! {
d1.run()
}

// Note: pass by ref mut to enforce exclusive access
#[allow(clippy::needless_pass_by_ref_mut)]
fn init_i2c_puppet_irq(gpio: &mut d1_pac::GPIO, plic: &mut Plic) -> &'static WaitCell {
use d1_pac::Interrupt;
use mnemos_d1_core::plic::Priority;
Expand Down
6 changes: 6 additions & 0 deletions platforms/allwinner-d1/core/src/clint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ impl Clint {
self.clint
}

/// Summon the clint peripheral
///
/// # Safety
///
/// This is intended for use in interrupt context. Care should be taken not to have
/// multiple instances live at the same time that may race or cause other UB issues
#[must_use]
pub unsafe fn summon() -> Self {
Self {
Expand Down
5 changes: 4 additions & 1 deletion platforms/allwinner-d1/core/src/drivers/spim.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// Spi Sender
// Note: We sometimes force a pass by ref mut to enforce exclusive access
#![allow(clippy::needless_pass_by_ref_mut)]

//! Spi Sender

use core::ptr::NonNull;

Expand Down
3 changes: 3 additions & 0 deletions platforms/allwinner-d1/core/src/drivers/twi.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Note: We sometimes force a pass by ref mut to enforce exclusive access
#![allow(clippy::needless_pass_by_ref_mut)]

//! Drivers for the Allwinner D1's I²C/TWI peripherals.
//!
//! This module contains an implementation of a driver for controlling the
Expand Down
3 changes: 3 additions & 0 deletions platforms/allwinner-d1/core/src/drivers/uart.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Note: We sometimes force a pass by ref mut to enforce exclusive access
#![allow(clippy::needless_pass_by_ref_mut)]

use d1_pac::{GPIO, UART0};

use core::{
Expand Down
2 changes: 1 addition & 1 deletion platforms/beepy/src/i2c_puppet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ pub enum RegistrationError {
// https://github.com/solderparty/i2c_puppet#protocol
const ADDR: u8 = 0x1f;

//// i2c_puppet I2C registers
/// i2c_puppet I2C registers
mod reg {
/// To write with a register, we must OR the register number with this mask:
/// <https://github.com/solderparty/i2c_puppet#protocol>
Expand Down
4 changes: 2 additions & 2 deletions platforms/esp32c3-buddy/src/bin/qtpy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn main() -> ! {

let k = mnemos_esp32c3_buddy::init();
mnemos_esp32c3_buddy::spawn_serial(
&k,
k,
peripherals.USB_DEVICE,
&mut system.peripheral_clock_control,
);
Expand All @@ -51,5 +51,5 @@ fn main() -> ! {
// Alarm 1 will be used to generate "sleep until" interrupts.
let alarm1 = syst.alarm1;

mnemos_esp32c3_buddy::run(&k, alarm1)
mnemos_esp32c3_buddy::run(k, alarm1)
}
4 changes: 2 additions & 2 deletions platforms/esp32c3-buddy/src/bin/xiao.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn main() -> ! {

mnemos_esp32c3_buddy::spawn_daemons(k);
mnemos_esp32c3_buddy::spawn_serial(
&k,
k,
peripherals.USB_DEVICE,
&mut system.peripheral_clock_control,
);
Expand All @@ -51,5 +51,5 @@ fn main() -> ! {
// Alarm 1 will be used to generate "sleep until" interrupts.
let alarm1 = syst.alarm1;

mnemos_esp32c3_buddy::run(&k, alarm1)
mnemos_esp32c3_buddy::run(k, alarm1)
}
7 changes: 7 additions & 0 deletions platforms/esp32c3-buddy/src/heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ impl UnderlyingAllocator for UnderlyingEspHeap {
///
/// May or may not require a call to [UnderlyingAllocator::init()] before the allocator
/// is actually ready for use.
//
// clippy note: <https://rust-lang.github.io/rust-clippy/master/index.html#/declare_interior_mutable_const>
//
// > A “non-constant” const item is a legacy way to supply an initialized value to
// > downstream static items (e.g., the std::sync::ONCE_INIT constant). In this
// > case the use of const is legit, and this lint should be suppressed.
#[allow(clippy::declare_interior_mutable_const)]
const INIT: Self = UnderlyingEspHeap(EspHeap::empty());

/// Initialize the allocator, if it is necessary to populate with a region
Expand Down
4 changes: 2 additions & 2 deletions platforms/esp32c3-buddy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub fn run(k: &'static Kernel, alarm1: Alarm<Target, 1>) -> ! {
// Timer is downcounting
let elapsed = SystemTimer::now() - start;

let turn = k.timer().force_advance_ticks(elapsed as u64 / 2u64);
let turn = k.timer().force_advance_ticks(elapsed / 2u64);

// If there is nothing else scheduled, and we didn't just wake something up,
// sleep for some amount of time
Expand Down Expand Up @@ -138,7 +138,7 @@ pub fn run(k: &'static Kernel, alarm1: Alarm<Target, 1>) -> ! {
// Account for time slept
let elapsed = SystemTimer::now() - wfi_start;

let _turn = k.timer().force_advance_ticks(elapsed as u64 / 2u64);
let _turn = k.timer().force_advance_ticks(elapsed / 2u64);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions platforms/melpomene/src/sim_drivers/tcp_serial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ impl TcpSerial {

let _hdl = tokio::spawn(
async move {
let mut handle = a_ring;
let handle = a_ring;
loop {
match listener.accept().await {
Ok((stream, addr)) => {
process_stream(&mut handle, stream, irq.clone())
process_stream(&handle, stream, irq.clone())
.instrument(info_span!("process_stream", client.addr = %addr))
.await
}
Expand All @@ -88,7 +88,7 @@ impl TcpSerial {
}
}

async fn process_stream(handle: &mut BidiHandle, mut stream: TcpStream, irq: Arc<Notify>) {
async fn process_stream(handle: &BidiHandle, mut stream: TcpStream, irq: Arc<Notify>) {
loop {
// Wait until either the socket has data to read, or the other end of
// the BBQueue has data to write.
Expand All @@ -111,7 +111,7 @@ async fn process_stream(handle: &mut BidiHandle, mut stream: TcpStream, irq: Arc
// Try to read data, this may still fail with `WouldBlock`
// if the readiness event is a false positive.
match stream.try_read(&mut in_grant) {
Ok(used) if used == 0 => {
Ok(0) => {
warn!("Empty read, socket probably closed.");
return;
},
Expand Down
1 change: 1 addition & 0 deletions platforms/x86_64/core/src/bin/bootloader/framebuf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub(super) unsafe fn mk_framebuf() -> FramebufWriter {
/// This forcibly unlocks a potentially-locked mutex, violating mutual
/// exclusion! This should only be called in conditions where no other CPU core
/// will *ever* attempt to access the framebuffer again (such as while oopsing).
#[allow(dead_code)]
pub(super) unsafe fn force_unlock() {
if let Some((_, fb)) = FRAMEBUFFER.try_get() {
fb.force_unlock();
Expand Down
1 change: 1 addition & 0 deletions platforms/x86_64/core/src/bin/bootloader/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub fn kernel_start(info: &'static mut bootloader_api::BootInfo) -> ! {

#[cold]
#[cfg_attr(target_os = "none", panic_handler)]
#[allow(dead_code)]
fn panic(panic: &core::panic::PanicInfo<'_>) -> ! {
use core::fmt::Write;
use embedded_graphics::{
Expand Down
4 changes: 2 additions & 2 deletions platforms/x86_64/core/src/drivers/framebuf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ where
// if we have reached the bottom of the screen, we'll need to scroll
// previous framebuffer contents up to make room for new line(s) of
// text.
self.point.y = self.point.y + self.style.font.character_size.height as i32;
self.point.y += self.style.font.character_size.height as i32;
self.point.x = self.start_x;
}
}
Expand Down Expand Up @@ -133,7 +133,7 @@ where
// line, wrap the line.
let rem = self.px_to_len(self.width_px - (self.point.x as u32));
if line.len() > rem {
let (curr, next) = line.split_at(rem as usize);
let (curr, next) = line.split_at(rem);
line = next;
chunk = curr;
has_newline = true;
Expand Down
2 changes: 1 addition & 1 deletion platforms/x86_64/core/src/interrupt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl hal_core::interrupt::Handlers<Registers> for InterruptHandlers {
C: interrupt::Context<Registers = Registers> + interrupt::ctx::CodeFault,
{
// TODO: add a nice fault handler
let _fault = match cx.details() {
match cx.details() {
Some(deets) => panic!("code fault {}: \n{deets}", cx.fault_kind()),
None => panic!("code fault {}!", cx.fault_kind()),
};
Expand Down
2 changes: 1 addition & 1 deletion platforms/x86_64/core/src/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ where
}
}) as &mut dyn tracing::field::Visit,
);
writeln!(&mut writer, "").unwrap();
writeln!(&mut writer).unwrap();

self.point
.store(pack_point(writer.next_point()), Ordering::Release);
Expand Down
Loading