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 a handful of clippy lints #129

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
76f1d2c
Add clippy support to xtask check
smklein Jan 10, 2021
bfb8db4
Fix a handful of clippy lints
smklein Jan 10, 2021
e67dbb0
Merge branch 'master' into clippy
smklein Jan 12, 2021
abac873
Separate clippy from check task
smklein Jan 12, 2021
add2784
Update README with necessary dependencies
smklein Jan 10, 2021
54939f1
Don't allow an unnecssary transmute
smklein Jan 12, 2021
e55e9dc
Merge branch 'master' into clippy-fixes
smklein Jan 12, 2021
978788c
Merge branch 'master' into clippy-fixes
smklein Jan 17, 2021
c5cb79b
Merge branch 'master' into clippy
smklein Jan 17, 2021
1ac846e
Merge branch 'clippy' into clippy-fixes
smklein Jan 17, 2021
e8a4f73
Omit unnecessary type annotation
smklein Jan 17, 2021
aa2b08b
Format kern's cfg-if'd modules
smklein Jan 20, 2021
5f062f6
Merge branch 'format' into clippy-fixes
smklein Jan 20, 2021
d4122c4
Fix more lints
smklein Jan 20, 2021
c82eab0
Allow a limited set of lints
smklein Jan 20, 2021
ff1507f
Merge branch 'clippy' into format
smklein Jan 20, 2021
4edc5ba
Merge branch 'format' into clippy-fixes
smklein Jan 20, 2021
d416006
Merge branch 'master' into clippy
smklein Jan 20, 2021
b2316fb
Merge branch 'clippy' into format
smklein Jan 20, 2021
dfd9190
Merge branch 'format' into clippy-fixes
smklein Jan 20, 2021
2e564db
Format
smklein Jan 20, 2021
11926b5
Merge branch 'master' into clippy
smklein Jan 24, 2021
3b8c964
Merge branch 'clippy' into format
smklein Jan 24, 2021
812d534
Merge branch 'format' into clippy-fixes
smklein Jan 24, 2021
3963743
Merge branch 'master' into clippy
smklein Jan 26, 2021
020d875
Merge branch 'clippy' into clippy-fixes
smklein Jan 26, 2021
0078616
Merge branch 'master' into clippy
smklein Jan 26, 2021
f8f487c
Merge branch 'clippy' into clippy-fixes
smklein Jan 26, 2021
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
3 changes: 2 additions & 1 deletion abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use zerocopy::{AsBytes, FromBytes, Unaligned};

/// Magic number that appears at the start of an application header (`App`) to
/// reassure the kernel that it is not reading uninitialized Flash.
pub const CURRENT_APP_MAGIC: u32 = 0x1DE_fa7a1;
#[allow(clippy::unusual_byte_groupings)]
pub const CURRENT_APP_MAGIC: u32 = 0x1DE_FA7A1;

/// Number of region slots in a `TaskDesc` record. Needs to be less or equal to
/// than the number of regions in the MPU; may be less to improve context switch
Expand Down
18 changes: 6 additions & 12 deletions drv/lpc55-spi/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fn write_byte(spi: &device::spi0::RegisterBlock, tx: &mut Option<Transmit>) {
// not easy. For now just send 0 if we're trying to receive but
// not actually write
spi.fifowr
.write(|w| unsafe { w.len().bits(7).txdata().bits(0x00 as u16) });
.write(|w| unsafe { w.len().bits(7).txdata().bits(0x00_u16) });
smklein marked this conversation as resolved.
Show resolved Hide resolved

return;
}
Expand Down Expand Up @@ -305,14 +305,11 @@ fn write_byte(spi: &device::spi0::RegisterBlock, tx: &mut Option<Transmit>) {
});
if txs.pos == txs.len {
spi.fifointenclr.write(|w| w.txlvl().set_bit());
core::mem::replace(tx, None).unwrap().task.reply(());
tx.take().unwrap().task.reply(());
}
} else {
spi.fifointenclr.write(|w| w.txlvl().set_bit());
core::mem::replace(tx, None)
.unwrap()
.task
.reply_fail(ResponseCode::BadArg);
tx.take().unwrap().task.reply_fail(ResponseCode::BadArg);
}
}

