Regs access

[joleeee]

trafficstars

I think the if cfg! is kinda weird and should probably rather have it just gate the entire function instead? Anyway, I'm not sure how to handle assert!(len <= ints.len()); better, but it should never happen anyway so it probably doesn't matter?

Also I just picked 64 because python uses that, though I have a hard time imagining reading/writing to more than 64 registers: https://github.com/capstone-engine/capstone/blob/5fb8a423d4455cade99b12912142fd3a0c10d957/bindings/python/capstone/init.py#L929

[joleeee]

Also @tmfink, whats the current status on the project? Seems like it's missing some updates? I could possibly step in and try and get things up to date.

[tmfink]

I think the if cfg! is kinda weird and should probably rather have it just gate the entire function instead? Anyway, I'm not sure how to handle assert!(len <= ints.len()); better, but it should never happen anyway so it probably doesn't matter?

See the review comments (to come).

Also I just picked 64 because python uses that, though I have a hard time imagining reading/writing to more than 64 registers: https://github.com/capstone-engine/capstone/blob/5fb8a423d4455cade99b12912142fd3a0c10d957/bindings/python/capstone/init.py#L929

64 is a fine value, but should probably be defined as a constant (magic numbers are not self-describing).

Looks like this would fix #63.

[tmfink]

Also @tmfink, whats the current status on the project? Seems like it's missing some updates? I could possibly step in and try and get things up to date.

I'll file a separate issue to address this question.

To update the internal capstone version, please:

1. Update the update_capstone.rs script and run it to update the C library: https://github.com/capstone-rust/capstone-rs/blob/7c9a51f6eb81709cde1de7f7498a0e16f29bd852/capstone-sys/scripts/update_capstone.sh#L6
2. capstone-sys/pre_generated/update-bindings.md
3. Fix any build/test issues that come up.

[codecov[bot]]

Codecov Report

Merging #149 (105a803) into master (7c9a51f) will decrease coverage by 0.01%. The diff coverage is n/a.

:exclamation: Current head 105a803 differs from pull request most recent head 29e13a0. Consider uploading reports for the commit 29e13a0 to get more accurate results

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
- Coverage   93.99%   93.98%   -0.01%     
==========================================
  Files          22       22              
  Lines        2731     2728       -3     
  Branches     2685     2682       -3     
==========================================
- Hits         2567     2564       -3     
  Misses        164      164              

Files	Coverage Δ
capstone-rs/src/capstone.rs	92.36% <ø> (ø)

... and 3 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tmfink]

Also @tmfink, whats the current status on the project? Seems like it's missing some updates? I could possibly step in and try and get things up to date.

I'll file a separate issue to address this question.

I just filed #151. Thanks for asking about this. I've thought about this for a while but you finally gave me the push I needed. :smile: