add a handful of features and some testing#46
add a handful of features and some testing#46williballenthin wants to merge 50 commits intoidalib-rs:masterfrom
Conversation
fd88c74 to
1019108
Compare
ff72393 to
ea48de4
Compare
|
@xorpse for my awareness, is this PR on track to be included in a future release? i'd like to write a few local utilities, and i'm not sure if i should prepare to keep using my fork, or the upstream crate. |
and other byte operations
idalib/src/insn.rs
Outdated
| unsafe { idalib_get_disasm_line(autocxx::c_ulonglong(self.inner.ea)) } | ||
| } | ||
|
|
||
| pub fn print_operand(&self, n: usize) -> String { |
There was a problem hiding this comment.
We should ensure the index is valid, not just return a String for any index. In any case, let's move this to be a method on Operand itself, i.e., via Debug or Display, etc.
There was a problem hiding this comment.
moving formatting into Display on Operand requires operand to gain a new field to track the ea, and therefore not transparent. does that work for you?
|
thanks for the thorough review @xorpse! happy to address these comments and i'll ping you when they're handled. |
|
please take a look at 64ccba6 that adds |
That sounds fine to me :), thanks for fixing everything--I think we'd also need to add make sure that Operand and Insn are scoped to the IDB as we do with other types, otherwise we'd be able to call |
This PR adds a handful of features that are useful for porting capa to Rust (local experiment).
Perhaps more interestingly, it proposes a testing mechanism. I found that using the default test harness didn't work with idalib-rs, because IDA has to be used from the main thread, and the test harness spawns tests on worker threads. But, with custom test binaries, we can still develop and run tests, with just a bit more effort. Let me know what you think.