Expand All @@ -328,19 +325,16 @@ fn read_byte(spi: &device::spi0::RegisterBlock, tx: &mut Option<Transmit>) {

let borrow = txs.task.borrow(0);

if let Some(_) = borrow.write_at(txs.pos, byte) {
if borrow.write_at(txs.pos, byte).is_some() {
txs.pos += 1;
if txs.pos == txs.len {
spi.fifointenclr.write(|w| w.txlvl().set_bit());
spi.fifointenclr.write(|w| w.rxlvl().set_bit());
core::mem::replace(tx, None).unwrap().task.reply(());
tx.take().unwrap().task.reply(());
}
} else {
spi.fifointenclr.write(|w| w.txlvl().set_bit());
spi.fifointenclr.write(|w| w.rxlvl().set_bit());
core::mem::replace(tx, None)
.unwrap()
.task
.reply_fail(ResponseCode::BadArg);
tx.take().unwrap().task.reply_fail(ResponseCode::BadArg);
}
}
2 changes: 1 addition & 1 deletion drv/stm32f4-usart/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ fn step_transmit(
state: &mut Option<Transmit>,
) -> hl::Caller<()> {
usart.cr1.modify(|_, w| w.txeie().disabled());
core::mem::replace(state, None).unwrap().caller
state.take().unwrap().caller
}

let txs = if let Some(txs) = tx { txs } else { return };
Expand Down
2 changes: 1 addition & 1 deletion drv/stm32h7-usart/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ fn step_transmit(
state: &mut Option<Transmit>,
) -> hl::Caller<()> {
usart.cr1.modify(|_, w| w.txeie().disabled());
core::mem::replace(state, None).unwrap().caller
state.take().unwrap().caller
}

let txs = if let Some(txs) = tx { txs } else { return };
Expand Down
3 changes: 1 addition & 2 deletions kern/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fs::File;
use std::io::Write;
use std::path::PathBuf;

fn main() -> Result<(), Box<dyn std::error::Error>> {
fn main() {
build_util::expose_m_profile();

let out = &PathBuf::from(env::var_os("OUT_DIR").unwrap());
Expand Down Expand Up @@ -39,5 +39,4 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
writeln!(const_file, "pub const EXC_RETURN_CONST : u32 = 0xFFFFFFED;")
.unwrap();
}
Ok(())
}
37 changes: 20 additions & 17 deletions kern/src/arch/arm_m.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//!
//! For performance and (believe it or not) simplicity, this implementation uses
//! several different interrupt service routines:
//!
//!
//! - `SVCall` implements the `SVC` instruction used to make syscalls.
//! - `SysTick` handles interrupts from the System Tick Timer, used to maintain
//! the kernel timestamp.
Expand Down Expand Up @@ -72,11 +72,11 @@ use core::ptr::NonNull;

use zerocopy::FromBytes;

use abi::{FaultSource, FaultInfo};
use crate::app;
use crate::task;
use crate::time::Timestamp;
use crate::umem::USlice;
use abi::{FaultInfo, FaultSource};

/// Log things from kernel context. This macro is made visible to the rest of
/// the kernel by a chain of `#[macro_use]` attributes, but its implementation
Expand All @@ -96,8 +96,8 @@ use crate::umem::USlice;
///
#[cfg(not(any(feature = "klog-semihosting", feature = "klog-itm")))]
macro_rules! klog {
($s:expr) => { };
($s:expr, $($tt:tt)*) => { };
($s:expr) => {};
($s:expr, $($tt:tt)*) => {};
}

#[cfg(feature = "klog-itm")]
Expand Down Expand Up @@ -126,18 +126,18 @@ macro_rules! klog {

macro_rules! uassert {
($cond : expr) => {
if ! $cond {
if !$cond {
panic!("Assertion failed!");
}
}
};
}

macro_rules! uassert_eq {
($cond1 : expr, $cond2 : expr) => {
if ! ($cond1 == $cond2) {
if !($cond1 == $cond2) {
panic!("Assertion failed!");
}
}
};
}

/// On ARMvx-M we use a global to record the task table position and extent.
Expand Down Expand Up @@ -448,15 +448,15 @@ pub fn apply_memory_protection(task: &task::Task) {
// Outer/inner non-cacheable, outer shared.
(0b01000100, 0b10)
} else {
let rw =
u32::from(ratts.contains(app::RegionAttributes::READ)) << 1
let rw = u32::from(ratts.contains(app::RegionAttributes::READ))
<< 1
smklein marked this conversation as resolved.
Show resolved Hide resolved
| u32::from(ratts.contains(app::RegionAttributes::WRITE));
// write-back transient, not shared
(0b0100_0100 | rw | rw << 4, 0b00)
};

// RLAR = our upper bound
let rlar = region.base + region.size
let rlar = (region.base + region.size)
| (i as u32) << 1 // AttrIndx
| (1 << 0); // enable

Expand All @@ -472,11 +472,11 @@ pub fn apply_memory_protection(task: &task::Task) {
// MAIR
if rnr < 4 {
let mut mair0 = (0xe000_edc0 as *const u32).read_volatile();
mair0 = mair0 | (mair as u32) << (rnr * 8);
mair0 |= (mair as u32) << (rnr * 8);
core::ptr::write_volatile(0xe000_edc0 as *mut u32, mair0);
} else {
let mut mair1 = (0xe000_edc4 as *const u32).read_volatile();
mair1 = mair1 | (mair as u32) << ((rnr - 4) * 8);
mair1 |= (mair as u32) << ((rnr - 4) * 8);
core::ptr::write_volatile(0xe000_edc4 as *mut u32, mair1);
}
// RBAR
Expand Down Expand Up @@ -680,7 +680,9 @@ pub unsafe extern "C" fn SVCall() {
///
/// You can use this safely at kernel entry points, exactly once, to create a
/// reference to the task table.
pub unsafe fn with_task_table<R>(body: impl FnOnce(&mut [task::Task]) -> R) -> R{
pub unsafe fn with_task_table<R>(
body: impl FnOnce(&mut [task::Task]) -> R,
) -> R {
let tasks = core::slice::from_raw_parts_mut(
TASK_TABLE_BASE.expect("kernel not started").as_mut(),
TASK_TABLE_SIZE,
Expand All @@ -694,7 +696,7 @@ pub unsafe fn with_task_table<R>(body: impl FnOnce(&mut [task::Task]) -> R) -> R
///
/// Because the lifetime of the reference passed into `body` is anonymous, the
/// reference can't easily be stored, which is deliberate.
pub fn with_irq_table<R>(body: impl FnOnce(&[abi::Interrupt]) -> R) -> R{
pub fn with_irq_table<R>(body: impl FnOnce(&[abi::Interrupt]) -> R) -> R {
// Safety: as long as a legit pointer was stored in IRQ_TABLE_BASE, or no
// pointer has been stored, we can do this safely.
let table = unsafe {
Expand Down Expand Up @@ -747,6 +749,7 @@ fn safe_sys_tick_handler(ticks: &mut u64, tasks: &mut [task::Task]) {
// accidentally do it again.
*ticks += 1;
let now = Timestamp::from(*ticks);
#[allow(clippy::drop_ref)]
smklein marked this conversation as resolved.
Show resolved Hide resolved
drop(ticks);

// Process any timers.
Expand All @@ -770,7 +773,8 @@ fn pend_context_switch_from_isr() {
#[naked]
#[no_mangle]
pub unsafe extern "C" fn PendSV() {
asm!("
asm!(
"
@ store volatile state.
@ first, get a pointer to the current task.
movw r0, #:lower16:CURRENT_TASK_PTR
Expand Down Expand Up @@ -847,7 +851,6 @@ pub unsafe extern "C" fn DefaultHandler() {
// 13 is currently reserved
// 14=PendSV is handled above by its own handler
// 15=SysTick is handled above by its own handler

x if x > 16 => {
// Hardware interrupt
let irq_num = exception_num - 16;
Expand Down
4 changes: 2 additions & 2 deletions kern/src/kipc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ pub fn handle_kernel_message(
2 => restart_task(tasks, caller, maybe_message?),
_ => {
// Task has sent an unknown message to the kernel. That's bad.
return Err(UserError::Unrecoverable(FaultInfo::SyscallUsage(
Err(UserError::Unrecoverable(FaultInfo::SyscallUsage(
UsageError::BadKernelMessage,
)));
)))
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions kern/src/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,20 @@ pub unsafe fn start_kernel(
for &region_idx in &task.regions {
uassert!(region_idx < app_header.region_count as u8);
let region = &regions[region_idx as usize];
if task.entry_point.wrapping_sub(region.base) < region.size {
if region.attributes.contains(app::RegionAttributes::EXECUTE) {
entry_pt_found = true;
}
if task.entry_point.wrapping_sub(region.base) < region.size
&& region.attributes.contains(app::RegionAttributes::EXECUTE)
{
entry_pt_found = true;
}
// Note that stack pointer is compared using <=, because it's okay
// to have it point just off the end as the stack is initially
// empty.
if task.initial_stack.wrapping_sub(region.base) <= region.size {
if region.attributes.contains(
if task.initial_stack.wrapping_sub(region.base) <= region.size
&& region.attributes.contains(
app::RegionAttributes::READ | app::RegionAttributes::WRITE,
) {
stack_ptr_found = true;
}
)
{
stack_ptr_found = true;
}
}

Expand Down
29 changes: 12 additions & 17 deletions kern/src/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fn safe_syscall_entry(nr: u32, current: usize, tasks: &mut [Task]) -> NextTask {
Ok(Sysnum::BorrowWrite) => borrow_write(tasks, current),
Ok(Sysnum::BorrowInfo) => borrow_info(tasks, current),
Ok(Sysnum::IrqControl) => irq_control(tasks, current),
Ok(Sysnum::Panic) => explicit_panic(tasks, current),
Ok(Sysnum::Panic) => Ok(explicit_panic(tasks, current)),
Ok(Sysnum::GetTimer) => Ok(get_timer(&mut tasks[current], arch::now())),
Err(_) => {
// Bogus syscall number! That's a fault.
Expand Down Expand Up @@ -156,7 +156,7 @@ fn send(tasks: &mut [Task], caller: usize) -> Result<NextTask, UserError> {
tasks[caller].set_healthy_state(SchedState::InSend(callee_id));
// We may not know what task to run next, but we're pretty sure it isn't the
// caller.
return Ok(NextTask::Other.combine(next_task));
Ok(NextTask::Other.combine(next_task))
}

/// Implementation of the RECV IPC primitive.
Expand Down Expand Up @@ -364,7 +364,7 @@ fn reply(tasks: &mut [Task], caller: usize) -> Result<NextTask, FaultInfo> {
// KEY ASSUMPTION: sends go from less important tasks to more important
// tasks. As a result, Reply doesn't have scheduling implications unless
// the task using it faults.
return Ok(NextTask::Same);
Ok(NextTask::Same)
}

/// Implementation of the `SET_TIMER` syscall.
Expand Down Expand Up @@ -431,12 +431,12 @@ fn borrow_read(
tasks[caller]
.save_mut()
.set_borrow_response_and_length(0, n);
return Ok(NextTask::Same);
Ok(NextTask::Same)
}
Err(interact) => {
let wake_hint = interact.apply_to_src(tasks, lender)?;
// Copy failed but not our side, report defecting lender.
return Err(UserError::Recoverable(abi::DEFECT, wake_hint));
Err(UserError::Recoverable(abi::DEFECT, wake_hint))
}
}
}
Expand Down Expand Up @@ -477,12 +477,12 @@ fn borrow_write(
tasks[caller]
.save_mut()
.set_borrow_response_and_length(0, n);
return Ok(NextTask::Same);
Ok(NextTask::Same)
}
Err(interact) => {
let wake_hint = interact.apply_to_dst(tasks, lender)?;
// Copy failed but not our side, report defecting lender.
return Err(UserError::Recoverable(abi::DEFECT, wake_hint));
Err(UserError::Recoverable(abi::DEFECT, wake_hint))
}
}
}
Expand All @@ -503,7 +503,7 @@ fn borrow_info(
tasks[caller]
.save_mut()
.set_borrow_info(lease.attributes.bits(), lease.length as usize);
return Ok(NextTask::Same);
Ok(NextTask::Same)
}

fn borrow_lease(
Expand Down Expand Up @@ -564,12 +564,10 @@ fn borrow_lease(
if offset <= lease.length as usize {
lease.base_address += offset as u32;
lease.length -= offset as u32;
Ok(lease)
} else {
return Err(
FaultInfo::SyscallUsage(UsageError::OffsetOutOfRange).into()
);
Err(FaultInfo::SyscallUsage(UsageError::OffsetOutOfRange).into())
}
Ok(lease)
} else {
// Borrower provided an invalid lease number. Borrower was told the
// number of leases on successful RECV and should respect that. (Note:
Expand Down Expand Up @@ -700,10 +698,7 @@ fn irq_control(
Ok(NextTask::Same)
}

fn explicit_panic(
tasks: &mut [Task],
caller: usize,
) -> Result<NextTask, UserError> {
fn explicit_panic(tasks: &mut [Task], caller: usize) -> NextTask {
// Make an attempt at printing the message.
let args = tasks[caller].save().as_panic_args();
let message = args.message();
Expand All @@ -724,5 +719,5 @@ fn explicit_panic(
}
}

Ok(task::force_fault(tasks, caller, FaultInfo::Panic))
task::force_fault(tasks, caller, FaultInfo::Panic)
}
2 changes: 1 addition & 1 deletion kern/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ pub fn check_task_id_against_table(
return Err(UserError::Recoverable(code, NextTask::Same));
}

return Ok(id.index());
Ok(id.index())
}

/// Selects a new task to run after `previous`. Tries to be fair, kind of.
Expand Down
2 changes: 1 addition & 1 deletion task-ping/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const UART: Task = Task::usart_driver;
fn nullread() {
unsafe {
// 0 is not in a region we can access; memory fault
(0 as *const u8).read_volatile();
core::ptr::null::<u8>().read_volatile();
}
}

Expand Down
2 changes: 1 addition & 1 deletion task-pong/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn main() -> ! {
loop {
match user_leds.led_toggle(current >> 1) {
Ok(_) => {
current = current + 1;
current += 1;
break;
}
Err(drv_user_leds_api::LedError::NoSuchLed) => {
Expand Down
Loading