Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: Implement CString-related RFCs #22482

Merged
merged 1 commit into from
Feb 19, 2015
Merged

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of RFC 592 and RFC 840. These
two RFCs tweak the behavior of CString and add a new CStr unsized slice type
to the module.

The new CStr type is only constructable via two methods:

  1. By deref'ing from a CString
  2. Unsafely via CStr::from_ptr

The purpose of CStr is to be an unsized type which is a thin pointer to a
libc::c_char (currently it is a fat pointer slice due to implementation
limitations). Strings from C can be safely represented with a CStr and an
appropriate lifetime as well. Consumers of &CString should now consume &CStr
instead to allow producers to pass in C-originating strings instead of just
Rust-allocated strings.

A new constructor was added to CString, new, which takes T: IntoBytes
instead of separate from_slice and from_vec methods (both have been
deprecated in favor of new). The new method returns a Result instead of
panicking. The error variant contains the relevant information about where the
error happened and bytes (if present). Conversions are provided to the
io::Error and old_io::IoError types via the FromError trait which
translate to InvalidInput.

This is a breaking change due to the modification of existing #[unstable] APIs
and new deprecation, and more detailed information can be found in the two RFCs.
Notable breakage includes:

  • All construction of CString now needs to use new and handle the outgoing
    Result.
  • Usage of CString as a byte slice now explicitly needs a .as_bytes() call.
  • The as_slice* methods have been removed in favor of just having the
    as_bytes* methods.

Closes #22469
Closes #22470
[breaking-change]

@rust-highfive
Copy link
Collaborator

r? @huonw

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

@alexcrichton
Copy link
Member Author

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned huonw Feb 18, 2015
@aturon
Copy link
Member

aturon commented Feb 18, 2015

Hm, I see you decided against introducing additional constructors. It seems like from_str would be pretty common...

@aturon
Copy link
Member

aturon commented Feb 18, 2015

Likewise from_string.

@aturon
Copy link
Member

aturon commented Feb 18, 2015

Also, did we want to consider #16035 here?

@aturon
Copy link
Member

aturon commented Feb 18, 2015

I suppose that, since this doesn't stabilize the module now, we can do these other changes later on. Other than those, the PR LGTM (and I actually quite like how this played out in io).

@alexcrichton
Copy link
Member Author

I'm a little worried about adding a large number of from_foo constructors (it should probably be done with a trait instead). Another route would possibly be to add an IntoCString trait with one into_cstring method that's defined for strs, strings, byte slices, vectors, etc. That would, however, require 2 imports instead of the usual one to make the type usable (IntoCString + CString).

I did in general yeah feel a little uncomfortable marking these changes as #[stable] just yet for these reasons. I don't mind exploring some other possibilities in the meantime though.

To make progress on #16035 I think that we would need to #[deprecate] the as_ptr method. I think we would want to do some more analysis in this area first, though. In switching the return type I immediately get 10 errors in the standard library for patterns like:

let ptr = opt_cstr.map(|s| s.as_ptr()).unwrap_or(0 as *const _);

In general I'm pretty wary about #16035 due to our forays to "make unsafe APIs a little safer" largely being unsuccessful. I do agree, though, that if we do it now is the time. I would probably personally vote to not do it myself.

@alexcrichton
Copy link
Member Author

I have updated with a new CString::new constructor that @aturon and I talked about offline.

re-r? @aturon

This commit is an implementation of [RFC 592][r592] and [RFC 840][r840]. These
two RFCs tweak the behavior of `CString` and add a new `CStr` unsized slice type
to the module.

[r592]: https://github.com/rust-lang/rfcs/blob/master/text/0592-c-str-deref.md
[r840]: https://github.com/rust-lang/rfcs/blob/master/text/0840-no-panic-in-c-string.md

The new `CStr` type is only constructable via two methods:

1. By `deref`'ing from a `CString`
2. Unsafely via `CStr::from_ptr`

The purpose of `CStr` is to be an unsized type which is a thin pointer to a
`libc::c_char` (currently it is a fat pointer slice due to implementation
limitations). Strings from C can be safely represented with a `CStr` and an
appropriate lifetime as well. Consumers of `&CString` should now consume `&CStr`
instead to allow producers to pass in C-originating strings instead of just
Rust-allocated strings.

A new constructor was added to `CString`, `new`, which takes `T: IntoBytes`
instead of separate `from_slice` and `from_vec` methods (both have been
deprecated in favor of `new`). The `new` method returns a `Result` instead of
panicking.  The error variant contains the relevant information about where the
error happened and bytes (if present). Conversions are provided to the
`io::Error` and `old_io::IoError` types via the `FromError` trait which
translate to `InvalidInput`.

This is a breaking change due to the modification of existing `#[unstable]` APIs
and new deprecation, and more detailed information can be found in the two RFCs.
Notable breakage includes:

* All construction of `CString` now needs to use `new` and handle the outgoing
  `Result`.
* Usage of `CString` as a byte slice now explicitly needs a `.as_bytes()` call.
* The `as_slice*` methods have been removed in favor of just having the
  `as_bytes*` methods.

Closes rust-lang#22469
Closes rust-lang#22470
[breaking-change]
@aturon
Copy link
Member

aturon commented Feb 18, 2015

@bors: r+ 1860ee5

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 19, 2015
This commit is an implementation of [RFC 592][r592] and [RFC 840][r840]. These
two RFCs tweak the behavior of `CString` and add a new `CStr` unsized slice type
to the module.

[r592]: https://github.com/rust-lang/rfcs/blob/master/text/0592-c-str-deref.md
[r840]: https://github.com/rust-lang/rfcs/blob/master/text/0840-no-panic-in-c-string.md

The new `CStr` type is only constructable via two methods:

1. By `deref`'ing from a `CString`
2. Unsafely via `CStr::from_ptr`

The purpose of `CStr` is to be an unsized type which is a thin pointer to a
`libc::c_char` (currently it is a fat pointer slice due to implementation
limitations). Strings from C can be safely represented with a `CStr` and an
appropriate lifetime as well. Consumers of `&CString` should now consume `&CStr`
instead to allow producers to pass in C-originating strings instead of just
Rust-allocated strings.

A new constructor was added to `CString`, `new`, which takes `T: IntoBytes`
instead of separate `from_slice` and `from_vec` methods (both have been
deprecated in favor of `new`). The `new` method returns a `Result` instead of
panicking.  The error variant contains the relevant information about where the
error happened and bytes (if present). Conversions are provided to the
`io::Error` and `old_io::IoError` types via the `FromError` trait which
translate to `InvalidInput`.

This is a breaking change due to the modification of existing `#[unstable]` APIs
and new deprecation, and more detailed information can be found in the two RFCs.
Notable breakage includes:

* All construction of `CString` now needs to use `new` and handle the outgoing
  `Result`.
* Usage of `CString` as a byte slice now explicitly needs a `.as_bytes()` call.
* The `as_slice*` methods have been removed in favor of just having the
  `as_bytes*` methods.

Closes rust-lang#22469
Closes rust-lang#22470
[breaking-change]
@bors bors merged commit 1860ee5 into rust-lang:master Feb 19, 2015
@alexcrichton alexcrichton deleted the cstr-changes branch March 27, 2015 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants