Open joleeee opened 9 months ago
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 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.
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:
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#L6capstone-sys/pre_generated/update-bindings.md
Merging #149 (105a803) into master (7c9a51f) will decrease coverage by
0.01%
. The diff coverage isn/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
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:
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