Skip to content

Commit

Permalink
deprecating CStr::as_ptr in favor of with_ptr
Browse files Browse the repository at this point in the history
  • Loading branch information
durka committed Jun 6, 2016
1 parent 8fb8742 commit d90c45c
Showing 1 changed file with 126 additions and 0 deletions.
126 changes: 126 additions & 0 deletions text/0000-cstr-with-ptr.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
- Feature Name: `cstr_with_ptr`
- Start Date: 2016-06-06
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Deprecate `CStr::as_ptr`, which returns a raw pointer and thereby invites unsound code, in favor of
a new method, `CStr::with_ptr`, which passes the pointer to a user-provided closure.

# Motivation
[motivation]: #motivation

`CString` wraps a C string and provides easy, safe access to the raw pointer to the string, for
FFI interop. The safety is correct, as there's nothing unsound about having a raw pointer around
until you dereference it. However, raw pointers are not tracked by the borrow checker, and once you
have one it's as easy in Rust as it is in C to keep it around too long (namely, after the backing
`CString` is dropped) and have a dangling pointer, which leads to unsound code.

There are [hundreds of projects](https://users.rust-lang.org/t/you-should-stop-telling-people-that-safe-rust-is-always-safe/6094/7)
including a call like `CString::new(...).unwrap().as_ptr()`, which is evidence that this UB
mistake is widespread.

By changing the API from a method _returning_ a raw pointer to one that runs a user-provided
closure, we make it easier to write sound code than to write unsound code. Sure, you can still
have the pointer escape the closure (see [Drawbacks](#drawbacks)), but it's a clear choice. The
API change tips the scales towards soundness.

# Detailed design
[design]: #detailed-design

1. Deprecate `CStr::as_ptr`, with the following message: "too easy to misuse because it returns a
raw pointer; use `with_ptr` instead". The method is stable, so it will not be removed in any 1.x
release.

2. Add the following method to `CStr`:

```rust
/// Calls the provided closure, passing the inner pointer to this C string.
///
/// The pointer will be valid for as long as `self` is and points to a contiguous region of
/// memory terminated with a 0 byte to represent the end of the string.
#[allow(deprecated)] // for the as_ptr call
fn with_ptr<F: FnOnce(*const c_char)>(&self, f: F) {
f(self.as_ptr());
}
```

For example usage see [this playpen](https://play.rust-lang.org/?gist=b6b1495ebee03fea679e95acb6b51ed6).

3. Modify the `CStr` and `CString` examples that use `as_ptr` to use `with_ptr` instead.

`CString::new` has the following example code:

```rust
use std::ffi::CString;
use std::os::raw::c_char;

extern { fn puts(s: *const c_char); }

fn main() {
let to_print = CString::new("Hello!").unwrap();
unsafe {
puts(to_print.as_ptr());
}
}
```

Under this proposal, it would change to:

```rust
use std::ffi::CString;
use std::os::raw::c_char;

extern { fn puts(s: *const c_char); }

fn main() {
let to_print = CString::new("Hello!").unwrap();
to_print.with_ptr(|p| unsafe {
puts(p);
});
}
```

There are nearly identical examples on `CString` and `CStr` themselves which would change in the
same way.

# Drawbacks
[drawbacks]: #drawbacks

- It deprecates another stable method, which contributes to perception of API churn.
- It adds surface area to the API of `CStr`.
- It's still rather easy to circumvent the help and write unsound code by "leaking" the pointer out
of the closure:

```rust
let mut ptr = ptr::null();
{
let s = CString::new("foo").unwrap();
s.with_ptr(|p| { ptr = p; });
}
/* ptr is now dangling */
```

# Alternatives
[alternatives]: #alternatives

- Do nothing. It remains very easy to write unsound code using `CString` and `CStr::as_ptr`.
- Move the `temporary_cstring_as_ptr` lint, which warns on the most common way to write said unsound
code (calling `as_ptr` on a temporary `CString`) from Clippy to rustc.

# Unresolved questions
[unresolved]: #unresolved-questions

- Should `with_ptr` allow the closure to return a value? It could be

```rust
fn with_ptr<F: FnOnce(*const c_char) -> O, O>(&self, f: F) -> O {
f(self.as_ptr())
}
```

which might be convenient but would make it easier to "leak" the pointer (as easy as
`let ptr = s.with_ptr(|p| p);`).

0 comments on commit d90c45c

Please sign in to comment.