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 clippy lints #6310

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion rust/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn main() {
let _ = std::fs::create_dir("target/doc");
let _ = std::fs::copy("../docs/img/favicon.ico", "target/doc/favicon.ico");
let _ = std::fs::copy(
"under_construction.png",
"assets/under_construction.png",
mkrasnitski marked this conversation as resolved.
Show resolved Hide resolved
"target/doc/under_construction.png",
);
let _ = std::fs::copy("../docs/img/logo.png", "target/doc/logo.png");
Expand Down
1 change: 1 addition & 0 deletions rust/clippy.toml
mkrasnitski marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
too-many-arguments-threshold = 8
26 changes: 16 additions & 10 deletions rust/src/architecture.rs
Copy link
Member

Choose a reason for hiding this comment

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

I find the before to be more readable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it would be best if the definition of BNInstructionInfo was the following instead:

pub struct BNInstructionInfo {
    pub length: usize,
    pub branchCount: usize,
    pub archTransitionByTargetAddr: bool,
    pub delaySlots: u8,
    pub branchInfo: [BNBranchInfo; 3usize],
}

pub struct BNBranchInfo {
    pub branchType: BNBranchType,
    pub branchTarget: u64,
    #[doc = " If null, same architecture as instruction"]
    pub branchArch: *mut BNArchitecture,
}

Then we could iterate over all the branch info at once instead of by-index. This would require changing the C++ definition though.

Original file line number Diff line number Diff line change
Expand Up @@ -200,25 +200,31 @@ impl From<BNInstructionInfo> for InstructionInfo {
fn from(value: BNInstructionInfo) -> Self {
// TODO: This is quite ugly, but we destructure the branch info so this will have to do.
let mut branch_info = [None; NUM_BRANCH_INFO];
for i in 0..value.branchCount.min(NUM_BRANCH_INFO) {
let branch_target = value.branchTarget[i];
branch_info[i] = Some(BranchInfo {
kind: match value.branchType[i] {
BNBranchType::UnconditionalBranch => BranchKind::Unconditional(branch_target),
BNBranchType::FalseBranch => BranchKind::False(branch_target),
BNBranchType::TrueBranch => BranchKind::True(branch_target),
BNBranchType::CallDestination => BranchKind::Call(branch_target),
for (i, slot) in branch_info
.iter_mut()
.enumerate()
.take(value.branchCount.min(NUM_BRANCH_INFO))
{
let target = value.branchTarget[i];
let kind = value.branchType[i];
let arch = value.branchArch[i];
*slot = Some(BranchInfo {
kind: match kind {
BNBranchType::UnconditionalBranch => BranchKind::Unconditional(target),
BNBranchType::FalseBranch => BranchKind::False(target),
BNBranchType::TrueBranch => BranchKind::True(target),
BNBranchType::CallDestination => BranchKind::Call(target),
BNBranchType::FunctionReturn => BranchKind::FunctionReturn,
BNBranchType::SystemCall => BranchKind::SystemCall,
BNBranchType::IndirectBranch => BranchKind::Indirect,
BNBranchType::ExceptionBranch => BranchKind::Exception,
BNBranchType::UnresolvedBranch => BranchKind::Unresolved,
BNBranchType::UserDefinedBranch => BranchKind::UserDefined,
},
arch: if value.branchArch[i].is_null() {
arch: if arch.is_null() {
None
} else {
Some(unsafe { CoreArchitecture::from_raw(value.branchArch[i]) })
Some(unsafe { CoreArchitecture::from_raw(arch) })
},
});
}
Expand Down
2 changes: 1 addition & 1 deletion rust/src/basicblock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl<C: BlockContext> BasicBlock<C> {
// TODO iterated dominance frontier
}

impl<'a, C: BlockContext> IntoIterator for &'a BasicBlock<C> {
impl<C: BlockContext> IntoIterator for &BasicBlock<C> {
type Item = C::Instruction;
type IntoIter = C::Iter;

Expand Down
2 changes: 1 addition & 1 deletion rust/src/binaryview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ pub trait BinaryViewExt: BinaryViewBase {
let name_array = unsafe { Array::<QualifiedName>::new(result_names, result_count, ()) };
id_array
.into_iter()
.zip(name_array.into_iter())
.zip(&name_array)
.map(|(id, name)| (id.to_owned(), name))
.collect()
}
Expand Down
6 changes: 3 additions & 3 deletions rust/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,13 @@ impl Component {
}

/// Original name set for this component

///
/// :note: The `.display_name` property should be used for `bv.get_component_by_path()` lookups.

///
/// This can differ from the .display_name property if one of its sibling components has the same .original_name; In that
/// case, .name will be an automatically generated unique name (e.g. "MyComponentName (1)") while .original_name will
/// remain what was originally set (e.g. "MyComponentName")

///
/// If this component has a duplicate name and is moved to a component where none of its siblings share its name,
/// .name will return the original "MyComponentName"
pub fn name(&self) -> BnString {
Expand Down
2 changes: 1 addition & 1 deletion rust/src/databuffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl DataBuffer {

impl Default for DataBuffer {
fn default() -> Self {
Self(unsafe { BNCreateDataBuffer([].as_ptr() as *const c_void, 0) })
Self(unsafe { BNCreateDataBuffer([].as_ptr(), 0) })
}
}

Expand Down
7 changes: 2 additions & 5 deletions rust/src/demangle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,7 @@ impl Demangler {
})
}

extern "C" fn cb_free_var_name<C>(_ctxt: *mut c_void, name: *mut BNQualifiedName)
where
C: CustomDemangler,
{
extern "C" fn cb_free_var_name(_ctxt: *mut c_void, name: *mut BNQualifiedName) {
ffi_wrap!("CustomDemangler::cb_free_var_name", unsafe {
// TODO: What is the point of this free callback?
// TODO: The core can just call BNFreeQualifiedName.
Expand All @@ -323,7 +320,7 @@ impl Demangler {
context: ctxt as *mut c_void,
isMangledString: Some(cb_is_mangled_string::<C>),
demangle: Some(cb_demangle::<C>),
freeVarName: Some(cb_free_var_name::<C>),
freeVarName: Some(cb_free_var_name),
};

unsafe {
Expand Down
2 changes: 1 addition & 1 deletion rust/src/disassembly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl From<InstructionTextToken> for BNInstructionTextToken {
let kind_value = value.kind.try_value().unwrap_or(0);
let operand = value.kind.try_operand().unwrap_or(0);
let size = value.kind.try_size().unwrap_or(0);
let type_names = value.kind.try_type_names().unwrap_or(vec![]);
let type_names = value.kind.try_type_names().unwrap_or_default();
Self {
type_: value.kind.into(),
// Expected to be freed with `InstructionTextToken::free_raw`.
Expand Down
47 changes: 21 additions & 26 deletions rust/src/enterprise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,44 +20,39 @@ pub fn checkout_license(duration: Duration) -> Result<(), EnterpriseCheckoutErro
return Ok(());
}

if !is_server_initialized() {
if !initialize_server() && is_server_floating_license() {
return Err(EnterpriseCheckoutError(server_last_error().to_string()));
}
if !is_server_initialized() && !initialize_server() && is_server_floating_license() {
return Err(EnterpriseCheckoutError(server_last_error().to_string()));
}

if is_server_floating_license() {
if !is_server_connected() && !connect_server() {
return Err(EnterpriseCheckoutError(server_last_error().to_string()));
}

if !is_server_authenticated() {
if !authenticate_server_with_method("Keychain", false) {
let Some(username) = std::env::var("BN_ENTERPRISE_USERNAME").ok() else {
return Err(EnterpriseCheckoutError("BN_ENTERPRISE_USERNAME not set when attempting to authenticate with credentials".to_string()));
};
let Some(password) = std::env::var("BN_ENTERPRISE_PASSWORD").ok() else {
return Err(EnterpriseCheckoutError("BN_ENTERPRISE_PASSWORD not set when attempting to authenticate with credentials".to_string()));
};
if !authenticate_server_with_credentials(username, password, true) {
let failed_message = "Could not checkout a license: Not authenticated. Try one of the following: \n \
- Log in and check out a license for an extended time\n \
- Set BN_ENTERPRISE_USERNAME and BN_ENTERPRISE_PASSWORD environment variables\n \
- Use binaryninja::enterprise::{authenticate_server_with_method OR authenticate_server_with_credentials} in your code";
return Err(EnterpriseCheckoutError(failed_message.to_string()));
}
if !is_server_authenticated() && !authenticate_server_with_method("Keychain", false) {
let Some(username) = std::env::var("BN_ENTERPRISE_USERNAME").ok() else {
return Err(EnterpriseCheckoutError("BN_ENTERPRISE_USERNAME not set when attempting to authenticate with credentials".to_string()));
};
let Some(password) = std::env::var("BN_ENTERPRISE_PASSWORD").ok() else {
return Err(EnterpriseCheckoutError("BN_ENTERPRISE_PASSWORD not set when attempting to authenticate with credentials".to_string()));
};
if !authenticate_server_with_credentials(username, password, true) {
let failed_message = "Could not checkout a license: Not authenticated. Try one of the following: \n \
- Log in and check out a license for an extended time\n \
- Set BN_ENTERPRISE_USERNAME and BN_ENTERPRISE_PASSWORD environment variables\n \
- Use binaryninja::enterprise::{authenticate_server_with_method OR authenticate_server_with_credentials} in your code";
return Err(EnterpriseCheckoutError(failed_message.to_string()));
}
}
}

if !is_server_license_still_activated()
|| (!is_server_floating_license() && crate::license_expiration_time() < SystemTime::now())
if (!is_server_license_still_activated()
|| (!is_server_floating_license() && crate::license_expiration_time() < SystemTime::now()))
&& !update_server_license(duration)
{
if !update_server_license(duration) {
return Err(EnterpriseCheckoutError(
"Failed to refresh expired license".to_string(),
));
}
return Err(EnterpriseCheckoutError(
"Failed to refresh expired license".to_string(),
));
}

Ok(())
Expand Down
4 changes: 2 additions & 2 deletions rust/src/flowgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl<'a> FlowGraphNode<'a> {
}
}

unsafe impl<'a> RefCountable for FlowGraphNode<'a> {
unsafe impl RefCountable for FlowGraphNode<'_> {
unsafe fn inc_ref(handle: &Self) -> Ref<Self> {
Ref::new(Self {
handle: BNNewFlowGraphNodeReference(handle.handle),
Expand All @@ -104,7 +104,7 @@ unsafe impl<'a> RefCountable for FlowGraphNode<'a> {
}
}

impl<'a> ToOwned for FlowGraphNode<'a> {
impl ToOwned for FlowGraphNode<'_> {
type Owned = Ref<Self>;

fn to_owned(&self) -> Self::Owned {
Expand Down
2 changes: 1 addition & 1 deletion rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ unsafe extern "C" fn cb_progress_func<F: FnMut(usize, usize) -> bool>(
if ctxt.is_null() {
return true;
}
let closure: &mut F = std::mem::transmute(ctxt);
let closure = &mut *(ctxt as *mut F);
closure(progress, total)
}

Expand Down
20 changes: 10 additions & 10 deletions rust/src/llil/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ pub struct LowLevelILFunction<A: Architecture, M: FunctionMutability, F: Functio
_form: PhantomData<F>,
}

impl<'func, A, M, F> LowLevelILFunction<A, M, F>
impl<A, M, F> LowLevelILFunction<A, M, F>
where
A: 'func + Architecture,
A: Architecture,
M: FunctionMutability,
F: FunctionForm,
{
Expand Down Expand Up @@ -137,9 +137,9 @@ where
// LLIL basic blocks are not available until the function object
// is finalized, so ensure we can't try requesting basic blocks
// during lifting
impl<'func, A, F> LowLevelILFunction<A, Finalized, F>
impl<A, F> LowLevelILFunction<A, Finalized, F>
where
A: 'func + Architecture,
A: Architecture,
F: FunctionForm,
{
pub fn basic_blocks(&self) -> Array<BasicBlock<LowLevelBlock<A, Finalized, F>>> {
Expand Down Expand Up @@ -176,9 +176,9 @@ impl LowLevelILFunction<CoreArchitecture, Mutable, NonSSA<LiftedNonSSA>> {
}
}

impl<'func, A, M, F> ToOwned for LowLevelILFunction<A, M, F>
impl<A, M, F> ToOwned for LowLevelILFunction<A, M, F>
where
A: 'func + Architecture,
A: Architecture,
M: FunctionMutability,
F: FunctionForm,
{
Expand All @@ -189,9 +189,9 @@ where
}
}

unsafe impl<'func, A, M, F> RefCountable for LowLevelILFunction<A, M, F>
unsafe impl<A, M, F> RefCountable for LowLevelILFunction<A, M, F>
where
A: 'func + Architecture,
A: Architecture,
M: FunctionMutability,
F: FunctionForm,
{
Expand All @@ -210,9 +210,9 @@ where
}
}

impl<'func, A, M, F> fmt::Debug for LowLevelILFunction<A, M, F>
impl<A, M, F> fmt::Debug for LowLevelILFunction<A, M, F>
where
A: 'func + Architecture,
A: Architecture,
M: FunctionMutability,
F: FunctionForm,
{
Expand Down
2 changes: 1 addition & 1 deletion rust/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ unsafe extern "C" fn cb_progress_func<F: FnMut(usize, usize) -> bool>(
if ctxt.is_null() {
return true;
}
let closure: &mut F = mem::transmute(ctxt);
let closure = &mut *(ctxt as *mut F);
closure(progress, total)
}

Expand Down
2 changes: 1 addition & 1 deletion rust/src/section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl<S: BnStrCompatible> SectionBuilder<S> {
let len = self.range.end.wrapping_sub(start);

unsafe {
let nul_str = std::ffi::CStr::from_bytes_with_nul_unchecked(b"\x00").as_ptr();
let nul_str = c"".as_ptr();
let name_ptr = name.as_ref().as_ptr() as *mut _;
let ty_ptr = ty.map_or(nul_str, |s| s.as_ref().as_ptr() as *mut _);
let linked_section_ptr =
Expand Down
4 changes: 0 additions & 4 deletions rust/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ impl Settings {
}
}

pub fn default() -> Ref<Self> {
Self::new("default")
}

pub fn set_resource_id<S: BnStrCompatible>(&self, resource_id: S) {
let resource_id = resource_id.into_bytes_with_nul();
unsafe { BNSettingsSetResourceId(self.handle, resource_id.as_ref().as_ptr() as *mut _) };
Expand Down
2 changes: 1 addition & 1 deletion rust/src/typearchive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ impl TypeArchive {
}

/// Get a list of all types' names and ids in the archive at a current snapshot

///
/// * `snapshot` - Snapshot id to search for types
pub fn get_type_names_and_ids<S: BnStrCompatible>(
&self,
Expand Down
2 changes: 1 addition & 1 deletion rust/src/typecontainer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ impl TypeContainer {

/// Parse an entire block of source into types, variables, and functions, with
/// knowledge of the types in the Type Container.

///
/// * `source` - Source code to parse
/// * `file_name` - Name of the file containing the source (optional: exists on disk)
/// * `options` - String arguments to pass as options, e.g. command line arguments
Expand Down
6 changes: 3 additions & 3 deletions rust/src/typeparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,9 @@ impl From<BNTypeParserResult> for TypeParserResult {
let raw_functions =
unsafe { std::slice::from_raw_parts(value.functions, value.functionCount) };
let result = TypeParserResult {
types: raw_types.iter().map(|t| ParsedType::from(t)).collect(),
variables: raw_variables.iter().map(|t| ParsedType::from(t)).collect(),
functions: raw_functions.iter().map(|t| ParsedType::from(t)).collect(),
types: raw_types.iter().map(ParsedType::from).collect(),
variables: raw_variables.iter().map(ParsedType::from).collect(),
functions: raw_functions.iter().map(ParsedType::from).collect(),
};
// SAFETY: `value` must be a properly initialized BNTypeParserResult.
unsafe { BNFreeTypeParserResult(&mut value) };
Expand Down
8 changes: 2 additions & 6 deletions rust/src/typeprinter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,8 @@ impl CoreTypePrinter {
impl Default for CoreTypePrinter {
fn default() -> Self {
// TODO: Remove this entirely, there is no "default", its view specific lets not make this some defined behavior.
let default_settings = crate::settings::Settings::default();
let name = default_settings.get_string(
std::ffi::CStr::from_bytes_with_nul(b"analysis.types.printerName\x00").unwrap(),
None,
None,
);
let default_settings = crate::settings::Settings::new("default");
let name = default_settings.get_string("analysis.types.printerName", None, None);
Self::printer_by_name(name).unwrap()
}
}
Expand Down
2 changes: 1 addition & 1 deletion rust/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ unsafe impl CoreArrayProviderInner for (&str, Variable, &Type) {
) -> (&'a str, Variable, &'a Type) {
let name = CStr::from_ptr(raw.name).to_str().unwrap();
let var = Variable::from(raw.var);
let var_type = std::mem::transmute(&raw.type_);
let var_type = &*(raw.type_ as *mut Type);
(name, var, var_type)
}
}
Expand Down
6 changes: 4 additions & 2 deletions rust/src/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,8 +771,10 @@ impl From<BNPossibleValueSet> for PossibleValueSet {
// TODO: Anything requiring core allocation is missing!
impl From<PossibleValueSet> for BNPossibleValueSet {
fn from(value: PossibleValueSet) -> Self {
let mut raw = BNPossibleValueSet::default();
raw.state = value.value_type();
let mut raw = BNPossibleValueSet {
state: value.value_type(),
..Default::default()
};
match value {
PossibleValueSet::UndeterminedValue => {}
PossibleValueSet::EntryValue { reg } => {
Expand Down
Loading