FFI Code Review

Review Workflow

•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
•Check build.rs -- Verify link directives (cargo:rustc-link-lib, cargo:rustc-link-search), bindgen configuration, and C source compilation
•Check extern blocks -- Verify calling conventions, symbol declarations, and safety annotations
•Check type layout -- Every type crossing FFI must be #[repr(C)] or a primitive FFI type
•Check string and pointer handling -- CStr/CString usage, null checks, ownership transfers
•Check callbacks -- extern "C" fn pointers, panic safety across FFI boundary
•Verify before reporting -- Load beagle-rust:review-verification-protocol before submitting findings

Report findings as:

text

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

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

• Foreign function declarations use extern "C" (explicit, not bare extern)
• Edition 2024: extern "C" {} blocks written as unsafe extern "C" {}
• Functions exposed to C use extern "C" fn (not default Rust calling convention)
• Calling convention matches the foreign library ("C", "system" for Win32 API)
• #[link(name = "...")] specifies the correct library name
• #[link(name = "...", kind = "static")] used when statically linking

• Exported functions use #[no_mangle] to preserve symbol names
• Edition 2024: #[no_mangle] written as #[unsafe(no_mangle)]
• Edition 2024: #[export_name = "..."] written as #[unsafe(export_name = "...")]
• #[link_name = "..."] used when Rust name differs from C symbol
• Exported items are pub (only public #[no_mangle] symbols appear in library output)

• Every struct/union crossing FFI has #[repr(C)] -- Rust's default layout is undefined
• Primitive types use std::ffi / std::os::raw equivalents (c_int, c_char, c_void)
• No bare i32 where C uses int -- use c_int (width varies by platform)
• Quirky C types like __be32 use byte arrays ([u8; 4]), not Rust integers
• Enums crossing FFI use #[repr(C)] or #[repr(u8)]/#[repr(u32)] with explicit discriminants
• C-style bitflag enums use a newtype around an integer (or bitflags crate), not a Rust enum
• #[non_exhaustive] on enums representing C enumerations that may gain new values

• C strings use CStr (borrowed) or CString (owned), never &str or String
• CString::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()
• Incoming *const c_char validated with CStr::from_ptr() inside unsafe
• No assumption that C strings are valid UTF-8 -- use to_str() which returns Result
• OS paths use OsStr/OsString and CStr, not &str

• Clear ownership contract: who allocates, who frees
• Rust-allocated memory freed by Rust (Box::from_raw), C-allocated freed by C
• Box::into_raw / Box::from_raw paired correctly for heap transfers
• Vec::into_raw_parts used when passing arrays to C (pointer + length + capacity)
• Destructor functions exposed for every opaque Rust type given to C
• No Drop running on C-allocated memory (and vice versa)

• Callback types are extern "C" fn(...), not closures or fn(...)
• Callbacks use std::panic::catch_unwind to prevent panics from unwinding across FFI
• Callback context passed as *mut c_void with safe reconstruction at call site
• Option<extern "C" fn(...)> used for nullable function pointers (niche optimization)

• Bindgen output reviewed for correctness (auto-generated types may need adjustment)
• -sys crate pattern used for raw bindings, separate crate for safe wrappers
• build.rs uses cargo:rustc-link-lib and cargo:rustc-link-search correctly
• links key in Cargo.toml prevents duplicate linking of the same native library
• Platform-specific bindings generated per-build (not checked in for a single platform)

• Every unsafe block has a // SAFETY: comment explaining invariants
• Every public FFI wrapper function documents safety requirements
• Edition 2024: unsafe fn bodies use explicit unsafe {} blocks around unsafe ops

•Missing #[repr(C)] on types crossing FFI boundary (undefined memory layout)
•Wrong string handling: &str/String where CStr/CString required
•Ownership confusion: freeing C-allocated memory with Rust's allocator (or vice versa)
•Panic unwinding across FFI boundary without catch_unwind
•Using Rust enum for C bitflags (invalid discriminant = undefined behavior)
•Passing closure where extern "C" fn pointer required

•Using i32 instead of c_int for C int (correct on most platforms but not portable)
•Missing #[non_exhaustive] on enums mapping to extensible C enumerations
•Verbose manual bindings where bindgen would be more maintainable
•Checked-in bindings without platform guards

•unsafe extern "C" {} in edition 2024 -- correct form for foreign declarations
•#[unsafe(no_mangle)] in edition 2024 -- correct form for symbol export
•Option<extern "C" fn(...)> for nullable callbacks -- niche optimization guaranteed
•Option<NonNull<T>> for nullable pointers -- zero-cost nullable pointer pattern
•*mut c_void for opaque C types -- standard when internal layout is irrelevant
•Distinct empty structs wrapping c_void for type-safe opaque pointers -- prevents pointer confusion
•CStr::from_bytes_with_nul_unchecked with compile-time literal -- safe when literal is known null-terminated
•extern "C-unwind" for controlled unwinding -- valid per RFC 2945
•include!(concat!(env!("OUT_DIR"), "/bindings.rs")) in bindgen crates -- standard pattern
•Box::into_raw / Box::from_raw pairs for ownership transfer -- correct pattern when paired

Load and follow beagle-rust:review-verification-protocol before reporting any issue.