More rust cleanup

- Use GroupId instead of u64
- Use ProgressCallback in place of ProgressExecutor
- Misc cleanup of FileMetadata
- Add `save_to_path` and `save_to_accessor` to save modified binaries
- Added binary_view unit tests
- Added collaboration unit tests
- Fixed a few issues with the collaboration apis
- Renamed Command registration functions so that there is no import ambiguity
- Split out RemoteUndoEntry
- Collaboration apis now have a explicit `_with_progress` set of apis
- Misc clippy lint fixes

Author: emesare

plugins/dwarf/dwarf_export/src/lib.rs

@@ -13,7 +13,7 @@ use std::fs;
 use binaryninja::logger::Logger;
 use binaryninja::{
     binary_view::{BinaryView, BinaryViewBase, BinaryViewExt},
-    command::{register, Command},
+    command::{register_command, Command},
     confidence::Conf,
     interaction,
     interaction::{FormResponses, FormResponses::Index},
@@ -781,7 +781,7 @@ pub extern "C" fn CorePluginInit() -> bool {
         .with_level(LevelFilter::Debug)
         .init();
 
-    register(
+    register_command(
         "Export as DWARF",
         "Export current analysis state and annotations as DWARF for import into other tools",
         MyCommand {},

plugins/dwarf/dwarfdump/src/lib.rs

@@ -14,7 +14,7 @@
 
 use binaryninja::{
     binary_view::{BinaryView, BinaryViewExt},
-    command::{register, Command},
+    command::{register_command, Command},
     disassembly::{DisassemblyTextLine, InstructionTextToken, InstructionTextTokenKind},
     flowgraph::{BranchType, EdgeStyle, FlowGraph, FlowGraphNode, FlowGraphOption},
 };
@@ -325,7 +325,7 @@ impl Command for DWARFDump {
 
 #[no_mangle]
 pub extern "C" fn UIPluginInit() -> bool {
-    register(
+    register_command(
         "DWARF Dump",
         "Show embedded DWARF info as a tree structure for you to navigate",
         DWARFDump {},

plugins/warp/src/plugin.rs

@@ -186,67 +186,67 @@ pub extern "C" fn CorePluginInit() -> bool {
 
     workflow::insert_workflow();
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Run Matcher",
         "Run the matcher manually",
         workflow::RunMatcher {},
     );
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Debug\\Cache",
         "Debug cache sizes... because...",
         DebugCache {},
     );
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Debug\\Invalidate Caches",
         "Invalidate all WARP caches",
         DebugInvalidateCache {},
     );
 
-    binaryninja::command::register_for_function(
+    binaryninja::command::register_command_for_function(
         "WARP\\Debug\\Function Signature",
         "Print the entire signature for the function",
         DebugFunction {},
     );
 
-    binaryninja::command::register_for_function(
+    binaryninja::command::register_command_for_function(
         "WARP\\Debug\\Function Matcher",
         "Print all possible matches for the function",
         DebugMatcher {},
     );
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Debug\\Apply Signature File Types",
         "Load all types from a signature file and ignore functions",
         types::LoadTypes {},
     );
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Load Signature File",
         "Load file into the matcher, this does NOT kick off matcher analysis",
         load::LoadSignatureFile {},
     );
 
-    binaryninja::command::register_for_function(
+    binaryninja::command::register_command_for_function(
         "WARP\\Copy Function GUID",
         "Copy the computed GUID for the function",
         copy::CopyFunctionGUID {},
     );
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Find Function From GUID",
         "Locate the function in the view using a GUID",
         find::FindFunctionFromGUID {},
     );
 
-    binaryninja::command::register(
+    binaryninja::command::register_command(
         "WARP\\Generate Signature File",
         "Generates a signature file containing all binary view functions",
         create::CreateSignatureFile {},
     );
 
-    binaryninja::command::register_for_function(
+    binaryninja::command::register_command_for_function(
         "WARP\\Add Function Signature to File",
         "Stores the signature for the function in the signature file",
         add::AddFunctionSignature {},

rust/src/binary_view.rs

@@ -37,7 +37,7 @@ use crate::function::{Function, NativeBlock};
 use crate::linear_view::{LinearDisassemblyLine, LinearViewCursor};
 use crate::metadata::Metadata;
 use crate::platform::Platform;
-use crate::progress::ProgressExecutor;
+use crate::progress::{NoProgressCallback, ProgressCallback};
 use crate::project::file::ProjectFile;
 use crate::rc::*;
 use crate::references::{CodeReference, DataReference};
@@ -145,11 +145,10 @@ pub trait BinaryViewBase: AsRef<BinaryView> {
     fn default_endianness(&self) -> Endianness;
     fn address_size(&self) -> usize;
 
-    // TODO saving fileaccessor
     fn save(&self) -> bool {
         self.as_ref()
             .parent_view()
-            .map(|bv| bv.save())
+            .map(|view| view.save())
             .unwrap_or(false)
     }
 }
@@ -627,38 +626,37 @@ pub trait BinaryViewExt: BinaryViewBase {
         T: Iterator<Item = I>,
         I: Into<QualifiedNameTypeAndId>,
     {
-        self.define_auto_types_with_progress(names_sources_and_types, ProgressExecutor::default())
+        self.define_auto_types_with_progress(names_sources_and_types, NoProgressCallback)
     }
 
-    fn define_auto_types_with_progress<T, I>(
+    fn define_auto_types_with_progress<T, I, P>(
         &self,
         names_sources_and_types: T,
-        progress: impl Into<ProgressExecutor>,
+        mut progress: P,
     ) -> HashMap<String, QualifiedName>
     where
         T: Iterator<Item = I>,
         I: Into<QualifiedNameTypeAndId>,
+        P: ProgressCallback,
     {
         let mut types: Vec<BNQualifiedNameTypeAndId> = names_sources_and_types
             .map(Into::into)
             .map(QualifiedNameTypeAndId::into_raw)
             .collect();
         let mut result_ids: *mut *mut c_char = std::ptr::null_mut();
         let mut result_names: *mut BNQualifiedName = std::ptr::null_mut();
-        let boxed_progress = Box::new(progress.into());
-        let leaked_boxed_progress = Box::into_raw(boxed_progress);
+
         let result_count = unsafe {
             BNDefineAnalysisTypes(
                 self.as_ref().handle,
                 types.as_mut_ptr(),
                 types.len(),
-                Some(ProgressExecutor::cb_execute),
-                leaked_boxed_progress as *mut c_void,
+                Some(P::cb_progress_callback),
+                &mut progress as *mut P as *mut c_void,
                 &mut result_ids as *mut _,
                 &mut result_names as *mut _,
             )
         };
-        let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
 
         for ty in types {
             QualifiedNameTypeAndId::free_raw(ty);
@@ -678,33 +676,30 @@ pub trait BinaryViewExt: BinaryViewBase {
         T: Iterator<Item = I>,
         I: Into<QualifiedNameAndType>,
     {
-        self.define_user_types_with_progress(names_and_types, ProgressExecutor::default());
+        self.define_user_types_with_progress(names_and_types, NoProgressCallback);
     }
 
-    fn define_user_types_with_progress<T, I>(
-        &self,
-        names_and_types: T,
-        progress: impl Into<ProgressExecutor>,
-    ) where
+    fn define_user_types_with_progress<T, I, P>(&self, names_and_types: T, mut progress: P)
+    where
         T: Iterator<Item = I>,
         I: Into<QualifiedNameAndType>,
+        P: ProgressCallback,
     {
         let mut types: Vec<BNQualifiedNameAndType> = names_and_types
             .map(Into::into)
             .map(QualifiedNameAndType::into_raw)
             .collect();
-        let boxed_progress = Box::new(progress.into());
-        let leaked_boxed_progress = Box::into_raw(boxed_progress);
+
         unsafe {
             BNDefineUserAnalysisTypes(
                 self.as_ref().handle,
                 types.as_mut_ptr(),
                 types.len(),
-                Some(ProgressExecutor::cb_execute),
-                leaked_boxed_progress as *mut c_void,
+                Some(P::cb_progress_callback),
+                &mut progress as *mut P as *mut c_void,
             )
         };
-        let _ = unsafe { Box::from_raw(leaked_boxed_progress) };
+
         for ty in types {
             QualifiedNameAndType::free_raw(ty);
         }
@@ -1865,8 +1860,7 @@ impl BinaryView {
     }
 
     pub fn from_accessor(meta: &FileMetadata, file: &mut FileAccessor) -> Result<Ref<Self>> {
-        let handle =
-            unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut file.api_object as *mut _) };
+        let handle = unsafe { BNCreateBinaryDataViewFromFile(meta.handle, &mut file.api_object) };
 
         if handle.is_null() {
             return Err(());
@@ -1886,6 +1880,17 @@ impl BinaryView {
 
         unsafe { Ok(Ref::new(Self { handle })) }
     }
+
+    /// Save the original binary file to the provided `file_path` along with any modifications.
+    pub fn save_to_path(&self, file_path: impl AsRef<Path>) -> bool {
+        let file = file_path.as_ref().into_bytes_with_nul();
+        unsafe { BNSaveToFilename(self.handle, file.as_ptr() as *mut _) }
+    }
+
+    /// Save the original binary file to the provided [`FileAccessor`] along with any modifications.
+    pub fn save_to_accessor(&self, file: &mut FileAccessor) -> bool {
+        unsafe { BNSaveToFile(self.handle, &mut file.api_object) }
+    }
 }
 
 impl BinaryViewBase for BinaryView {