Hi, so I'm trying to write a wrapper around the following C function:
af_err af_info_string(char **str, const bool verbose)
{
try {
std::string infoStr = getDeviceInfo();
af_alloc_host((void**)str, sizeof(char) * (infoStr.size() + 1));
// Need to do a deep copy
// str.c_str wont cut it
infoStr.copy(*str, infoStr.size());
(*str)[infoStr.size()] = '\0';
} CATCHALL;
return AF_SUCCESS;
}
There are two things - I don't understand why it needs to be a double pointer, rather than single (It seems they always just use the first). And second, I'm just not sure how to correctly write a safe wrapper around that?
In C you represent strings as a char-array which is "equivalent" to a pointer.(Simplified)
So actually you pass a pointer to an array of chars, which is your string.
You need a double pointer because the af_alloc_host call modifies a pointer, so you need a pointer to a pointer to pass to the function so it can modify the pointer. As Chris mentioned, you probably want to do something with CString:
// Something like this
fn info_string(verbose: bool) -> Result<CString, AFErr> {
let tmp = 0 as *mut c_char;
// NOTE: Not sure if unsafe is needed here
let err = unsafe { af_info_string(*mut tmp, verbose ) };
// TODO: error handling on err return value
let ret = unsafe { CString::from_raw(tmp) };
Ok(ret)
}
Based on my very, very cursory reading of ArrayFire's documentation, it may make sense to make your own wrapper around CString that uses ArrayFire's af_dealloc_host to free memory when the variable goes out of scope; Rust then tries to deallocate the memory in the CString, which will probably break because the allocator used by the C code is probably not the same as the one used by the Rust code
Unfortunately, the memory won't be leaked. Instead, something worse would happen - CString would attempt to free the memory allocated by af_alloc_host (probably malloc) using Rust's memory allocator (probably jemalloc). This is a really bad thing.
Also, @cactorium, I'm not sure why use c_void here? Imho this should be enough:
Use Cstr::from_raw, call to_str to obtain &str, then to_owned to get a new String, then free the memory using af_free_host and then return previously created String.
Or if you really want to avoid reallocating the string, create a wrapper type, similar to CString, which would deallocate using af_alloc_free as @cactorium suggested.
Woo, my bad on both points; on the first one I misread the source for ffi/c_str.rs and thought the Drop implementation was nulling out the pointer for some reason, and on the second I misread the type signature of the function because af_alloc_host takes in a void**. Fixed in my previous post
Is it not possible to perform a deep copy of the string such that the new String lives in the rust allocation space? I'm not too concern with speed here.
Not really. The into_string is a method only on CString and it does not perform a deep copy (and probably will result in crash on drop if you constructed the CString from raw parts). The to_str+to_owned on the other hand works on CStr and will perform a deep copy and return a regular String allocated by Rust.
So just to finalize the discussion for anyone having to do this. One interesting thing I've noticed is that you need to explicitely tell Rust that you are letting the ownership away so that it does not deallocate. To make it clear the correct code looks like this:
let mut tmp: *mut c_char = 0 as *mut c_char;
let err_val = af_info_string(&mut tmp, verbose);
HANDLE_ERROR(AfError::from(err_val));
// Take ownership to Rust
let foreign = CString::from_raw(tmp);
// Make string in Rust allocated space
result = foreign.to_str().unwrap().to_owned();
// Transfer ownership out of Rust
tmp = foreign.into_raw();
// Free the foreign buffer
let err_val = af_free_host(tmp as *mut c_void);
HANDLE_ERROR(AfError::from(err_val));
If however, you do not call foerign.into_raw() I guess Rust still thinks that this is its owned space and deallocates it, thus braking hell.
Happy to see you have it working! That's a correct solution, but your use of CString is unnecessary. You could just change CString to CStr and get rid of that into_raw. Btw, both the old and new values of tmp are equal!
Also, your code will do bad things if unwrap panics - the CString will try to deallocate C memory.
Oh, I did not pay attention to CStr. So I Assume you mean something like this: result = CStr::from_ptr(tmp).to_string_lossy().into_owned(); (it also eliminiates the possible panics).