From 6fba6025b06753a5ac61604f55a48839e84753e7 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Fri, 2 Aug 2024 18:07:13 -0400 Subject: [PATCH] Apply some of the `cargo clippy` suggestions --- src/capture.rs | 6 ++-- src/lib.rs | 32 ++++++++++----------- src/print.rs | 2 +- src/print/fuchsia.rs | 2 +- src/symbolize/gimli.rs | 21 +++++++------- src/symbolize/gimli/elf.rs | 8 ++++-- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 6 ++-- src/symbolize/mod.rs | 2 +- src/types.rs | 6 ++-- tests/skip_inner_frames.rs | 4 +++ tests/smoke.rs | 16 +++++------ 11 files changed, 56 insertions(+), 49 deletions(-) diff --git a/src/capture.rs b/src/capture.rs index f2154995a..fef2964da 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -1,3 +1,5 @@ +#![allow(clippy::from_over_into)] + #[cfg(feature = "serde")] use crate::resolve; use crate::PrintFmt; @@ -351,7 +353,7 @@ impl From for BacktraceFrame { } } -// we don't want implementing `impl From for Vec` on purpose, +// we don't want to implement `impl From for Vec` on purpose, // because "... additional directions for Vec can weaken type inference ..." // more information on https://github.com/rust-lang/backtrace-rs/pull/526 impl Into> for Backtrace { @@ -452,7 +454,7 @@ impl BacktraceSymbol { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn filename(&self) -> Option<&Path> { - self.filename.as_ref().map(|p| &**p) + self.filename.as_deref() } /// Same as `Symbol::lineno` diff --git a/src/lib.rs b/src/lib.rs index 0b9bb56ae..ab5e64c9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,26 +18,24 @@ //! Next: //! //! ``` -//! fn main() { //! # // Unsafe here so test passes on no_std. //! # #[cfg(feature = "std")] { -//! backtrace::trace(|frame| { -//! let ip = frame.ip(); -//! let symbol_address = frame.symbol_address(); -//! -//! // Resolve this instruction pointer to a symbol name -//! backtrace::resolve_frame(frame, |symbol| { -//! if let Some(name) = symbol.name() { -//! // ... -//! } -//! if let Some(filename) = symbol.filename() { -//! // ... -//! } -//! }); -//! -//! true // keep going to the next frame +//! backtrace::trace(|frame| { +//! let ip = frame.ip(); +//! let symbol_address = frame.symbol_address(); +//! +//! // Resolve this instruction pointer to a symbol name +//! backtrace::resolve_frame(frame, |symbol| { +//! if let Some(name) = symbol.name() { +//! // ... +//! } +//! if let Some(filename) = symbol.filename() { +//! // ... +//! } //! }); -//! } +//! +//! true // keep going to the next frame +//! }); //! # } //! ``` //! diff --git a/src/print.rs b/src/print.rs index 6e38a0f13..a89535de1 100644 --- a/src/print.rs +++ b/src/print.rs @@ -289,7 +289,7 @@ impl BacktraceFrameFmt<'_, '_, '_> { write!(self.fmt.fmt, ":{colno}")?; } - write!(self.fmt.fmt, "\n")?; + writeln!(self.fmt.fmt)?; Ok(()) } diff --git a/src/print/fuchsia.rs b/src/print/fuchsia.rs index 5a5aae530..5a571147e 100644 --- a/src/print/fuchsia.rs +++ b/src/print/fuchsia.rs @@ -196,7 +196,7 @@ impl<'a> Iterator for NoteIter<'a> { type Item = Note<'a>; fn next(&mut self) -> Option { // Check if we've reached the end. - if self.base.len() == 0 || self.error { + if self.base.is_empty() || self.error { return None; } // We transmute out an nhdr but we carefully consider the resulting diff --git a/src/symbolize/gimli.rs b/src/symbolize/gimli.rs index 7465f755c..247e142eb 100644 --- a/src/symbolize/gimli.rs +++ b/src/symbolize/gimli.rs @@ -12,7 +12,6 @@ use super::SymbolName; use addr2line::gimli; use core::convert::TryInto; use core::mem; -use core::u32; use libc::c_void; use mystd::ffi::OsString; use mystd::fs::File; @@ -100,7 +99,7 @@ impl Mapping { // only borrow `map` and `stash` and we're preserving them below. cx: unsafe { core::mem::transmute::, Context<'static>>(cx) }, _map: data, - stash: stash, + stash, }) } } @@ -119,16 +118,18 @@ impl<'data> Context<'data> { dwp: Option>, ) -> Option> { let mut sections = gimli::Dwarf::load(|id| -> Result<_, ()> { - if cfg!(not(target_os = "aix")) { + #[cfg(not(target_os = "aix"))] + { let data = object.section(stash, id.name()).unwrap_or(&[]); Ok(EndianSlice::new(data, Endian)) + } + + #[cfg(target_os = "aix")] + if let Some(name) = id.xcoff_name() { + let data = object.section(stash, name).unwrap_or(&[]); + Ok(EndianSlice::new(data, Endian)) } else { - if let Some(name) = id.xcoff_name() { - let data = object.section(stash, name).unwrap_or(&[]); - Ok(EndianSlice::new(data, Endian)) - } else { - Ok(EndianSlice::new(&[], Endian)) - } + Ok(EndianSlice::new(&[], Endian)) } }) .ok()?; @@ -336,7 +337,7 @@ impl Cache { // never happen, and symbolicating backtraces would be ssssllllooooowwww. static mut MAPPINGS_CACHE: Option = None; - f(MAPPINGS_CACHE.get_or_insert_with(|| Cache::new())) + f(MAPPINGS_CACHE.get_or_insert_with(Cache::new)) } fn avma_to_svma(&self, addr: *const u8) -> Option<(usize, *const u8)> { diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs index 906a30054..5771f93f6 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -1,3 +1,5 @@ +#![allow(clippy::useless_conversion)] + use super::mystd::ffi::{OsStr, OsString}; use super::mystd::fs; use super::mystd::os::unix::ffi::{OsStrExt, OsStringExt}; @@ -21,7 +23,7 @@ impl Mapping { pub fn new(path: &Path) -> Option { let map = super::mmap(path)?; Mapping::mk_or_other(map, |map, stash| { - let object = Object::parse(&map)?; + let object = Object::parse(map)?; // Try to locate an external debug file using the build ID. if let Some(path_debug) = object.build_id().and_then(locate_build_id) { @@ -47,7 +49,7 @@ impl Mapping { fn new_debug(original_path: &Path, path: PathBuf, crc: Option) -> Option { let map = super::mmap(&path)?; Mapping::mk(map, |map, stash| { - let object = Object::parse(&map)?; + let object = Object::parse(map)?; if let Some(_crc) = crc { // TODO: check crc @@ -224,7 +226,7 @@ impl<'a> Object<'a> { .map(|(_index, section)| section) } - pub fn search_symtab<'b>(&'b self, addr: u64) -> Option<&'b [u8]> { + pub fn search_symtab(&self, addr: u64) -> Option<&[u8]> { // Same sort of binary search as Windows above let i = match self.syms.binary_search_by_key(&addr, |sym| sym.address) { Ok(i) => i, diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index c8558e626..566cf9018 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -14,7 +14,7 @@ pub(super) fn native_libraries() -> Vec { unsafe { libc::dl_iterate_phdr(Some(callback), core::ptr::addr_of_mut!(ret).cast()); } - return ret; + ret } fn infer_current_exe(base_addr: usize) -> OsString { @@ -65,8 +65,8 @@ unsafe extern "C" fn callback( segments: headers .iter() .map(|header| LibrarySegment { - len: (*header).p_memsz as usize, - stated_virtual_memory_address: (*header).p_vaddr as usize, + len: header.p_memsz as usize, + stated_virtual_memory_address: header.p_vaddr as usize, }) .collect(), bias: info.dlpi_addr as usize, diff --git a/src/symbolize/mod.rs b/src/symbolize/mod.rs index 64542bdd0..9a5d873c2 100644 --- a/src/symbolize/mod.rs +++ b/src/symbolize/mod.rs @@ -63,7 +63,7 @@ pub fn resolve(addr: *mut c_void, cb: F) { unsafe { resolve_unsynchronized(addr, cb) } } -/// Resolve a previously capture frame to a symbol, passing the symbol to the +/// Resolve a previously captured frame to a symbol, passing the symbol to the /// specified closure. /// /// This function performs the same function as `resolve` except that it takes a diff --git a/src/types.rs b/src/types.rs index c7e551059..c419247a6 100644 --- a/src/types.rs +++ b/src/types.rs @@ -33,9 +33,9 @@ impl<'a> BytesOrWideString<'a> { pub fn to_str_lossy(&self) -> Cow<'a, str> { use self::BytesOrWideString::*; - match self { - &Bytes(slice) => String::from_utf8_lossy(slice), - &Wide(wide) => Cow::Owned(String::from_utf16_lossy(wide)), + match *self { + Bytes(slice) => String::from_utf8_lossy(slice), + Wide(wide) => Cow::Owned(String::from_utf16_lossy(wide)), } } diff --git a/tests/skip_inner_frames.rs b/tests/skip_inner_frames.rs index bce8d7fa5..61725a785 100644 --- a/tests/skip_inner_frames.rs +++ b/tests/skip_inner_frames.rs @@ -44,4 +44,8 @@ fn backtrace_new_should_start_with_call_site_trace() { let this_ip = backtrace_new_should_start_with_call_site_trace as *mut c_void; let frame_ip = b.frames().first().unwrap().symbol_address(); assert_eq!(this_ip, frame_ip); + + let trace = format!("{b:?}"); + assert!(trace.starts_with(" 0: skip_inner_frames::backtrace_new_should_start_with_call_site_trace\n at ")); + assert!(trace.ends_with("\n")); } diff --git a/tests/smoke.rs b/tests/smoke.rs index cbb93347f..fd5684f97 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -190,20 +190,20 @@ fn smoke_test_frames() { ); } if expected_line != 0 { - assert!( - line == expected_line, - "bad line number on frame for `{expected_name}`: {line} != {expected_line}" - ); + assert_eq!( + line, + expected_line, + "bad line number on frame for `{expected_name}`: {line} != {expected_line}"); } // dbghelp on MSVC doesn't support column numbers if !cfg!(target_env = "msvc") { let col = col.expect("didn't find a column number"); if expected_col != 0 { - assert!( - col == expected_col, - "bad column number on frame for `{expected_name}`: {col} != {expected_col}", - ); + assert_eq!( + col, + expected_col, + "bad column number on frame for `{expected_name}`: {col} != {expected_col}"); } } }