Ffi Code Review
by @anderskev
Reviews Rust FFI code for type safety, memory layout compatibility, string handling, callback patterns, and unsafe boundary correctness. Use when reviewing e...
clawhub install ffi-code-review📖 About This Skill
name: ffi-code-review description: "Reviews Rust FFI code for type safety, memory layout compatibility, string handling, callback patterns, and unsafe boundary correctness. Use when reviewing extern blocks, #[repr(C)] types, bindgen output, or code calling C/C++ libraries."
FFI Code Review
Review Workflow
1. Check Cargo.toml -- Note Rust edition (2024 has breaking changes to extern blocks and unsafe attributes), build-dependencies (bindgen, cc, pkg-config), crate-type (cdylib, staticlib), and links key
2. Check build.rs -- Verify link directives (cargo:rustc-link-lib, cargo:rustc-link-search), bindgen configuration, and C source compilation
3. Check extern blocks -- Verify calling conventions, symbol declarations, and safety annotations
4. Check type layout -- Every type crossing FFI must be #[repr(C)] or a primitive FFI type
5. Check string and pointer handling -- CStr/CString usage, null checks, ownership transfers
6. Check callbacks -- extern "C" fn pointers, panic safety across FFI boundary
7. Gates -- Complete Gates below before reporting; do not skip ahead on “internal verification”
Gates
Complete in order. Do not emit findings until Gate 4 passes for each issue.
Gate 1 — Crate context (on disk)
PASS when: You opened the reviewed crate’s Cargo.toml (workspace member path if applicable) and recorded edition =, plus any of links, crate-type, or build-dependencies that matter for this FFI.
Blocks rationalization: Edition-specific findings (unsafe extern "C" {}, #[unsafe(no_mangle)], etc.) require this — if edition is not 2024, do not flag 2024-only requirements.
Gate 2 — Linkage and binding sources
PASS when: If the crate links native code or uses bindgen/pkg-config, you opened build.rs (or the checked-in bindings entry point). If there is no build.rs, you stated that bindings are hand-written and reviewed those extern / include! sites.
Artifact: At least one path you opened (e.g. build.rs, src/ffi.rs, or OUT_DIR bindings via include!).
Gate 3 — Code evidence
PASS when: Every planned finding has a target [FILE:LINE] from a full function/block you read, not only diff hunks or partial snippets.
Gate 4 — Pre-report protocol
PASS when: You loaded and applied beagle-rust:review-verification-protocol, including FFI-Specific Verification for repr(C), safety comments, ownership/callbacks, or bindgen-heavy code.
Output Format
Report findings as:
[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.
Quick Reference
| Issue Type | Reference | |------------|-----------| | C-to-Rust type mapping, repr(C) layout, enums, opaque types | references/type-mapping.md | | Safe wrappers, ownership transfer, callbacks, build.rs, testing | references/safety-patterns.md |
Review Checklist
extern Blocks and Calling Conventions
extern "C" (explicit, not bare extern)extern "C" {} blocks written as unsafe extern "C" {}extern "C" fn (not default Rust calling convention)"C", "system" for Win32 API)#[link(name = "...")] specifies the correct library name#[link(name = "...", kind = "static")] used when statically linkingSymbol Management
#[no_mangle] to preserve symbol names#[no_mangle] written as #[unsafe(no_mangle)]#[export_name = "..."] written as #[unsafe(export_name = "...")]#[link_name = "..."] used when Rust name differs from C symbolpub (only public #[no_mangle] symbols appear in library output)Type Layout
#[repr(C)] -- Rust's default layout is undefinedstd::ffi / std::os::raw equivalents (c_int, c_char, c_void)i32 where C uses int -- use c_int (width varies by platform)__be32 use byte arrays ([u8; 4]), not Rust integers#[repr(C)] or #[repr(u8)]/#[repr(u32)] with explicit discriminantsbitflags crate), not a Rust enum#[non_exhaustive] on enums representing C enumerations that may gain new valuesString Handling
CStr (borrowed) or CString (owned), never &str or StringCString::new() result is checked for interior null bytes (returns Err on \0)CString outlives any *const c_char pointer derived from it via .as_ptr()*const c_char validated with CStr::from_ptr() inside unsafeto_str() which returns ResultOsStr/OsString and CStr, not &strOwnership and Allocation
Box::from_raw), C-allocated freed by CBox::into_raw / Box::from_raw paired correctly for heap transfersVec::into_raw_parts used when passing arrays to C (pointer + length + capacity)Drop running on C-allocated memory (and vice versa)Callbacks
extern "C" fn(...), not closures or fn(...)std::panic::catch_unwind to prevent panics from unwinding across FFI*mut c_void with safe reconstruction at call siteOption used for nullable function pointers (niche optimization)Bindgen and Build Scripts
-sys crate pattern used for raw bindings, separate crate for safe wrappersbuild.rs uses cargo:rustc-link-lib and cargo:rustc-link-search correctlylinks key in Cargo.toml prevents duplicate linking of the same native librarySafety Documentation
unsafe block has a // SAFETY: comment explaining invariantsunsafe fn bodies use explicit unsafe {} blocks around unsafe opsSeverity Calibration
Critical (Block Merge)
#[repr(C)] on types crossing FFI boundary (undefined memory layout)&str/String where CStr/CString requiredcatch_unwindextern "C" fn pointer requiredMajor (Should Fix)
unsafe blocks or public FFI functions*const T / *mut T before dereferencingCString dropped before its pointer is used by C (dangling pointer)#[link(name = "...")] causing link failures on some platformsextern block not marked unsafe extern#[no_mangle] not wrapped in #[unsafe(...)]Minor (Consider Fixing)
i32 instead of c_int for C int (correct on most platforms but not portable)#[non_exhaustive] on enums mapping to extensible C enumerationsInformational
-sys crate*mut c_void pointersOption> for nullable pointersValid Patterns (Do NOT Flag)
unsafe extern "C" {} in edition 2024 -- correct form for foreign declarations#[unsafe(no_mangle)] in edition 2024 -- correct form for symbol exportOption for nullable callbacks -- niche optimization guaranteedOption> for nullable pointers -- zero-cost nullable pointer pattern*mut c_void for opaque C types -- standard when internal layout is irrelevantc_void for type-safe opaque pointers -- prevents pointer confusionCStr::from_bytes_with_nul_unchecked with compile-time literal -- safe when literal is known null-terminatedextern "C-unwind" for controlled unwinding -- valid per RFC 2945include!(concat!(env!("OUT_DIR"), "/bindings.rs")) in bindgen crates -- standard patternBox::into_raw / Box::from_raw pairs for ownership transfer -- correct pattern when pairedBefore Submitting Findings
Complete Gates 1-4 in order before reporting any issue; Gate 4 incorporates beagle-rust:review-verification-protocol.