From 34b8067a2d2ccb656fc6ed4fd8081f180a3833d8 Mon Sep 17 00:00:00 2001 From: Sam Leffler Date: Thu, 12 May 2022 01:51:28 +0000 Subject: [PATCH] ProcessManager:: clippy findings Change-Id: I9cc9044399cc8c74803484100743f156cfacf4d3 GitOrigin-RevId: e4a95dde642e5ea2660d2eb8f1a6fd8a18bcc443 --- .../kata-proc-component/src/run.rs | 135 ++++++++---------- .../kata-proc-interface/src/bundle_image.rs | 12 +- .../kata-proc-interface/src/lib.rs | 5 +- .../kata-proc-manager/src/proc_manager/mod.rs | 13 +- .../src/sel4bundle/arch/riscv32.rs | 1 + .../src/sel4bundle/feature/mcs.rs | 1 + .../src/sel4bundle/feature/spill_tcb_args.rs | 3 +- .../kata-proc-manager/src/sel4bundle/mod.rs | 10 +- 8 files changed, 82 insertions(+), 98 deletions(-) diff --git a/apps/system/components/ProcessManager/kata-proc-component/src/run.rs b/apps/system/components/ProcessManager/kata-proc-component/src/run.rs index 55e274a..75df2d1 100644 --- a/apps/system/components/ProcessManager/kata-proc-component/src/run.rs +++ b/apps/system/components/ProcessManager/kata-proc-component/src/run.rs @@ -2,6 +2,7 @@ // Code here binds the camkes component to the rust code. #![no_std] +#![allow(clippy::missing_safety_doc)] use core::slice; use cstr_core::CStr; @@ -13,7 +14,6 @@ use kata_os_common::sel4_sys; use kata_proc_interface::*; use kata_proc_manager::KATA_PROC; use log::{info, trace}; -use postcard; use sel4_sys::seL4_CNode_Delete; use sel4_sys::seL4_CPtr; @@ -41,43 +41,34 @@ extern "C" { static mut PKG_MGMT_RECV_SLOT: seL4_CPtr = 0; #[no_mangle] -pub extern "C" fn pre_init() { +pub unsafe extern "C" fn pre_init() { static KATA_LOGGER: KataLogger = KataLogger; log::set_logger(&KATA_LOGGER).unwrap(); // NB: set to max; the LoggerInterface will filter log::set_max_level(log::LevelFilter::Trace); static mut HEAP_MEMORY: [u8; 16 * 1024] = [0; 16 * 1024]; - unsafe { - allocator::ALLOCATOR.init(HEAP_MEMORY.as_mut_ptr() as usize, HEAP_MEMORY.len()); - trace!( - "setup heap: start_addr {:p} size {}", - HEAP_MEMORY.as_ptr(), - HEAP_MEMORY.len() - ); - } + allocator::ALLOCATOR.init(HEAP_MEMORY.as_mut_ptr() as usize, HEAP_MEMORY.len()); + trace!( + "setup heap: start_addr {:p} size {}", + HEAP_MEMORY.as_ptr(), + HEAP_MEMORY.len() + ); // Complete KATA_PROC setup. This is as early as we can do it given that // it needs the GlobalAllocator. - unsafe { - KATA_PROC.init(); - trace!( - "ProcessManager has capacity for {} bundles", - KATA_PROC.capacity() - ); - } + KATA_PROC.init(); + trace!("ProcessManager has capacity for {} bundles", KATA_PROC.capacity()); - unsafe { - KATA_CSPACE_SLOTS.init( - /*first_slot=*/ SELF_CNODE_FIRST_SLOT, - /*size=*/ SELF_CNODE_LAST_SLOT - SELF_CNODE_FIRST_SLOT - ); - trace!("setup cspace slots: first slot {} free {}", - KATA_CSPACE_SLOTS.base_slot(), - KATA_CSPACE_SLOTS.free_slots()); + KATA_CSPACE_SLOTS.init( + /*first_slot=*/ SELF_CNODE_FIRST_SLOT, + /*size=*/ SELF_CNODE_LAST_SLOT - SELF_CNODE_FIRST_SLOT + ); + trace!("setup cspace slots: first slot {} free {}", + KATA_CSPACE_SLOTS.base_slot(), + KATA_CSPACE_SLOTS.free_slots()); - PKG_MGMT_RECV_SLOT = KATA_CSPACE_SLOTS.alloc(1).unwrap(); - } + PKG_MGMT_RECV_SLOT = KATA_CSPACE_SLOTS.alloc(1).unwrap(); } fn debug_check_empty(tag: &str, path: &seL4_Path) { @@ -94,17 +85,15 @@ fn init_recv_path(tag: &str, path: &seL4_Path) { } #[no_mangle] -pub extern "C" fn pkg_mgmt__init() { - unsafe { - // Point the receive path to the well-known slot for receiving - // CNode's from clients for pkg_mgmt requests. - // - // NB: this must be done here (rather than someplace like pre_init) - // so it's in the context of the PackageManagementInterface thread - // (so we write the correct ipc buffer). - init_recv_path("pkg_mgmt", - &(SELF_CNODE, PKG_MGMT_RECV_SLOT, seL4_WordBits)); - } +pub unsafe extern "C" fn pkg_mgmt__init() { + // Point the receive path to the well-known slot for receiving + // CNode's from clients for pkg_mgmt requests. + // + // NB: this must be done here (rather than someplace like pre_init) + // so it's in the context of the PackageManagementInterface thread + // (so we write the correct ipc buffer). + init_recv_path("pkg_mgmt", + &(SELF_CNODE, PKG_MGMT_RECV_SLOT, seL4_WordBits)); } // Clears any capability the specified path points to. @@ -115,36 +104,34 @@ fn clear_path(path: &seL4_Path) { // PackageManagerInterface glue stubs. #[no_mangle] -pub extern "C" fn pkg_mgmt_install( +pub unsafe extern "C" fn pkg_mgmt_install( c_request_len: u32, c_request: *const u8, c_raw_data: *mut RawBundleIdData, ) -> ProcessManagerError { - unsafe { - let recv_path = seL4_GetCapReceivePath(); - // NB: make sure noone clobbers the setup done in pkg_mgmt__init - assert_eq!(recv_path, (SELF_CNODE, PKG_MGMT_RECV_SLOT, seL4_WordBits)); + let recv_path = seL4_GetCapReceivePath(); + // NB: make sure noone clobbers the setup done in pkg_mgmt__init + assert_eq!(recv_path, (SELF_CNODE, PKG_MGMT_RECV_SLOT, seL4_WordBits)); - let request_slice = slice::from_raw_parts(c_request, c_request_len as usize); - let ret_status = match postcard::from_bytes::(request_slice) { - Ok(mut pkg_contents) => { - sel4_sys::debug_assert_slot_cnode!(recv_path.1, - "Expected cnode in slot {} but has cap type {:?}", - recv_path.1, sel4_sys::cap_identify(recv_path.1)); - pkg_contents.cnode = recv_path.1; - match KATA_PROC.install(&pkg_contents) { - Ok(bundle_id) => match postcard::to_slice(&bundle_id, &mut (*c_raw_data)[..]) { - Ok(_) => ProcessManagerError::Success, - Err(_) => ProcessManagerError::SerializeError, - }, - Err(e) => e, - } + let request_slice = slice::from_raw_parts(c_request, c_request_len as usize); + let ret_status = match postcard::from_bytes::(request_slice) { + Ok(mut pkg_contents) => { + sel4_sys::debug_assert_slot_cnode!(recv_path.1, + "Expected cnode in slot {} but has cap type {:?}", + recv_path.1, sel4_sys::cap_identify(recv_path.1)); + pkg_contents.cnode = recv_path.1; + match KATA_PROC.install(&pkg_contents) { + Ok(bundle_id) => match postcard::to_slice(&bundle_id, &mut (*c_raw_data)[..]) { + Ok(_) => ProcessManagerError::Success, + Err(_) => ProcessManagerError::SerializeError, + }, + Err(e) => e, } - Err(e) => e.into(), - }; - clear_path(&recv_path); - ret_status - } + } + Err(e) => e.into(), + }; + clear_path(&recv_path); + ret_status } fn check_pkg_mgmt_empty(tag: &str) -> seL4_Path { @@ -158,12 +145,12 @@ fn check_pkg_mgmt_empty(tag: &str) -> seL4_Path { } #[no_mangle] -pub extern "C" fn pkg_mgmt_uninstall( +pub unsafe extern "C" fn pkg_mgmt_uninstall( c_bundle_id: *const cstr_core::c_char ) -> ProcessManagerError { let recv_path = check_pkg_mgmt_empty("pkg_mgmt_uninstall"); - let ret_status = match unsafe { CStr::from_ptr(c_bundle_id).to_str() } { - Ok(bundle_id) => match unsafe { KATA_PROC.uninstall(bundle_id) } { + let ret_status = match CStr::from_ptr(c_bundle_id).to_str() { + Ok(bundle_id) => match KATA_PROC.uninstall(bundle_id) { Ok(_) => ProcessManagerError::Success, Err(e) => e, }, @@ -175,11 +162,11 @@ pub extern "C" fn pkg_mgmt_uninstall( // ProcessControlInterface glue stubs. #[no_mangle] -pub extern "C" fn proc_ctrl_start( +pub unsafe extern "C" fn proc_ctrl_start( c_bundle_id: *const cstr_core::c_char ) -> ProcessManagerError { - match unsafe { CStr::from_ptr(c_bundle_id).to_str() } { - Ok(bundle_id) => match unsafe { KATA_PROC.start(bundle_id) } { + match CStr::from_ptr(c_bundle_id).to_str() { + Ok(bundle_id) => match KATA_PROC.start(bundle_id) { Ok(_) => ProcessManagerError::Success, Err(e) => e, }, @@ -188,11 +175,11 @@ pub extern "C" fn proc_ctrl_start( } #[no_mangle] -pub extern "C" fn proc_ctrl_stop( +pub unsafe extern "C" fn proc_ctrl_stop( c_bundle_id: *const cstr_core::c_char ) -> ProcessManagerError { - match unsafe { CStr::from_ptr(c_bundle_id).to_str() } { - Ok(str) => match unsafe { KATA_PROC.stop(str) } { + match CStr::from_ptr(c_bundle_id).to_str() { + Ok(str) => match KATA_PROC.stop(str) { Ok(_) => ProcessManagerError::Success, Err(e) => e, }, @@ -201,15 +188,15 @@ pub extern "C" fn proc_ctrl_stop( } #[no_mangle] -pub extern "C" fn proc_ctrl_get_running_bundles( +pub unsafe extern "C" fn proc_ctrl_get_running_bundles( c_raw_data: *mut RawBundleIdData, ) -> ProcessManagerError { - match unsafe { KATA_PROC.get_running_bundles() } { + match KATA_PROC.get_running_bundles() { Ok(bundles) => { // Serialize the bundle_id's in the result buffer. If we // overflow the buffer, an error is returned and the // contents are undefined (postcard does not specify). - match unsafe { postcard::to_slice(&bundles, &mut (*c_raw_data)[..]) } { + match postcard::to_slice(&bundles, &mut (*c_raw_data)[..]) { Ok(_) => ProcessManagerError::Success, Err(_) => ProcessManagerError::DeserializeError, } diff --git a/apps/system/components/ProcessManager/kata-proc-interface/src/bundle_image.rs b/apps/system/components/ProcessManager/kata-proc-interface/src/bundle_image.rs index 3e18a44..1acbd5a 100644 --- a/apps/system/components/ProcessManager/kata-proc-interface/src/bundle_image.rs +++ b/apps/system/components/ProcessManager/kata-proc-interface/src/bundle_image.rs @@ -43,10 +43,8 @@ enum BundleImageError { BadSectionIO, } impl From for BundleImageError { - fn from(err: seL4_Error) -> BundleImageError { - match err { - _ => BundleImageError::CapMoveFailed, - } + fn from(_err: seL4_Error) -> BundleImageError { + BundleImageError::CapMoveFailed } } @@ -123,7 +121,7 @@ pub struct BundleImage<'a> { impl<'a> BundleImage<'a> { pub fn new(frames: &'a ObjDescBundle) -> Self { BundleImage { - frames: frames, + frames, cur_frame: None, last_frame: None, cur_pos: 0, @@ -276,7 +274,7 @@ impl<'a> io::Seek for BundleImage<'a> { impl<'a> io::Read for BundleImage<'a> { fn read(&mut self, buf: &mut [u8]) -> io::Result { let mut cursor = &mut *buf; - while cursor.len() > 0 { + while !cursor.is_empty() { let available_bytes = self.mapped_bytes - self.bytes_read; if available_bytes > 0 { // Fill from the current frame (as space permits). @@ -301,7 +299,7 @@ impl<'a> io::Read for BundleImage<'a> { self.unmap_current_frame().map_err(|_| io::Error)?; } } - if cursor.len() == 0 { break } + if cursor.is_empty() { break } // Map the next frame for read. self.map_next_frame().map_err(|_| io::Error)?; diff --git a/apps/system/components/ProcessManager/kata-proc-interface/src/lib.rs b/apps/system/components/ProcessManager/kata-proc-interface/src/lib.rs index b970a2d..7fc29b5 100644 --- a/apps/system/components/ProcessManager/kata-proc-interface/src/lib.rs +++ b/apps/system/components/ProcessManager/kata-proc-interface/src/lib.rs @@ -12,7 +12,6 @@ use kata_memory_interface::ObjDescBundle; use kata_memory_interface::RAW_OBJ_DESC_DATA_SIZE; use kata_os_common::sel4_sys; use kata_security_interface::SecurityRequestError; -use postcard; use serde::{Deserialize, Serialize}; use sel4_sys::seL4_SetCap; @@ -45,9 +44,9 @@ pub struct Bundle { pub app_memory_size: u32, } impl Bundle { - pub fn new(bundle_id: &String) -> Self { + pub fn new(bundle_id: &str) -> Self { Bundle { - app_id: bundle_id.clone(), + app_id: String::from(bundle_id), app_memory_size: 0u32, } } diff --git a/apps/system/components/ProcessManager/kata-proc-manager/src/proc_manager/mod.rs b/apps/system/components/ProcessManager/kata-proc-manager/src/proc_manager/mod.rs index 885d2c7..7b70566 100644 --- a/apps/system/components/ProcessManager/kata-proc-manager/src/proc_manager/mod.rs +++ b/apps/system/components/ProcessManager/kata-proc-manager/src/proc_manager/mod.rs @@ -99,15 +99,12 @@ impl PackageManagementInterface for ProcessManager { trace!("uninstall bundle_id {}", bundle_id); let bid = BundleId::from_str(bundle_id); - match self.bundles.get(&bid) { - Some(bundle) => { - trace!("uninstall state {:?}", bundle.state); - if bundle.state == BundleState::Running { - return Err(ProcessManagerError::BundleRunning); - } - let _ = self.bundles.remove(&bid); + if let Some(bundle) = self.bundles.get(&bid) { + trace!("uninstall state {:?}", bundle.state); + if bundle.state == BundleState::Running { + return Err(ProcessManagerError::BundleRunning); } - None => {} + let _ = self.bundles.remove(&bid); } // NB: the hashmap is ephemeral so always call through to the manager self.manager.uninstall(bundle_id) diff --git a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/arch/riscv32.rs b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/arch/riscv32.rs index be86fa6..85c8c62 100644 --- a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/arch/riscv32.rs +++ b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/arch/riscv32.rs @@ -29,6 +29,7 @@ pub fn get_user_context(pc: seL4_Word, sp: seL4_Word, argv: &[seL4_Word]) t0: 0, t1: 0, t2: 0, t3: 0, t4: 0, t5: 0, t6: 0, tp: 0, }; + #[allow(clippy::len_zero)] unsafe { regs.pc = pc; regs.sp = sp; // NB: may be adjusted from self.tcb_sp diff --git a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/mcs.rs b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/mcs.rs index cc18753..e33a128 100644 --- a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/mcs.rs +++ b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/mcs.rs @@ -38,6 +38,7 @@ pub fn SchedControl_Configure( } } +#[allow(clippy::too_many_arguments)] pub fn TCB_Configure( sel4_tcb: seL4_Word, _sel4_fault_ep: seL4_CPtr, diff --git a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/spill_tcb_args.rs b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/spill_tcb_args.rs index eafd185..cb2c087 100644 --- a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/spill_tcb_args.rs +++ b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/feature/spill_tcb_args.rs @@ -78,9 +78,10 @@ impl seL4BundleImpl { "TCB {}'s initial arguments cause its stack to cross a page boundary", self.tcb_name ); + let byte_offset = sp % copy_region.size(); unsafe { ptr::write( - &mut copy_region.as_word_mut()[(sp % copy_region.size()) / size_of::()], + &mut copy_region.as_word_mut()[byte_offset / size_of::()], argv[i], ) }; diff --git a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/mod.rs b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/mod.rs index d6b089b..8f6e0d4 100644 --- a/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/mod.rs +++ b/apps/system/components/ProcessManager/kata-proc-manager/src/sel4bundle/mod.rs @@ -107,7 +107,7 @@ const_assert!(seL4_WordBits == 32 || seL4_WordBits == 64); // for an explanation of how this is used). // TODO(sleffler): move to sel4-sys because it exposes internals fn make_guard(guard_bits: seL4_Word, guard_size: seL4_Word) -> seL4_Word { - ((guard_bits << 0) & ((1 << 18) -1)) | ((guard_size << 18) | ((1 << 4) - 1)) + ((guard_bits) & ((1 << 18) -1)) | ((guard_size << 18) | ((1 << 4) - 1)) } fn roundup(a: usize, b: usize) -> usize { @@ -151,7 +151,7 @@ impl CopyRegion { pub fn size(&self) -> usize { PAGE_SIZE } // Returns a mutable [u8] ref to the mapped region. - pub fn as_mut(&self) -> &mut [u8] { + pub fn as_mut(&mut self) -> &mut [u8] { assert!(self.cur_frame.is_some()); unsafe { core::slice::from_raw_parts_mut( @@ -161,7 +161,7 @@ impl CopyRegion { } // Returns a mutable [seL4_Word] ref to the mapped region. - pub fn as_word_mut(&self) -> &mut [seL4_Word] { + pub fn as_word_mut(&mut self) -> &mut [seL4_Word] { assert!(self.cur_frame.is_some()); unsafe { core::slice::from_raw_parts_mut( @@ -353,8 +353,8 @@ impl seL4BundleImpl { Ok(seL4BundleImpl { bundle_frames: bundle_frames.clone(), - dynamic_objs: dynamic_objs, - cspace_root: cspace_root, + dynamic_objs, + cspace_root, cap_tcb: CSpaceSlot::new(), // Top-level dup for suspend/resume first_page: first_vaddr / PAGE_SIZE,