Skip to content
Snippets Groups Projects
  • Daniel Henry-Mantilla's avatar
    27d86048
    Fix Rust bindings (#437) · 27d86048
    Daniel Henry-Mantilla authored
    - A bool coming from C cannot be trusted; functionally it is a c_int,
        and conversion to bool must be done by comparing to 0;
    
      - similarly, a `#[repr(C)]` enum cannot be trusted to be a `u32`;
        it is more often than not a `c_int`. This could cause breakage with
        the ABI of the C functions of the binding. So the signatures and
        enum definitions have been adapted based on the
        `include/keystone/keystone.h` definitions.
    
      - and most importantly: a ks_handle is a `ks_engine *` in C parlance,
        and using `size_t` to represent it is not the right way. Moreover,
        so doing prevents the `Sync` and `Send` traits from being
        auto-unimplemented, meaning that this is implicitly asserting that
        keystone is multithread-safe, even for the same instance. If that is
        the case, then a `unsafe impl Sync for Keystone {}` (ditto for
        `Send`) should be added to make such assertion explicit.
    
        That's why an opaque type using the classic Rust idiom of a
        zero_sized #[repr(C)] struct has been made (while waiting for Rust
        to feature external types), and `Option<ptr::NonNull<_>>` is being
        used as the pointer type (equivalent to *mut _), but it allows to
        communicate nullable / non-nullable invariants at the type-level.
        This has then been transposed to the internals of keystone-rs.
    Fix Rust bindings (#437)
    Daniel Henry-Mantilla authored
    - A bool coming from C cannot be trusted; functionally it is a c_int,
        and conversion to bool must be done by comparing to 0;
    
      - similarly, a `#[repr(C)]` enum cannot be trusted to be a `u32`;
        it is more often than not a `c_int`. This could cause breakage with
        the ABI of the C functions of the binding. So the signatures and
        enum definitions have been adapted based on the
        `include/keystone/keystone.h` definitions.
    
      - and most importantly: a ks_handle is a `ks_engine *` in C parlance,
        and using `size_t` to represent it is not the right way. Moreover,
        so doing prevents the `Sync` and `Send` traits from being
        auto-unimplemented, meaning that this is implicitly asserting that
        keystone is multithread-safe, even for the same instance. If that is
        the case, then a `unsafe impl Sync for Keystone {}` (ditto for
        `Send`) should be added to make such assertion explicit.
    
        That's why an opaque type using the classic Rust idiom of a
        zero_sized #[repr(C)] struct has been made (while waiting for Rust
        to feature external types), and `Option<ptr::NonNull<_>>` is being
        used as the pointer type (equivalent to *mut _), but it allows to
        communicate nullable / non-nullable invariants at the type-level.
        This has then been transposed to the internals of keystone-rs.