feature: wrap every C function on unsafe blocks

[je-vv]

Please let me know if a "discussion" would be a better fit...

Every C function requires to be wrapped by an unsafe block, otherwise rust complains about it because it considers C unasfe.

What I see getting generate for an arbitrary C function is:

extern "C" {
    #[doc = "C function documentation"]
    pub fn c_function_name(
        arg_1: arg_1__type,
        arg_2: arg_2__type,
    ) -> ::std::os::raw::c_int;
}

When the generator like:

    let bindings = bindgen::Builder::default()
        .header("include/aruba/dp.h")
        .parse_callbacks(Box::new(bindgen::CargoCallbacks))
        .generate()
        .expect("Unable to generate bindings");

The thing with the generated rust bindings, is that one always has to wrap each binding with an unsafe block, since C is considered unsafe...

Note I also tried:

    let bindings = bindgen::Builder::default()
        .header("include/aruba/dp.h")
        .wrap_unsafe_ops(true)
        .parse_callbacks(Box::new(bindgen::CargoCallbacks))
        .generate()
        .expect("Unable to generate bindings");

And that doesn't help, one still needs to wrap bindings with unsafe blocks. The doc for unsafe_ops sort of suggest it only wraps functions considered unsafe on the C context, not any C function.

Does it make sense? Should all C functions be wrapped by default with unsafe blocks, and if wanted, turn that behavior off?

Thanks !

[tgross35]

Could you clarify what your actual output is and what you expect? Are you saying that fn c_function_name should be unsafe fn c_function_name?

[je-vv]

Like

unsafe {
  pub fn c_function_name() -> ...
}

No sure if there, or somewhere else, since that looks more like a declaration. But yes, something like that.

[tgross35]

What problem are you trying to solve? That unsafe block doesn’t do anything.

[je-vv]

To avoid having to manually add the unsafe block every time calling one of the binded C functions. As things are, when calling those functions, if not wrapping them up with unsafe, rust complains because rust considers C funstions unsafe. Besides ending up with a very populated code with unsafe, it could be avoided introducing the unsafe wrappers when creating the bindings.

[tgross35]

Unsafe blocks don't work like that, I think you mean just making the generated functions not unsafe fn - but it is not going to happen anyway. Calling FFI code is inherently unsafe, even for functions that don't work with pointers, just because Rust can't check that its rules for safety are upheld.

Unsafe doesn't mean unsound though, so usually your abstraction layer provides a thin wrapper to make it safe. It is always good practice to explain why an unsafe block is indeed safe, even if there are no preconditions. Usually that looks something like:

pub fn function_name(a: i32, b: i32) -> i32 {
    // SAFETY: FFI call with no preconditions
    unsafe { c_function_name(a, b) }
}

[je-vv]

Well, yes. I was looking for something like that, because when one is binding hundreds of C functions, none of them really unsafe, it's quite a lot of writing unsafe every time one of such functions is called. Automating such wrapping sounds much more better than having to is on every call for C functions...

[ojeda]

If they are truly safe to call (which is rare, but it does happen sometimes), then I agree it could be nice to automate.

However, doing it unconditionally is typically wrong, i.e. the symbols should be manually picked, e.g. from a list/regex passed in a flag or perhaps supporting a [[safe]] attribute or similar on the C side.

Would it be possible to see some of the functions you are dealing with?

[pvdrz]

when one is binding hundreds of C functions, none of them really unsafe

Sorry but they are literally unsafe as you need to hold safety invariants that you don't have to care about when calling non foreign functions.

it's quite a lot of writing unsafe every time one of such functions is called.

This is a feature in my opinion. If you call a foreign function you have to be extra careful anyway, writing that unsafe forces you to think if if the call you're about to make is actually sound. You could follow @tgross35 advice and use a thin wrapper where you can explain on each case why calling a foreign function is safe,

Automating such wrapping sounds much more better than having to is on every call for C functions...

I'm personally opposed to add any functionality that would allow removing unsafe keywords in bulk just for the sake of convenience, others might disagree.