Syscall parameter type #4055
Syscall parameter type #4055
Conversation
WalkthroughSwitched UNIX-side FFI binding for mincore from using libc c_uchar pointers to c_char pointers, updating the cast of the residency vector accordingly. Function signatures and Windows-specific code remain unchanged; no public API changes reported. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔊@watchgou 🚀Thanks for your contribution🎉！ 💡CodeRabbit(AI) will review your code first🔥！ Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
rocketmq-rust-bot
added
AI review first
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rocketmq-store/src/utils/ffi.rs (1)
91-99: Makeveca*mut u8and uselibc::c_uchar— avoid const-to-mutable cast (soundness + Clippy).Repo scan: only a commented call found at rocketmq-store/src/log_file/mapped_file/default_mapped_file_impl.rs:758; no active callers.
- pub fn mincore(addr: *const u8, len: usize, vec: *const u8) -> i32 { + pub fn mincore(addr: *const u8, len: usize, vec: *mut u8) -> i32 { #[cfg(unix)] { use std::ffi::c_void; - use libc::c_char; - unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_char) } + use libc::c_uchar; + unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_uchar) } }
🧹 Nitpick comments (2)
rocketmq-store/src/utils/ffi.rs (2)
100-104: Windows stub currently returns success; consider signaling “not supported.”Returning
0may be interpreted as success by callers. Consider-1(and documenting this) or gating call sites oncfg(unix)so Windows never calls this path.Minimal change:
- // Windows does not have mincore, so we just return 0 - 0 + // Windows does not have mincore; signal unsupported + -1Please confirm no Windows call sites rely on the current behavior.
45-50: Comment mismatch with behavior on Windows mlock/munlock.Comments say “just return Ok” but the code calls VirtualLock/VirtualUnlock and can error. Update comments for accuracy.
- // Windows does not have mlock, so we just return Ok + // Windows does not have mlock; use VirtualLock and map errors ... - // Windows does not have munlock, so we just return Ok + // Windows does not have munlock; use VirtualUnlock and map errorsAlso applies to: 71-75
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rocketmq-store/src/utils/ffi.rs(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
rocketmq-store/src/utils/ffi.rs
[error] 98-98: Command cargo clippy -- -D warnings failed due to compile error: mismatched types in FFI call to libc::mincore. The function expects vec: *mut c_uchar, but vec is passed as *mut c_char in rocketmq-store/src/utils/ffi.rs:98 (E0308).
🪛 GitHub Actions: Codecov
rocketmq-store/src/utils/ffi.rs
[error] 98-98: Rust compile error: mismatched types in mincore call. expected *mut u8, found *mut i8 for vec parameter in unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_char) } at rocketmq-store/src/utils/ffi.rs:98.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: auto-approve
| use libc::c_char; | ||
|
|
||
| unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_uchar) } | ||
| unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_char) } | ||
| } |
There was a problem hiding this comment.
Fix FFI type for mincore vec: use c_uchar, not c_char (compile error E0308).
libc::mincore expects vec: *mut c_uchar (aka *mut u8). Passing *mut c_char (*mut i8) causes the current CI failure.
Apply this diff:
- use libc::c_char;
+ use libc::c_uchar;
- unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_char) }
+ unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_uchar) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use libc::c_char; | |
| unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_uchar) } | |
| unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_char) } | |
| } | |
| use libc::c_uchar; | |
| unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_uchar) } | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] 98-98: Command cargo clippy -- -D warnings failed due to compile error: mismatched types in FFI call to libc::mincore. The function expects vec: *mut c_uchar, but vec is passed as *mut c_char in rocketmq-store/src/utils/ffi.rs:98 (E0308).
🪛 GitHub Actions: Codecov
[error] 98-98: Rust compile error: mismatched types in mincore call. expected *mut u8, found *mut i8 for vec parameter in unsafe { libc::mincore(addr as *mut c_void, len, vec as *mut c_char) } at rocketmq-store/src/utils/ffi.rs:98.
🤖 Prompt for AI Agents
In rocketmq-store/src/utils/ffi.rs around lines 96 to 99, the FFI call to
libc::mincore uses a pointer typed as *mut c_char which is incorrect and causes
E0308; change the type to *mut c_uchar (and import/use libc::c_uchar if not
already) and cast vec to that pointer when calling mincore so the third
parameter matches libc::mincore's expected *mut c_uchar.
rocketmq-rust-bot
added
approved
2 participants
Which Issue(s) This PR Fixes(Closes)
Fixes #issue_id
Brief Description
How Did You Test This Change?
Summary by CodeRabbit