Tracking issue for CString::{from_ptr, into_ptr}

[alexcrichton]

This is a tracking issue for the unstable cstr_memory feature in the standard library. These methods are likely ready to go as-is modulo naming. Their names should be consistent with Box::{from_raw, into_raw} (currently inconsistent).

cc #27768

[alexcrichton]

This feature is now entering its final comment period for stabilization.

I will post a PR soon for renaming these function to from_raw and into_raw.

[shepmaster]

renaming these function to from_raw and into_raw

Awesome! That makes sense to me.

[bluss]

This rename is important because CStr::from_ptr and CString::from_ptr are so different, but with similar names.

[bluss]

If you use CString::from_ptr on a non-rust allocated thing by mistake, you end up trying to reallocate string literals and things like that. See #27878.

[bluss]

We need to learn from these bugs. Premature dropping of CString leading to dangling pointers, and that some raw pointers transfer ownership and others are "borrowed". Can we use the type system better?

[shepmaster]

@bluss an interesting question. In my mind, once you hit a raw pointer, all bets are off and you have to manage everything yourself. Do you have any thoughts on how to improve that with types?

[geofft]

You can still get some type-safety with raw pointers, though you don't get ownership tracking. I can see a potential API along these lines:

#[repr(i8)] enum RustAllocChar {...}

impl CString {
    fn into_raw(self) -> *const RustAllocChar;
    unsafe fn from_raw(ptr: *const RustAllocChar) -> CString;
}

which lets you write something like

extern {
    fn do_stuff(string: *mut RustAllocChar, free: fn(*mut RustAllocChar));
}

extern fn free(ptr: *mut RustAllocChar) {
    drop(CString::from_raw(ptr));
}

fn main() {
    let s = CString::new("Hello!").unwrap();
    unsafe {do_stuff(s.into_raw(), free)};
}

or for a library

#[no_mangle] // char *mylib_do_stuff(void);
extern fn mylib_do_stuff() -> *mut RustOwnedChar {...}
#[no_mangle] // void mylib_free(char *s);
extern fn mylib_free(s: *mut RustOwnedChar) {...}

So RustAllocChar (or whatever you want to call it -- maybe CChar?) is as valid a binding of C's char as libc::c_char is, but it clearly states that the memory is allocated by Rust: into_raw() no longer returns a type that is indistinguishable from foreign-allocated, otherwise-owned, or static strings. So it's harder to leak it by passing it somewhere where *const c_char was expected, and it's harder to pass the result of .as_ptr() (which is only valid until the CString goes out of scope) where you really wanted .into_ptr(), since they have different types. And it's harder to accidentally free things that shouldn't be freed, because they won't have type *const RustAllocChar.

[geofft]

Also, while I was writing the above, I realized -- shouldn't into_raw return a mutable pointer, not a constant one? It is indeed mutable, in that it's an owned, heap-allocated array of bytes: C code can safely modify it within its length. It is also semantically common for C APIs to use mutable pointers vs. const pointers to indicate ownership: for instance, void free(void *ptr) takes a mutable pointer, so if you only have a constant pointer, you know you can't free it and therefore you don't own it. So if you're "transfer[ring] ownership of the string to a C caller", as our docs say, the C code is probably expecting a mutable pointer regardless of whether it intends to mutate it.

Similarly, from_raw should take a mutable pointer, since C APIs are probably happy to free the string by passing it back as a mutable pointer (since the usual way they free strings is free). This would encourage good API design and also discourage things like from_rawing a string literal, but it's not strictly necessary.

[EDIT: Box::from_raw and into_raw return and take mutable pointers, respectively, so this change would match their API.]

[shepmaster]

but it clearly states that the memory is allocated by Rust

I disagree. There's no reason to assume that a *const Foo was allocated by Rust or the system allocator or any other allocator.

a type that is indistinguishable from foreign-allocated, otherwise-owned, or static strings

The problem is that we are dealing with C /FFI and these types are indistinguishable.

I think that the additional cognitive overhead of having to remember that a RustAllocChar can be coerced to a *const c_char would also be annoying.

It also sound like you are trying to use these methods inside of Rust code to call FFI code. That's definitely not the intended use case, as you don't want to have a one-way transfer of ownership due to the reallocation requirements.

Box::from_raw and into_raw return and take mutable pointers, respectively

That's a good reason to change them, and it was probably a mistake on my part originally. Honestly, the fluidity of const / mut with raw pointers always makes me treat it as a second-class citizen. ^_^

[geofft]

I disagree. There's no reason to assume that a *const Foo was allocated by Rust or the system allocator or any other allocator. [...] The problem is that we are dealing with C /FFI and these types are indistinguishable.

Maybe I was unclear, but this depends specifically on annotating types with a *const TheseBytesWereAllocatedByRust, not any old *const Foo, and have a guarantee that TheseBytesWereAllocatedByRust is only instantiated by CString (sorta like how we have a guarantee that, e.g., str is UTF-8, despite it having the exact same representation as [u8]). This static guarantee is powered by it being a non-instantiable enum, so only unsafe code can generate or consume pointers to it.

Obviously this requires defining the FFI interfaces to use these types when appropriate, and not use them when inappropriate. I don't think that's too hard (we just need to pick a less stupid name than the ones I've come up with).

To be fair, it is confusing that I'm distinguishing the type of the pointed-to thing, not the type of the pointer, when it is semantically a different sort of (raw) pointer to the same c_char. It would be nice if the FFI bindings could take a RustAllocPtr<c_char>, or perhaps even a CString directly. But I think there's no way to make that FFI-safe since you'd be wrapping the pointer with a struct, and struct {char *} and char * aren't interchangeable (they're passed differently on the x86-32 System V ABI, in particular). If there's a way to newtype a primitive type without wrapping it with a struct, that would be awesome. But short of that, I'm doing a newtype on the target of the pointer, since all data pointers have the same ABI (struct {char} * and char * are interchangeable on all platforms Rust is portable to).

I think that the additional cognitive overhead of having to remember that a RustAllocChar can be coerced to a *const c_char would also be annoying.

I'm not clear on when you'd need to coerce it, in Rust code. The only purpose of a *mut RustAllocChar is to be immediately converted to/from CString. If you need a *const c_char (which represents some sort of borrow), in Rust, go to the CString first and then borrow from it. While transmuting from *mut RustAllocChar to *const c_char is safe to do, it probably represents a memory leak.

It also sound like you are trying to use these methods inside of Rust code to call FFI code. That's definitely not the intended use case, as you don't want to have a one-way transfer of ownership due to the reallocation requirements.

Yeah, drop my first example with the callback because that's more confusing than it's worth. What I have in mind is implementing a C API contract that says something like, "This function returns a pointer. When you are done with it, free it by calling this other function," as you would do if you were implementing a C-ABI plugin spec, or writing a C-ABI library in Rust. Your example from #25777 does this. If I added both proper const usage (in Rust and C) and the newtype I'm suggesting (in Rust), you'd get:

#[no_mangle]
pub extern fn reverse(s: *const libc::c_char) -> *mut RustAllocChar {
    let s = unsafe { CStr::from_ptr(s) };
    let s2 = s.to_str().unwrap();
    let s3: String = s2.chars().rev().collect();
    let s4 = CString::new(s3).unwrap();
    s4.into_ptr()
}

#[no_mangle]
pub extern fn cleanup(s: *mut RustAllocChar) {
    unsafe { CString::from_ptr(s) };
}

#include <stdio.h>

extern char *reverse(const char *);
extern void cleanup(char *);

int main() {
  char *s = reverse("Hello, world!");
  printf("%s\n", s);
  cleanup(s);
}

which, at least on the Rust side, makes it clear what pointers are owned and which are borrowed. On the C side, the API docs or header files would probably have a comment on reverse() saying something like "The caller owns the resulting pointer and must free it by calling cleanup()", which conveys the same thing as the type, except in prose (which is basically how life in C goes).

fluidity of const / mut with raw pointers

Only when you're defining the FFI. Rust requires you to do an unsafe cast to turn const to mut, and C compilers give you a warning if you do it as an implicit cast. (This is what allows C programmers to have the discipline of owned-pointers-are-mutable-pointers + make-every-pointer-const-if-possible, since you can only implicitly cast mutable pointers to void * when calling free.)

Incidentally, the fact that C literals have type char * instead of const char * is a stupid historical artifact dating from C's need to be backwards-compatible with code from literally before const was invented. Good C code should treat literals as const char * as far as possible.

[alexcrichton]

@geofft

I agree with changing {into,from}_raw to taking *mut T instead of a *const T, but I'm less sure about adding a custom type to represent a Rust-owned character. You mention that "only unsafe code can generate or consume pointers to it" but can't you have safe (in terms of Rust) code that does 1234 as *mut RustAllocChar? In that sense these sorts of raw pointers can be manufactured at any time it seems like.

In general I feel the extra cognitive overhead of having an extra type just for this return value may not be worth the little bit of extra safety we get. For example there's still the case where Box::into_raw will return types like *mut i32 which can then be passed down into C (for example), and we probably wouldn't want to wrap that up an an extra wrapper.

[geofft]

You mention that "only unsafe code can generate or consume pointers to it" but can't you have safe (in terms of Rust) code that does 1234 as *mut RustAllocChar? In that sense these sorts of raw pointers can be manufactured at any time it seems like.

... sigh, yes, you're right. OK, I withdraw that.

So if the answer is that there's no practical way to increase the type-safety of these functions, that's fine. And from_raw taking a *mut c_char gets us most of the way there, anyway.

[aturon]

@gankro Some of the above discussion makes me think about your plans with Owned/Shared -- did you have a draft for those?