as_ptr returns a read-only pointer

[RalfJung]

Add comments to as_ptr methods to warn that these are read-only pointers, and writing to them is UB.

It was pointed out that CStr does not even have an as_mut_ptr. I originally was going to add one, but there is no method at all that would mutate a CStr. Was that a deliberate choice or should I add an as_mut_ptr (similar to what I did for str)?

[rust-highfive]

r? @Kimundi

(rust_highfive has picked a reviewer for you, use r? to override)

[cuviper]

It was pointed out that CStr does not even have an as_mut_ptr.

I believe the correct way is to use CString::into_raw followed by CString::from_raw to recalculate the length, as shown in the example here:
https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw

However, it seems to me this would not know the right length of the allocation to free, no? That doesn't matter for system free, but maybe it would for other allocators...

[RalfJung]

as_mut_ptr would anyway only permit non-length-changing mutations. It's a slice, after all.

[Ixrec]

Unsure if this qualifies as too pedantic or not, but "is never written to" sounds like it'd include the time after you stop using this pointer and any (raw?) reborrows thereof, the same way that (iiuc) constructing a Pin must guarantee the underlying value is never moved again for the rest of the program including after the Pin itself goes away.

Would "is never written to using this pointer" be better, or wrong in other ways?

[RalfJung]

Would "is never written to using this pointer" be better, or wrong in other ways?

"using this pointer or any pointer derived from it" would be correct and should cover your case?

[cuviper]

as_mut_ptr would anyway only permit non-length-changing mutations.

OK... but I expect the most common reason to want *mut c_char is for FFI to write something there, and I doubt that length-preserving would usually be guaranteed. This seems like a giant footgun.

[RalfJung]

How would anyone ever think they can perform length-changing changes in-place? (And the fact that this is in-place should be quite clear with a slice.)

But anyway, this PR does not even propose to add an as_mut_ptr, so I guess we're good?

[cuviper]

You asked if you should add as_mut_ptr, so I guess I'm saying "no". :)

[RalfJung]

Fair. :)

[RalfJung]

Cc @rust-lang/libs -- needs someone to review.

[SimonSapin]

It feels sort of bad to say “don’t do this” in the CStr case without providing an alternative like in other cases. Especially since a typical use case might be to interact with an existing C library whose API cannot easily be changed.

[RalfJung]

Well, this PR just documents the already existing reality. ;)

So are you saying we should also have CStr::as_mut_ptr, but with the requirement that whoever writes to this must not insert interior NULL bytes or break the UTF-8?

[SimonSapin]

(CStr doesn’t have str’s UTF-8 invariant, as shown by CStr::to_str returning a Result.)

Documenting the existing reality sounds good, I just wonder how often this is misused in the wild. Maybe someone more familar with C API conventions can comment: how uncommon is it to write to a caller-allocated, null-terminated buffer?

In the mean time:

@bors r+

[bors]

📌 Commit 30cf0e4 has been approved by SimonSapin