dnsserver commented 2 years ago

What do you think about this API?

tmfink commented 2 years ago

This is a big improvement over the last PR. :smile:

Overall, I like the approach of having separate type to "own" the cs_insn allocation. I also like how DisasmIter tracks the "updates" from repeated calls to cs_disasm_iter().

I have some comments that I will make in-line.

tmfink commented 2 years ago

I just pushed a fix (#116) for the GitHub action failure; please rebase to the current master branch to get the fix.

codecov[bot] commented 2 years ago

Codecov Report

Merging #115 (1e57323) into master (854de72) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 1e57323 differs from pull request most recent head 06dbfb6. Consider uploading reports for the commit 06dbfb6 to get more accurate results

@@           Coverage Diff           @@
##           master     #115   +/-   ##
=======================================
  Coverage   94.90%   94.90%           
=======================================
  Files          22       22           
  Lines        2670     2670           
=======================================
  Hits         2534     2534           
  Misses        136      136

Impacted Files	Coverage Δ
capstone-rs/src/capstone.rs	`93.52% <ø> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 854de72...06dbfb6. Read the comment docs.

capstone-rust / capstone-rs

Support cs_disasm_iter #115

Codecov Report