Add a try_from implementation to convert from a cs_insn to an Insn

[s1341]

This PR adds a try_from implementation which allows one to convert from a raw * const cs_insn to an Insn.

We need this for @frida-rust, as frida generates cs_insns internally, and we'd like to be able to use capstone-rs's rich API on those instructions.

[tmfink]

Thanks for the PR! I'm always happy to look at contributions!

Notes:

1. Any public API that takes in a raw pointer like this would need to be unsafe, so we couldn't use the TryFrom trait. Something like:

impl<'a> for Insn<'a> {
    /* rest of impl block */

    /// docs here
    unsafe fn from_raw(insn: *const cs_insn) -> Self {
        Self {
            insn: *insn,
            _marker: PhantomData,
        }
    }
}

2. Please add a doc comment with an "Unsafe" section that explains: a. The function creates a new lifetime from nothing, so it is up to the caller to properly restrict the lifetime. b. Any other invariants that the caller is expected to uphold (e.g. that insn is a valid, non-null pointer)
3. Please add some unit tests.

[s1341]

I'm happy to change to from_raw, and will add a comment as you recommended.

Regarding unit tests, how do you propose I implement those?

[s1341]

Ok. Implemented your recommendations and added a unit test.

[tmfink]

Yes please. Please make sure to update the contents of you commit message.

On Wed, Apr 7, 2021, 03:05 s1341 @.***> wrote:

@.**** commented on this pull request.

In capstone-rs/src/test.rs https://github.com/capstone-rust/capstone-rs/pull/91#discussion_r608517046 :

@@ -3092,3 +3092,21 @@ fn test_insn_size_and_alignment() { assert_eq!(original.id(), transmuted.id()); } } + +#[test] +fn test_insn_from_raw() {

• use capstone_sys::cs_insn;
• let cs = Capstone::new()
• .x86()
• .mode(x86::ArchMode::Mode64)
• .build()
• .unwrap();
• let insns = cs.disasm_all(X86_CODE, START_TEST_ADDR).unwrap();
• for insn in insns.iter() {
• let raw_insn = &insn.insn as *const cs_insn;
• assert_eq!(format!("{:?}", unsafe { Insn::from_raw(raw_insn) }), format!("{:?}", insn));

You want me to squash the entire PR into a single commit?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/capstone-rust/capstone-rs/pull/91#discussion_r608517046, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIFROGMCI6I67DZDVYC5ITTHQU45ANCNFSM42K3E2BQ .

[s1341]

There you go! Ready to merge.

[tmfink]

Thanks for handling the changes!

[s1341]

Awesome! Thanks.

Do you know when you will merge #88 and cut a release?

[tmfink]

Awesome! Thanks.

Do you know when you will merge #88 and cut a release?

88 is already merged. I think I want to fix the CI infra issues before merging.

[s1341]

Please let me know when you do cut a release, I need it for my tool...

[tmfink]

Please let me know when you do cut a release, I need it for my tool...

Just released 0.8.0 :smile:

[s1341]

Thanks! It works great!