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

Apply some of the cargo clippy suggestions #649

Merged
merged 1 commit into from
Sep 8, 2024
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
6 changes: 4 additions & 2 deletions src/capture.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::from_over_into)]

#[cfg(feature = "serde")]
use crate::resolve;
use crate::PrintFmt;
Expand Down Expand Up @@ -351,7 +353,7 @@ impl From<crate::Frame> for BacktraceFrame {
}
}

// we don't want implementing `impl From<Backtrace> for Vec<BacktraceFrame>` on purpose,
// we don't want to implement `impl From<Backtrace> for Vec<BacktraceFrame>` on purpose,
// because "... additional directions for Vec<T> can weaken type inference ..."
// more information on https://github.com/rust-lang/backtrace-rs/pull/526
impl Into<Vec<BacktraceFrame>> for Backtrace {
Expand Down Expand Up @@ -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`
Expand Down
32 changes: 15 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,24 @@
//! Next:
//!
//! ```
//! fn main() {
//! # // Unsafe here so test passes on no_std.
//! # #[cfg(feature = "std")] {
workingjubilee marked this conversation as resolved.
Show resolved Hide resolved
//! 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
//! });
//! # }
//! ```
//!
Expand Down
2 changes: 1 addition & 1 deletion src/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ impl BacktraceFrameFmt<'_, '_, '_> {
write!(self.fmt.fmt, ":{colno}")?;
}

write!(self.fmt.fmt, "\n")?;
writeln!(self.fmt.fmt)?;
nyurik marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion src/print/fuchsia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl<'a> Iterator for NoteIter<'a> {
type Item = Note<'a>;
fn next(&mut self) -> Option<Self::Item> {
// 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
Expand Down
3 changes: 1 addition & 2 deletions src/symbolize/gimli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -335,7 +334,7 @@ impl Cache {
// never happen, and symbolicating backtraces would be ssssllllooooowwww.
static mut MAPPINGS_CACHE: Option<Cache> = 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)> {
Expand Down
8 changes: 5 additions & 3 deletions src/symbolize/gimli/elf.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -21,7 +23,7 @@ impl Mapping {
pub fn new(path: &Path) -> Option<Mapping> {
let map = super::mmap(path)?;
Mapping::mk_or_other(map, |map, stash| {
let object = Object::parse(&map)?;
let object = Object::parse(map)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mk_or_other is such a weird function...


// Try to locate an external debug file using the build ID.
if let Some(path_debug) = object.build_id().and_then(locate_build_id) {
Expand All @@ -47,7 +49,7 @@ impl Mapping {
fn new_debug(original_path: &Path, path: PathBuf, crc: Option<u32>) -> Option<Mapping> {
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
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions src/symbolize/gimli/libs_dl_iterate_phdr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(super) fn native_libraries() -> Vec<Library> {
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 {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/symbolize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub fn resolve<F: FnMut(&Symbol)>(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
Expand Down
6 changes: 3 additions & 3 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
workingjubilee marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
4 changes: 4 additions & 0 deletions tests/skip_inner_frames.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}");
// FIXME: need more stacktrace content tests
assert!(trace.ends_with("\n"));
}
16 changes: 8 additions & 8 deletions tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
}
}
Expand Down
Loading