More rust cleanup

- Make workflow cloning explicit
- Add workflow tests
- Add missing property string list getting for settings
- Remove IntoActivityName (see #6257)

Author: emesare

plugins/warp/src/plugin/workflow.rs

@@ -78,15 +78,17 @@ pub fn insert_workflow() {
         }
     };
 
-    let function_meta_workflow = Workflow::new_from_copy("core.function.metaAnalysis");
+    let old_function_meta_workflow = Workflow::instance("core.function.metaAnalysis");
+    let function_meta_workflow = old_function_meta_workflow.clone("core.function.metaAnalysis");
     let guid_activity = Activity::new_with_action(GUID_ACTIVITY_CONFIG, guid_activity);
     function_meta_workflow
         .register_activity(&guid_activity)
         .unwrap();
     function_meta_workflow.insert("core.function.runFunctionRecognizers", [GUID_ACTIVITY_NAME]);
     function_meta_workflow.register().unwrap();
 
-    let module_meta_workflow = Workflow::new_from_copy("core.module.metaAnalysis");
+    let old_module_meta_workflow = Workflow::instance("core.module.metaAnalysis");
+    let module_meta_workflow = old_module_meta_workflow.clone("core.module.metaAnalysis");
     let matcher_activity = Activity::new_with_action(MATCHER_ACTIVITY_CONFIG, matcher_activity);
     module_meta_workflow
         .register_activity(&matcher_activity)

rust/README.md

@@ -34,7 +34,6 @@ fn main() {
         println!("{}:", func.symbol().full_name());
     }
 }
-
 ```
 
 ## Getting Started
@@ -61,7 +60,7 @@ binaryninjacore-sys = { git = "https://github.com/Vector35/binaryninja-api.git",
 ```
 
 `build.rs`:
