From ad7f5293db22e23d8d50077aa99f5d7a904092fc Mon Sep 17 00:00:00 2001 From: Michael Krasnitski Date: Fri, 6 Jan 2023 14:13:55 -0500 Subject: Rust API: Additional Cleanup --- rust/src/callingconvention.rs | 2 +- rust/src/custombinaryview.rs | 4 +-- rust/src/debuginfo.rs | 61 ++++++++++++++++++---------------- rust/src/demangle.rs | 2 +- rust/src/disassembly.rs | 2 +- rust/src/downloadprovider.rs | 5 ++- rust/src/flowgraph.rs | 9 +---- rust/src/function.rs | 5 +-- rust/src/lib.rs | 9 +++-- rust/src/llil/lifting.rs | 2 +- rust/src/metadata.rs | 4 +-- rust/src/rc.rs | 11 +++++-- rust/src/section.rs | 6 ++-- rust/src/symbol.rs | 18 ++++------ rust/src/types.rs | 77 ++++++++++++++++++------------------------- 15 files changed, 99 insertions(+), 118 deletions(-) (limited to 'rust/src') diff --git a/rust/src/callingconvention.rs b/rust/src/callingconvention.rs index 99b6c654..52d1407c 100644 --- a/rust/src/callingconvention.rs +++ b/rust/src/callingconvention.rs @@ -449,7 +449,7 @@ impl CallingConvention { pub fn variables_for_parameters( &self, params: &[FunctionParameter], - int_arg_registers: Option>, + int_arg_registers: Option<&[A::Register]>, ) -> Vec { let mut bn_params: Vec = vec![]; let mut name_strings = vec![]; diff --git a/rust/src/custombinaryview.rs b/rust/src/custombinaryview.rs index aa4e9e2f..90673fb7 100644 --- a/rust/src/custombinaryview.rs +++ b/rust/src/custombinaryview.rs @@ -441,10 +441,10 @@ impl<'a, T: CustomBinaryViewType> CustomViewBuilder<'a, T> { if context.initialized { mem::forget(context.args); // already consumed - // mem::drop(context.view); // cb_init was called + // mem::drop(context.view); // cb_init was called } else { mem::drop(context.args); // never consumed - // mem::forget(context.view); // cb_init was not called, is uninit + // mem::forget(context.view); // cb_init was not called, is uninit if context.raw_handle.is_null() { // being called here is essentially a guarantee that BNCreateBinaryViewOfType diff --git a/rust/src/debuginfo.rs b/rust/src/debuginfo.rs index 37335255..3ec0691c 100644 --- a/rust/src/debuginfo.rs +++ b/rust/src/debuginfo.rs @@ -575,15 +575,16 @@ impl DebugInfo { let names_and_types: &[*mut BNNameAndType] = unsafe { slice::from_raw_parts(raw_names_and_types as *mut _, count) }; - let mut result = Vec::with_capacity(count); - for name_and_type in names_and_types.iter().take(count) { - unsafe { - result.push(( - raw_to_string((**name_and_type).name).unwrap(), - Type::ref_from_raw(BNNewTypeReference((**name_and_type).type_)), - )) - }; - } + let result = names_and_types + .iter() + .take(count) + .map(|&name_and_type| unsafe { + ( + raw_to_string((*name_and_type).name).unwrap(), + Type::ref_from_raw(BNNewTypeReference((*name_and_type).type_)), + ) + }) + .collect(); unsafe { BNFreeNameAndTypeList(raw_names_and_types, count) }; result @@ -604,16 +605,17 @@ impl DebugInfo { let variables_and_names: &[*mut BNDataVariableAndName] = unsafe { slice::from_raw_parts(raw_variables_and_names as *mut _, count) }; - let mut result = Vec::with_capacity(count); - for variable_and_name in variables_and_names.iter().take(count) { - unsafe { - result.push(( - raw_to_string((**variable_and_name).name).unwrap(), - (**variable_and_name).address, - Type::ref_from_raw(BNNewTypeReference((**variable_and_name).type_)), - )) - }; - } + let result = variables_and_names + .iter() + .take(count) + .map(|&variable_and_name| unsafe { + ( + raw_to_string((*variable_and_name).name).unwrap(), + (*variable_and_name).address, + Type::ref_from_raw(BNNewTypeReference((*variable_and_name).type_)), + ) + }) + .collect(); unsafe { BNFreeDataVariablesAndName(raw_variables_and_names, count) }; result @@ -628,16 +630,17 @@ impl DebugInfo { let variables_and_names: &[*mut BNDataVariableAndNameAndDebugParser] = unsafe { slice::from_raw_parts(raw_variables_and_names as *mut _, count) }; - let mut result = Vec::with_capacity(count); - for variable_and_name in variables_and_names.iter().take(count) { - unsafe { - result.push(( - raw_to_string((**variable_and_name).parser).unwrap(), - raw_to_string((**variable_and_name).name).unwrap(), - Type::ref_from_raw(BNNewTypeReference((**variable_and_name).type_)), - )) - }; - } + let result = variables_and_names + .iter() + .take(count) + .map(|&variable_and_name| unsafe { + ( + raw_to_string((*variable_and_name).parser).unwrap(), + raw_to_string((*variable_and_name).name).unwrap(), + Type::ref_from_raw(BNNewTypeReference((*variable_and_name).type_)), + ) + }) + .collect(); unsafe { BNFreeDataVariableAndNameAndDebugParserList(raw_variables_and_names, count) }; result diff --git a/rust/src/demangle.rs b/rust/src/demangle.rs index e9948cb6..0b01a231 100644 --- a/rust/src/demangle.rs +++ b/rust/src/demangle.rs @@ -15,8 +15,8 @@ //! Interfaces for demangling and simplifying mangled names in binaries. use binaryninjacore_sys::*; -use std::{ffi::CStr, result}; use std::os::raw::c_char; +use std::{ffi::CStr, result}; use crate::architecture::CoreArchitecture; use crate::string::{BnStrCompatible, BnString}; diff --git a/rust/src/disassembly.rs b/rust/src/disassembly.rs index ff33e3dd..950335a3 100644 --- a/rust/src/disassembly.rs +++ b/rust/src/disassembly.rs @@ -289,7 +289,7 @@ impl DisassemblyTextLine { impl std::fmt::Display for DisassemblyTextLine { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { for token in self.tokens() { - write!(f, "{}", token.text())? + write!(f, "{}", token.text())?; } Ok(()) diff --git a/rust/src/downloadprovider.rs b/rust/src/downloadprovider.rs index dfc8b5e2..7de8704c 100644 --- a/rust/src/downloadprovider.rs +++ b/rust/src/downloadprovider.rs @@ -38,9 +38,8 @@ impl DownloadProvider { Ok(unsafe { Array::new(list, count, ()) }) } - /// TODO : Clippy isn't happy...says we should `impl Default`....excessive error checking might be preventing us from doing so - #[allow(clippy::should_implement_trait)] - pub fn default() -> Result { + /// TODO : We may want to `impl Default`....excessive error checking might be preventing us from doing so + pub fn try_default() -> Result { let s = Settings::new(""); let dp_name = s.get_string("network.downloadProviderName", None, None); Self::get(dp_name).ok_or(()) diff --git a/rust/src/flowgraph.rs b/rust/src/flowgraph.rs index 92b098cd..a0e6caaa 100644 --- a/rust/src/flowgraph.rs +++ b/rust/src/flowgraph.rs @@ -89,14 +89,7 @@ impl<'a> FlowGraphNode<'a> { target: &'a FlowGraphNode, edge_style: &'a EdgeStyle, ) { - unsafe { - BNAddFlowGraphNodeOutgoingEdge( - self.handle, - type_, - target.handle, - edge_style.0, - ) - } + unsafe { BNAddFlowGraphNodeOutgoingEdge(self.handle, type_, target.handle, edge_style.0) } } } diff --git a/rust/src/function.rs b/rust/src/function.rs index f56f0d64..aaefe5cd 100644 --- a/rust/src/function.rs +++ b/rust/src/function.rs @@ -35,10 +35,7 @@ pub struct Location { impl From for Location { fn from(addr: u64) -> Self { - Location { - arch: None, - addr, - } + Location { arch: None, addr } } } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index db550b49..18f73401 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -12,12 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -// TODO : If I commit this I've fucked up +// TODO : These clippy-allow are bad and needs to be removed #![allow(clippy::missing_safety_doc)] -#![allow(clippy::type_complexity)] -#![allow(clippy::from_over_into)] -// TODO : This is bad and needs to be removed #![allow(clippy::result_unit_err)] +#![allow(clippy::type_complexity)] #![doc(html_no_source)] #![doc(html_favicon_url = "/favicon.ico")] #![doc(html_logo_url = "/logo.png")] @@ -348,7 +346,8 @@ pub fn open_view>(filename: F) -> Result Self { - Label::new() + Self::new() } } diff --git a/rust/src/metadata.rs b/rust/src/metadata.rs index b17ba41c..1ccb9ae4 100644 --- a/rust/src/metadata.rs +++ b/rust/src/metadata.rs @@ -511,9 +511,9 @@ impl From> for Ref { } } -impl PartialEq for Ref { +impl PartialEq for Metadata { fn eq(&self, other: &Self) -> bool { - unsafe { BNMetadataIsEqual(self.as_ref().handle, other.as_ref().handle) } + unsafe { BNMetadataIsEqual(self.handle, other.handle) } } } diff --git a/rust/src/rc.rs b/rust/src/rc.rs index 57deddc3..4ffecb3f 100644 --- a/rust/src/rc.rs +++ b/rust/src/rc.rs @@ -16,6 +16,7 @@ use std::borrow::Borrow; use std::fmt::{Debug, Display, Formatter}; +use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::mem; use std::ops::{Deref, DerefMut}; @@ -118,7 +119,13 @@ impl PartialEq for Ref { } } -impl Eq for Ref {} +impl Eq for Ref {} + +impl Hash for Ref { + fn hash(&self, state: &mut H) { + self.contents.hash(state); + } +} // Guard provides access to a core-allocated resource whose // reference is held indirectly (e.g. a core-allocated array @@ -147,7 +154,7 @@ impl<'a, T> Guard<'a, T> where T: RefCountable, { - #[allow(clippy::should_implement_trait)] // This _is_ out own (lite) version of that trait + #[allow(clippy::should_implement_trait)] // This _is_ out own (lite) version of that trait pub fn clone(&self) -> Ref { unsafe { ::inc_ref(&self.contents) } } diff --git a/rust/src/section.rs b/rust/src/section.rs index b73809c0..49642b4d 100644 --- a/rust/src/section.rs +++ b/rust/src/section.rs @@ -46,11 +46,11 @@ impl From for Semantics { } } -impl Into for Semantics { - fn into(self) -> BNSectionSemantics { +impl From for BNSectionSemantics { + fn from(semantics: Semantics) -> Self { use self::BNSectionSemantics::*; - match self { + match semantics { Semantics::DefaultSection => DefaultSectionSemantics, Semantics::ReadOnlyCode => ReadOnlyCodeSectionSemantics, Semantics::ReadOnlyData => ReadOnlyDataSectionSemantics, diff --git a/rust/src/symbol.rs b/rust/src/symbol.rs index 0b3ef224..6b04582b 100644 --- a/rust/src/symbol.rs +++ b/rust/src/symbol.rs @@ -50,11 +50,11 @@ impl From for SymbolType { } } -impl Into for SymbolType { - fn into(self) -> BNSymbolType { +impl From for BNSymbolType { + fn from(symbol_type: SymbolType) -> Self { use self::BNSymbolType::*; - match self { + match symbol_type { SymbolType::Function => FunctionSymbol, SymbolType::LibraryFunction => LibraryFunctionSymbol, SymbolType::ImportAddress => ImportAddressSymbol, @@ -87,11 +87,11 @@ impl From for Binding { } } -impl Into for Binding { - fn into(self) -> BNSymbolBinding { +impl From for BNSymbolBinding { + fn from(binding: Binding) -> Self { use self::BNSymbolBinding::*; - match self { + match binding { Binding::None => NoBinding, Binding::Local => LocalBinding, Binding::Global => GlobalBinding, @@ -299,8 +299,4 @@ impl PartialEq for Symbol { } } -impl Hash for Ref { - fn hash(&self, state: &mut H) { - (**self).hash(state); - } -} +impl Eq for Symbol {} diff --git a/rust/src/types.rs b/rust/src/types.rs index d9b28b4c..75475b6e 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -244,47 +244,38 @@ impl From for Conf { } } -impl Into for Conf<&Type> { - fn into(self) -> BNTypeWithConfidence { - BNTypeWithConfidence { - type_: self.contents.handle, - confidence: self.confidence, - } - } -} - -impl Into for &Conf<&Type> { - fn into(self) -> BNTypeWithConfidence { - BNTypeWithConfidence { - type_: self.contents.handle, - confidence: self.confidence, +impl From> for BNTypeWithConfidence { + fn from(conf: Conf<&Type>) -> Self { + Self { + type_: conf.contents.handle, + confidence: conf.confidence, } } } -impl Into for Conf { - fn into(self) -> BNBoolWithConfidence { - BNBoolWithConfidence { - value: self.contents, - confidence: self.confidence, +impl From> for BNBoolWithConfidence { + fn from(conf: Conf) -> Self { + Self { + value: conf.contents, + confidence: conf.confidence, } } } -impl Into for Conf<&CallingConvention> { - fn into(self) -> BNCallingConventionWithConfidence { - BNCallingConventionWithConfidence { - convention: self.contents.handle, - confidence: self.confidence, +impl From>> for BNCallingConventionWithConfidence { + fn from(conf: Conf<&CallingConvention>) -> Self { + Self { + convention: conf.contents.handle, + confidence: conf.confidence, } } } -impl Into for Conf { - fn into(self) -> BNOffsetWithConfidence { - BNOffsetWithConfidence { - value: self.contents, - confidence: self.confidence, +impl From> for BNOffsetWithConfidence { + fn from(conf: Conf) -> Self { + Self { + value: conf.contents, + confidence: conf.confidence, } } } @@ -664,7 +655,6 @@ impl Drop for TypeBuilder { ////////// // Type -#[derive(Eq)] pub struct Type { pub(crate) handle: *mut BNType, } @@ -1245,6 +1235,8 @@ impl PartialEq for Type { } } +impl Eq for Type {} + impl Hash for Type { fn hash(&self, state: &mut H) { self.handle.hash(state); @@ -1417,12 +1409,7 @@ impl EnumerationBuilder { self } - pub fn replace( - &mut self, - id: usize, - name: S, - value: u64, - ) -> &mut Self { + pub fn replace(&self, id: usize, name: S, value: u64) -> &Self { let name = name.into_bytes_with_nul(); unsafe { BNReplaceEnumerationBuilderMember(self.handle, id, name.as_ref().as_ptr() as _, value); @@ -1622,7 +1609,7 @@ impl StructureBuilder { self } - pub fn append<'a, 'b, S: BnStrCompatible, T: Into>>( + pub fn append<'a, S: BnStrCompatible, T: Into>>( &'a self, t: T, name: S, @@ -1643,11 +1630,11 @@ impl StructureBuilder { self } - pub fn insert_member<'a, 'b>( - &'a mut self, - member: &'b StructureMember, + pub fn insert_member( + &mut self, + member: &StructureMember, overwrite_existing: bool, - ) -> &'a mut Self { + ) -> &mut Self { let ty = member.ty.clone(); self.insert( ty.as_ref(), @@ -1660,15 +1647,15 @@ impl StructureBuilder { self } - pub fn insert<'a, 'b, S: BnStrCompatible, T: Into>>( - &'a mut self, + pub fn insert<'a, S: BnStrCompatible, T: Into>>( + &mut self, t: T, name: S, offset: u64, overwrite_existing: bool, access: MemberAccess, scope: MemberScope, - ) -> &'a mut Self { + ) -> &mut Self { let name = name.into_bytes_with_nul(); unsafe { BNAddStructureBuilderMemberAtOffset( @@ -1967,7 +1954,7 @@ impl QualifiedName { pub fn join(&self) -> Cow { let join: *mut c_char = self.0.join; - unsafe { CStr::from_ptr(join).to_string_lossy() } + unsafe { CStr::from_ptr(join) }.to_string_lossy() } pub fn strings(&self) -> Vec> { -- cgit v1.3.1