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

Conversation

Projects
None yet
5 participants
@alexcrichton
Copy link
Member

commented Feb 18, 2015

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

This comment has been minimized.

Copy link
Collaborator

commented Feb 18, 2015

r? @huonw

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

@alexcrichton

This comment has been minimized.

Copy link
Member Author

commented Feb 18, 2015

r? @aturon

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

@aturon

This comment has been minimized.

Copy link
Member

commented Feb 18, 2015

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

@aturon

This comment has been minimized.

Copy link
Member

commented Feb 18, 2015

Likewise from_string.

@aturon

This comment has been minimized.

Copy link
Member

commented Feb 18, 2015

Also, did we want to consider #16035 here?

@aturon

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

commented Feb 18, 2015

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 alexcrichton force-pushed the alexcrichton:cstr-changes branch from 11b1916 to 9f414ea Feb 18, 2015

@alexcrichton

This comment has been minimized.

Copy link
Member Author

commented Feb 18, 2015

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

re-r? @aturon

@alexcrichton alexcrichton force-pushed the alexcrichton:cstr-changes branch from 9f414ea to 1860ee5 Feb 18, 2015

std: Implement CString-related RFCs
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 #22469
Closes #22470
[breaking-change]
@aturon

This comment has been minimized.

Copy link
Member

commented Feb 18, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 19, 2015

rollup merge of rust-lang#22482: alexcrichton/cstr-changes
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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexcrichton alexcrichton deleted the alexcrichton:cstr-changes branch Mar 27, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.