-```rust
+```doctestinjectablerust
 fn main() {
     let link_path =
         std::env::var_os("DEP_BINARYNINJACORE_PATH").expect("DEP_BINARYNINJACORE_PATH not specified");

rust/examples/workflow.rs

@@ -50,7 +50,8 @@ pub fn main() {
         binaryninja::headless::Session::new().expect("Failed to initialize session");
 
     println!("Registering workflow...");
-    let meta_workflow = Workflow::new_from_copy("core.function.metaAnalysis");
+    let old_meta_workflow = Workflow::instance("core.function.metaAnalysis");
+    let meta_workflow = old_meta_workflow.clone("core.function.metaAnalysis");
     let activity = Activity::new_with_action(RUST_ACTIVITY_CONFIG, example_activity);
     meta_workflow.register_activity(&activity).unwrap();
     meta_workflow.insert("core.function.runFunctionRecognizers", [RUST_ACTIVITY_NAME]);

rust/src/settings.rs

@@ -215,6 +215,28 @@ impl Settings {
         }
     }
 
+    pub fn get_property_string_list<S: BnStrCompatible>(
+        &self,
+        key: S,
+        property: S,
+    ) -> Array<BnString> {
+        let key = key.into_bytes_with_nul();
+        let property = property.into_bytes_with_nul();
+        let mut size: usize = 0;
+        unsafe {
+            Array::new(
+                BNSettingsQueryPropertyStringList(
+                    self.handle,
+                    key.as_ref().as_ptr() as *mut _,
+                    property.as_ref().as_ptr() as *mut _,
+                    &mut size,
+                ) as *mut *mut c_char,
+                size,
+                (),
+            )
+        }
+    }
+
     pub fn get_json<S: BnStrCompatible>(
         &self,
         key: S,

rust/src/workflow.rs

@@ -236,22 +236,6 @@ unsafe impl RefCountable for Activity {
     }
 }
 
-pub trait IntoActivityName {
-    fn activity_name(self) -> BnString;
-}
-
-impl IntoActivityName for &Activity {
-    fn activity_name(self) -> BnString {
-        self.name()
-    }
-}
-
-impl<S: BnStrCompatible> IntoActivityName for S {
-    fn activity_name(self) -> BnString {
-        BnString::new(self)
-    }
-}
-
 // TODO: We need to hide the JSON here behind a sensible/typed API.
 #[repr(transparent)]
 pub struct Workflow {
@@ -269,7 +253,7 @@ impl Workflow {
 
     /// Create a new unregistered [Workflow] with no activities.
     ///
-    /// To get a copy of an existing registered [Workflow] use [Workflow::new_from_copy].
+    /// To get a copy of an existing registered [Workflow] use [Workflow::clone].
     pub fn new<S: BnStrCompatible>(name: S) -> Self {
         let name = name.into_bytes_with_nul();
         let result = unsafe { BNCreateWorkflow(name.as_ref().as_ptr() as *const c_char) };
@@ -280,28 +264,26 @@ impl Workflow {
     ///
     /// * `name` - the name for the new [Workflow]
     #[must_use]
-    pub fn new_from_copy<S: BnStrCompatible + Clone>(name: S) -> Workflow {
-        Self::new_from_copy_with_root(name, "")
+    pub fn clone<S: BnStrCompatible + Clone>(&self, name: S) -> Workflow {
+        self.clone_with_root(name, "")
     }
 
     /// Make a new unregistered [Workflow], copying all activities, within `root_activity`, and the execution strategy.
     ///
     /// * `name` - the name for the new [Workflow]
     /// * `root_activity` - perform the clone operation with this activity as the root
     #[must_use]
-    pub fn new_from_copy_with_root<S: BnStrCompatible + Clone, A: IntoActivityName>(
+    pub fn clone_with_root<S: BnStrCompatible, A: BnStrCompatible>(
+        &self,
         name: S,
         root_activity: A,
     ) -> Workflow {
-        let raw_name = name.clone().into_bytes_with_nul();
-        let activity = root_activity.activity_name();
-        // I can't think of a single reason as to why we should let users pass a workflow handle into this.
-        // To prevent warning being emitted we default to the name.
-        let placeholder_workflow = Workflow::instance(name);
+        let raw_name = name.into_bytes_with_nul();
+        let activity = root_activity.into_bytes_with_nul();
         unsafe {
             Self::from_raw(
                 NonNull::new(BNWorkflowClone(
-                    placeholder_workflow.handle.as_ptr(),
+                    self.handle.as_ptr(),
                     raw_name.as_ref().as_ptr() as *const c_char,
                     activity.as_ref().as_ptr() as *const c_char,
                 ))
@@ -371,14 +353,16 @@ impl Workflow {
     ) -> Result<Activity, ()>
     where
         I: IntoIterator,
-        I::Item: IntoActivityName,
+        I::Item: BnStrCompatible,
     {
-        let subactivities_raw: Vec<BnString> = subactivities
+        let subactivities_raw: Vec<_> = subactivities
             .into_iter()
-            .map(|x| x.activity_name())
+            .map(|x| x.into_bytes_with_nul())
+            .collect();
+        let mut subactivities_ptr: Vec<*const _> = subactivities_raw
+            .iter()
+            .map(|x| x.as_ref().as_ptr() as *const c_char)
             .collect();
-        let mut subactivities_ptr: Vec<*const _> =
-            subactivities_raw.iter().map(|x| x.as_ptr()).collect();
         let result = unsafe {
             BNWorkflowRegisterActivity(
                 self.handle.as_ptr(),
@@ -392,17 +376,30 @@ impl Workflow {
     }
 
     /// Determine if an Activity exists in this [Workflow].
-    pub fn contains<A: IntoActivityName>(&self, activity: A) -> bool {
-        unsafe { BNWorkflowContains(self.handle.as_ptr(), activity.activity_name().as_ptr()) }
+    pub fn contains<A: BnStrCompatible>(&self, activity: A) -> bool {
+        unsafe {
+            BNWorkflowContains(
+                self.handle.as_ptr(),
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
+            )
+        }
+    }
+
+    /// Retrieve the configuration as an adjacency list in JSON for the [Workflow].
+    pub fn configuration(&self) -> BnString {
+        self.configuration_with_activity("")
     }
 
     /// Retrieve the configuration as an adjacency list in JSON for the
-    /// [Workflow], or if specified just for the given `activity`.
+    /// [Workflow], just for the given `activity`.
     ///
-    /// `activity` - if specified, return the configuration for the `activity`
-    pub fn configuration<A: IntoActivityName>(&self, activity: A) -> BnString {
+    /// `activity` - return the configuration for the `activity`
+    pub fn configuration_with_activity<A: BnStrCompatible>(&self, activity: A) -> BnString {
         let result = unsafe {
-            BNWorkflowGetConfiguration(self.handle.as_ptr(), activity.activity_name().as_ptr())
+            BNWorkflowGetConfiguration(
+                self.handle.as_ptr(),
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
+            )
         };
         assert!(!result.is_null());
         unsafe { BnString::from_raw(result) }
@@ -433,12 +430,12 @@ impl Workflow {
     /// specified just for the given `activity`.
     ///
     /// * `activity` - if specified, return the roots for the `activity`
-    pub fn activity_roots<A: IntoActivityName>(&self, activity: A) -> Array<BnString> {
+    pub fn activity_roots<A: BnStrCompatible>(&self, activity: A) -> Array<BnString> {
         let mut count = 0;
         let result = unsafe {
             BNWorkflowGetActivityRoots(
                 self.handle.as_ptr(),
-                activity.activity_name().as_ptr(),
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
                 &mut count,
             )
         };
@@ -450,7 +447,7 @@ impl Workflow {
     ///
     /// * `activity` - if specified, return the direct children and optionally the descendants of the `activity` (includes `activity`)
     /// * `immediate` - whether to include only direct children of `activity` or all descendants
-    pub fn subactivities<A: IntoActivityName>(
+    pub fn subactivities<A: BnStrCompatible>(
         &self,
         activity: A,
         immediate: bool,
@@ -459,7 +456,7 @@ impl Workflow {
         let result = unsafe {
             BNWorkflowGetSubactivities(
                 self.handle.as_ptr(),
-                activity.activity_name().as_ptr(),
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
                 immediate,
                 &mut count,
             )
@@ -474,20 +471,23 @@ impl Workflow {
     /// * `activities` - the list of Activities to assign
     pub fn assign_subactivities<A, I>(&self, activity: A, activities: I) -> bool
     where
-        A: IntoActivityName,
+        A: BnStrCompatible,
         I: IntoIterator,
-        I::Item: IntoActivityName,
+        I::Item: BnStrCompatible,
     {
-        let mut input_list: Vec<BnString> =
-            activities.into_iter().map(|a| a.activity_name()).collect();
-        // SAFETY: this works because BnString and *mut ffi::c_char are
-        // transmutable
-        let input_list_ptr = input_list.as_mut_ptr() as *mut *const c_char;
+        let input_list: Vec<_> = activities
+            .into_iter()
+            .map(|a| a.into_bytes_with_nul())
+            .collect();
+        let mut input_list_ptr: Vec<*const _> = input_list
+            .iter()
+            .map(|x| x.as_ref().as_ptr() as *const c_char)
+            .collect();
         unsafe {
             BNWorkflowAssignSubactivities(
                 self.handle.as_ptr(),
-                activity.activity_name().as_ptr(),
-                input_list_ptr,
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
+                input_list_ptr.as_mut_ptr(),
                 input_list.len(),
             )
         }
@@ -504,44 +504,52 @@ impl Workflow {
     /// * `activities` - the list of Activities to insert
     pub fn insert<A, I>(&self, activity: A, activities: I) -> bool
     where
-        A: IntoActivityName,
+        A: BnStrCompatible,
         I: IntoIterator,
-        I::Item: IntoActivityName,
+        I::Item: BnStrCompatible,
     {
-        let mut input_list: Vec<BnString> =
-            activities.into_iter().map(|a| a.activity_name()).collect();
-        // SAFETY: this works because BnString and *mut ffi::c_char are
-        // transmutable
-        let input_list_ptr = input_list.as_mut_ptr() as *mut *const c_char;
+        let input_list: Vec<_> = activities
+            .into_iter()
+            .map(|a| a.into_bytes_with_nul())
+            .collect();
+        let mut input_list_ptr: Vec<*const _> = input_list
+            .iter()
+            .map(|x| x.as_ref().as_ptr() as *const c_char)
+            .collect();
         unsafe {
             BNWorkflowInsert(
                 self.handle.as_ptr(),
-                activity.activity_name().as_ptr(),
-                input_list_ptr,
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
+                input_list_ptr.as_mut_ptr(),
                 input_list.len(),
             )
         }
     }
 
     /// Remove the specified `activity`
-    pub fn remove<A: IntoActivityName>(&self, activity: A) -> bool {
-        unsafe { BNWorkflowRemove(self.handle.as_ptr(), activity.activity_name().as_ptr()) }
+    pub fn remove<A: BnStrCompatible>(&self, activity: A) -> bool {
+        unsafe {
+            BNWorkflowRemove(
+                self.handle.as_ptr(),
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
+            )
+        }
     }
 
     /// Replace the specified `activity`.
     ///
     /// * `activity` - the Activity to replace
     /// * `new_activity` - the replacement Activity
-    pub fn replace<A: IntoActivityName, N: IntoActivityName>(
+    pub fn replace<A: BnStrCompatible, N: BnStrCompatible>(
         &self,
         activity: A,
         new_activity: N,
     ) -> bool {
         unsafe {
             BNWorkflowReplace(
                 self.handle.as_ptr(),
-                activity.activity_name().as_ptr(),
-                new_activity.activity_name().as_ptr(),
+                activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
+                new_activity.into_bytes_with_nul().as_ref().as_ptr() as *const c_char,
             )
         }
     }
@@ -550,15 +558,20 @@ impl Workflow {
     ///
     /// * `activity` - if specified, generate the Flowgraph using `activity` as the root
     /// * `sequential` - whether to generate a **Composite** or **Sequential** style graph
-    pub fn graph<A: IntoActivityName>(
+    pub fn graph<A: BnStrCompatible>(
         &self,
         activity: A,
         sequential: Option<bool>,
     ) -> Option<FlowGraph> {
         let sequential = sequential.unwrap_or(false);
-        let activity_name = activity.activity_name();
-        let graph =
-            unsafe { BNWorkflowGetGraph(self.handle.as_ptr(), activity_name.as_ptr(), sequential) };
+        let activity_name = activity.into_bytes_with_nul();
+        let graph = unsafe {
+            BNWorkflowGetGraph(
+                self.handle.as_ptr(),
+                activity_name.as_ref().as_ptr() as *const c_char,
+                sequential,
+            )
+        };
         if graph.is_null() {
             return None;
         }