How do you correctly create an owned `String` from `*const c_char`?

I have a c-function that returns a ptr to a string, but I do not know how long it will live, so I want to create a copy of it and return a String in my safe wrapper.

I am currently converting it like this:

use std::os::raw::c_char;

fn take_ownership_string(ptr: *const c_char) -> Option<String> {
    unsafe {
        if ptr.is_null() {
            None
        } else {
            Some(CStr::from_ptr(ptr).to_str().unwrap().to_string())
        }
    }
}

is this the correct way or could this be achieved with fewer conversions?

Seems reasonable to me, but why the unwrap() if you are already returning an Option? You could use .map(str::to_owned) instead. (And I'd suggest to_owned() because it might be slightly more efficient as it doesn't need to go through the Display impl in the absence of specialization).

4 Likes

(And do not forget to mark that conversion as unsafe, since safe Rust code could otherwise do:

take_ownership_string(0xbad as *const c_char) // Uh-oh

)

5 Likes

That changes the behaviour when not utf-8. A silent None makes debugging hard. Using to_string_lossy maybe best; depends how you want to define robust. For performant there is std::str::from_utf8_unchecked.

The implementation looks fine to me.

But yes; making this an unsafe fn instead of using an inner unsafe block would communicate the inherent unsafeness better.

It's not that it "would communicate better". Not marking this function unsafe is simply incorrect, since safe Rust must not ever invoke undefined behavior, and calling this function with non-null dangling pointer is undefined behavior.

4 Likes

Yes, I'm aware, and returning None, which is expected of an Option-returning function, is superior to panicking sometimes, which is absolutely unexpected from a function that seemingly supports graceful error handling.

Citation needed. What's "silent" about that? It's a completely explicit thing, and you can always break on the None case in a debugger…

Well, in this case it might be more correct to return Result<String, TakeOwnershipError>, with TakeOwnershipError being the enum with variants NullPtrError and Utf8Error.

3 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.