Blog Post: How To RiiR

This is my take on how to successfully transition a codebase to Rust.

What do you think? Are there different ways I could have tacked the issue?

17 Likes

Love the closing video!

2 Likes

That was a really neat blog post, with quite a few ergonomic tricks in the middle, and showing how to deal with many FFI issues :slight_smile: Congrats! :ok_hand:


That being said, I have two remarks:

  • there is no mention about panic!-ing (or rather, the needed lack thereof) across FFI boundaries, and how catch_unwind!, [profile.{debug,release}] panic = "abort" or an abort-on-drop bomb should be added when a extern "C" fn calls Rust code that may panic!.

  • /* let _: *mut _ = foo; // function param */
    if foo.is_null() { ... }
    let foo: &mut _ = unsafe { &mut *foo };
    

    is better (imho) written as

    /* let _: *mut _ = foo; // function param */
    let foo: &mut _ = unsafe { foo.as_mut() }.unwrap_or_else(|| { ... });

Thanks for pointing this out! I totally forgot about making the code panic-safe. I'll need to go back and update the relevant sections to make sure we talk about catch_unwind().

I'm not sure I agree with you on the second point. Most of the time if there's a null pointer you just want to return, which isn't something you do from a closure... Plus the explicit null check is more boring, which is exactly what I want when writing unsafe code :wink:

1 Like

That's a good point, and one of the reasons it seems it is time to generalize ? and get some form of postfix macro or "elvis operator" allowing custom control flow. Anyways, prefix macros do manage to make such pattern not too bad:

#[no_mangle] pub unsafe extern "C"
fn add_assign (x: *mut c_int, y: *const c_int) -> i8
{
    on_error_do! {
        return -1;
    }
    let x = unwrap!(x.as_mut());
    let y = unwrap!(y.as_ref());
    *x += *y;
    0
}

#[no_mangle] pub unsafe extern "C"
fn max (p: *const c_int, len: size_t) -> c_int
{
    on_error_do! {
        return -2;
    }
    let p = unwrap!(ptr::NonNull::new(p as _));
    let elems: &[c_int] =
        ::core::slice::from_raw_parts(
            p.as_ptr(),
            unwrap!(len.try_into().ok()),
        )
    ;
    elems
        .iter()
        .copied()
        .max()
        .unwrap_or(-1)
}

My reasoning behind using unwrap! rather than doing

if thing.is_null() {
    return ...;
}
let thing = &[mut] *thing;

is that the latter is similar to doing

if thing.is_none() { ... }
let thing = thing.unwrap_or_else(|| unsafe { hint::unreachable_unchecked() });

instead of the pervasive

let thing = if let Some(it) = thing { it } else {
    return ...;
};

With "manual checks", we are not using the type system to enforce the invariants we check.

That being said,

is indeed a valid point: keeping FFI functions simple is an undeniable asset.


By the way, I'll soon be publishing (updating) a crate to allow things like:

#[ffi_export(on_panic = return -1)]
unsafe
fn add_assign (x: *mut i32, y: *const i32) -> i8
{
    let x = x.as_mut().expect("Got NULL x");
    let y = y.as_ref().expect("Got NULL y");
    *x += *y;
    0
}

// or
#[ffi_export(on_panic = abort)]
// or
#[ffi_export(on_panic = |err_msg: &str| {
    eprintln!("Rust code errored: {}", err_msg);
    return -1;
})]
3 Likes