2
\$\begingroup\$

I am trying to expose a simple struct via FFI. Is this the best way to do it?

I am not 100% sure about the get_text function. This is taking the string out of the struct and cloning it before returning a pointer to it. I suspect this is going to leak. Whats the best idiomatic way around this? Do I need to provide a free_string function? Is there a way to just return a pointer into the Rust string?

use std::mem;
use std::os::raw::{c_char, c_void};
use std::ffi;


#[derive(Debug)]
pub struct SomeData {
    text : String,
    number : usize,
}


#[no_mangle]
/// Make the structure. Box it to stick on the heap and return a pointer to it.
/// This absolves Rust from having to deal with the memory, so it is the callers
/// responsibility to free it by calling drop_data.
pub extern "C" fn make_some_data (text: *const c_char, number: usize) -> *mut c_void {
    let cstr = unsafe {
        assert!(!text.is_null());
        ffi::CStr::from_ptr(text)
    };

    let data = cstr.to_str().ok().map(|utf8_str| {
        Box::into_raw(Box::new(SomeData {
            text: String::from(utf8_str),
            number,
        }))
    }).unwrap();

    data as *mut c_void
}

#[no_mangle]
/// Returns the text field of the struct.
pub extern "C" fn get_text(data: *mut c_void) -> *mut c_char {
    let data = unsafe {
        assert!(!data.is_null());
        Box::from_raw(data as *mut SomeData)
    };
    let text = data.text.clone();
    mem::forget(data);
    ffi::CString::new(text).unwrap().into_raw()
}

#[no_mangle]
/// Returns the number field of the struct.
pub extern "C" fn get_number(data: *mut c_void) -> usize {
    let data = unsafe {
        assert!(!data.is_null());
        Box::from_raw(data as *mut SomeData)
    };
    let number = data.number;
    mem::forget(data);
    number
}

#[no_mangle]
/// Frees the memory.
pub extern "C" fn drop_data (data: *mut c_void) {
   mem::drop(data); 
}
\$\endgroup\$
3
\$\begingroup\$

First of all, you should always run clippy:

error: this public function dereferences a raw pointer but is not marked `unsafe`
  --> src/lib.rs:20:29
   |
20 |         ffi::CStr::from_ptr(text)
   |                             ^^^^
   |
   = note: #[deny(clippy::not_unsafe_ptr_arg_deref)] on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref

error: calls to `std::mem::drop` with a value that implements Copy. Dropping a copy leaves the original intact.
  --> src/lib.rs:60:4
   |
60 |    mem::drop(data); 
   |    ^^^^^^^^^^^^^^^
   |
   = note: #[deny(clippy::drop_copy)] on by default
note: argument has type *mut std::ffi::c_void
  --> src/lib.rs:60:14
   |
60 |    mem::drop(data); 
   |              ^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy

drop(data) is indeed totally useless here pointer don't have drop implementation, you must use Box::from_raw(data as *mut SomeData) (BTW you didn't check for null here hope this is what you want). I also advise you to mark these function as unsafe.

For the style there is two sides as you seem to require the pointer is no null:

  • You could assume the pointer is always no null, add only a debug panic
  • You could panic if the pointer is null

I'm in favor of the first option but it's as you like, if you take the second option I advice to use Option<&T> this will prevent you to forget to check if the pointer is null, it's guarantee by Rust that Option<&c_char> will be None for null pointer value.

Your get_number() and get_text() use Box::from_raw(data as *mut SomeData) for nothing, just use directly the pointer or transform it to reference.

I'm unsure void pointer is necessary.

Do I need to provide a free_string function? Is there a way to just return a pointer into the Rust string?

yes of course you must use from_raw()

Final code:

use std::ffi;
use std::os::raw::{c_char, c_void};

#[derive(Debug)]
pub struct SomeData {
    text: String,
    number: usize,
}

#[no_mangle]
pub unsafe extern "C" fn make_some_data(text: Option<*const c_char>, number: usize) -> *mut c_void {
    ffi::CStr::from_ptr(text.unwrap())
        .to_str()
        .map(|str| {
            Box::into_raw(Box::new(SomeData {
                text: str.to_string(),
                number,
            }))
        })
        .unwrap() as *mut c_void
}

#[no_mangle]
pub unsafe extern "C" fn get_text(data: Option<&c_void>) -> *mut c_char {
    let data = my_real_data(data);
    ffi::CString::new(data.text.clone()).unwrap().into_raw()
}

#[no_mangle]
pub unsafe extern "C" fn get_number(data: Option<&c_void>) -> usize {
    let data = my_real_data(data);
    data.number
}

unsafe fn my_real_data(data: Option<&c_void>) -> &SomeData {
    &*(data.unwrap() as *const std::ffi::c_void as *const SomeData)
}

#[no_mangle]
pub unsafe extern "C" fn drop_data(data: Option<*mut c_void>) {
    Box::from_raw(data.unwrap() as *mut SomeData);
}

#[no_mangle]
pub unsafe extern "C" fn drop_text(data: Option<*mut c_char>) {
    ffi::CString::from_raw(data.unwrap());
}
\$\endgroup\$
  • \$\begingroup\$ Thankyou, very useful. I'm a little unclear about what you mean re Option. Are you suggesting the function parameter as fn get_text(data: Option<*const c_void>) instead of fn get_text(data: *mut c_void)? \$\endgroup\$ – Mongus Pong Jul 8 at 15:24
  • \$\begingroup\$ I think you mean data: Option<&mut c_char> instead of Option<*mut c_char> as the parameters to drop_data and drop_text? It compiles but crashes as you have it written there.. \$\endgroup\$ – Mongus Pong Jul 9 at 20:42
  • \$\begingroup\$ @MongusPong Please show the code that make it crash. \$\endgroup\$ – Stargateur Jul 10 at 1:19
  • \$\begingroup\$ It was the code as you had written here. The data was being passed in as None and so the unwrap was panicking. \$\endgroup\$ – Mongus Pong Jul 10 at 19:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.