Skip to content

Prefer raw::c_char or libc::c_char and fix arm #29775

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 11, 2015
Merged

Prefer raw::c_char or libc::c_char and fix arm #29775

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc60b2c
Prefer raw::c_char or libc::c_char and fix arm
arcnmx Nov 11, 2015
f9f7347
Cleaner c_char cfg logic
arcnmx Nov 11, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 30 additions & 37 deletions src/libstd/ffi/c_str.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use libc;
use mem;
use ops::Deref;
use option::Option::{self, Some, None};
use os::raw::c_char;
use result::Result::{self, Ok, Err};
use slice;
use str::{self, Utf8Error};
Expand All @@ -36,23 +37,21 @@ use vec::Vec;
///
/// A `CString` is created from either a byte slice or a byte vector. After
/// being created, a `CString` predominately inherits all of its methods from
/// the `Deref` implementation to `[libc::c_char]`. Note that the underlying
/// array is represented as an array of `libc::c_char` as opposed to `u8`. A
/// the `Deref` implementation to `[os::raw::c_char]`. Note that the underlying
/// array is represented as an array of `os::raw::c_char` as opposed to `u8`. A
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine to omit the os::raw:: in these two and just mention c_char.

/// `u8` slice can be obtained with the `as_bytes` method. Slices produced from
/// a `CString` do *not* contain the trailing nul terminator unless otherwise
/// specified.
///
/// # Examples
///
/// ```no_run
/// # #![feature(libc)]
/// # extern crate libc;
/// # fn main() {
/// use std::ffi::CString;
/// use libc;
/// use std::os::raw::c_char;
///
/// extern {
/// fn my_printer(s: *const libc::c_char);
/// fn my_printer(s: *const c_char);
/// }
///
/// let c_to_print = CString::new("Hello, world!").unwrap();
Expand Down Expand Up @@ -83,11 +82,10 @@ pub struct CString {
/// Inspecting a foreign C string
///
/// ```no_run
/// # #![feature(libc)]
/// extern crate libc;
/// use std::ffi::CStr;
/// use std::os::raw::c_char;
///
/// extern { fn my_string() -> *const libc::c_char; }
/// extern { fn my_string() -> *const c_char; }
///
/// fn main() {
/// unsafe {
Expand All @@ -100,12 +98,11 @@ pub struct CString {
/// Passing a Rust-originating C string
///
/// ```no_run
/// # #![feature(libc)]
/// extern crate libc;
/// use std::ffi::{CString, CStr};
/// use std::os::raw::c_char;
///
/// fn work(data: &CStr) {
/// extern { fn work_with(data: *const libc::c_char); }
/// extern { fn work_with(data: *const c_char); }
///
/// unsafe { work_with(data.as_ptr()) }
/// }
Expand All @@ -119,11 +116,10 @@ pub struct CString {
/// Converting a foreign C string into a Rust `String`
///
/// ```no_run
/// # #![feature(libc)]
/// extern crate libc;
/// use std::ffi::CStr;
/// use std::os::raw::c_char;
///
/// extern { fn my_string() -> *const libc::c_char; }
/// extern { fn my_string() -> *const c_char; }
///
/// fn my_string_safe() -> String {
/// unsafe {
Expand All @@ -139,10 +135,10 @@ pub struct CString {
#[stable(feature = "rust1", since = "1.0.0")]
pub struct CStr {
// FIXME: this should not be represented with a DST slice but rather with
// just a raw `libc::c_char` along with some form of marker to make
// just a raw `c_char` along with some form of marker to make
// this an unsized type. Essentially `sizeof(&CStr)` should be the
// same as `sizeof(&c_char)` but `CStr` should be an unsized type.
inner: [libc::c_char]
inner: [c_char]
}

/// An error returned from `CString::new` to indicate that a nul byte was found
Expand All @@ -169,11 +165,10 @@ impl CString {
/// # Examples
///
/// ```no_run
/// # #![feature(libc)]
/// extern crate libc;
/// use std::ffi::CString;
/// use std::os::raw::c_char;
///
/// extern { fn puts(s: *const libc::c_char); }
/// extern { fn puts(s: *const c_char); }
///
/// fn main() {
/// let to_print = CString::new("Hello!").unwrap();
Expand Down Expand Up @@ -220,7 +215,7 @@ impl CString {
#[unstable(feature = "cstr_memory2", reason = "recently added",
issue = "27769")]
#[deprecated(since = "1.4.0", reason = "renamed to from_raw")]
pub unsafe fn from_ptr(ptr: *const libc::c_char) -> CString {
pub unsafe fn from_ptr(ptr: *const c_char) -> CString {
CString::from_raw(ptr as *mut _)
}

Expand All @@ -230,7 +225,7 @@ impl CString {
/// `into_raw`. The length of the string will be recalculated
/// using the pointer.
#[stable(feature = "cstr_memory", since = "1.4.0")]
pub unsafe fn from_raw(ptr: *mut libc::c_char) -> CString {
pub unsafe fn from_raw(ptr: *mut c_char) -> CString {
let len = libc::strlen(ptr) + 1; // Including the NUL byte
let slice = slice::from_raw_parts(ptr, len as usize);
CString { inner: mem::transmute(slice) }
Expand All @@ -247,7 +242,7 @@ impl CString {
#[unstable(feature = "cstr_memory2", reason = "recently added",
issue = "27769")]
#[deprecated(since = "1.4.0", reason = "renamed to into_raw")]
pub fn into_ptr(self) -> *const libc::c_char {
pub fn into_ptr(self) -> *const c_char {
self.into_raw() as *const _
}

Expand All @@ -260,8 +255,8 @@ impl CString {
///
/// Failure to call `from_raw` will lead to a memory leak.
#[stable(feature = "cstr_memory", since = "1.4.0")]
pub fn into_raw(self) -> *mut libc::c_char {
Box::into_raw(self.inner) as *mut libc::c_char
pub fn into_raw(self) -> *mut c_char {
Box::into_raw(self.inner) as *mut c_char
}

/// Converts the `CString` into a `String` if it contains valid Unicode data.
Expand Down Expand Up @@ -426,15 +421,13 @@ impl CStr {
/// # Examples
///
/// ```no_run
/// # #![feature(libc)]
/// # extern crate libc;
/// # fn main() {
/// use std::ffi::CStr;
/// use std::os::raw::c_char;
/// use std::str;
/// use libc;
///
/// extern {
/// fn my_string() -> *const libc::c_char;
/// fn my_string() -> *const c_char;
/// }
///
/// unsafe {
Expand All @@ -445,7 +438,7 @@ impl CStr {
/// # }
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub unsafe fn from_ptr<'a>(ptr: *const libc::c_char) -> &'a CStr {
pub unsafe fn from_ptr<'a>(ptr: *const c_char) -> &'a CStr {
let len = libc::strlen(ptr);
mem::transmute(slice::from_raw_parts(ptr, len as usize + 1))
}
Expand All @@ -456,7 +449,7 @@ impl CStr {
/// to a contiguous region of memory terminated with a 0 byte to represent
/// the end of the string.
#[stable(feature = "rust1", since = "1.0.0")]
pub fn as_ptr(&self) -> *const libc::c_char {
pub fn as_ptr(&self) -> *const c_char {
self.inner.as_ptr()
}

Expand Down Expand Up @@ -560,14 +553,14 @@ impl ToOwned for CStr {
mod tests {
use prelude::v1::*;
use super::*;
use libc;
use os::raw::c_char;
use borrow::Cow::{Borrowed, Owned};
use hash::{SipHasher, Hash, Hasher};

#[test]
fn c_to_rust() {
let data = b"123\0";
let ptr = data.as_ptr() as *const libc::c_char;
let ptr = data.as_ptr() as *const c_char;
unsafe {
assert_eq!(CStr::from_ptr(ptr).to_bytes(), b"123");
assert_eq!(CStr::from_ptr(ptr).to_bytes_with_nul(), b"123\0");
Expand Down Expand Up @@ -616,13 +609,13 @@ mod tests {
#[test]
fn to_str() {
let data = b"123\xE2\x80\xA6\0";
let ptr = data.as_ptr() as *const libc::c_char;
let ptr = data.as_ptr() as *const c_char;
unsafe {
assert_eq!(CStr::from_ptr(ptr).to_str(), Ok("123…"));
assert_eq!(CStr::from_ptr(ptr).to_string_lossy(), Borrowed("123…"));
}
let data = b"123\xE2\0";
let ptr = data.as_ptr() as *const libc::c_char;
let ptr = data.as_ptr() as *const c_char;
unsafe {
assert!(CStr::from_ptr(ptr).to_str().is_err());
assert_eq!(CStr::from_ptr(ptr).to_string_lossy(), Owned::<str>(format!("123\u{FFFD}")));
Expand All @@ -632,7 +625,7 @@ mod tests {
#[test]
fn to_owned() {
let data = b"123\0";
let ptr = data.as_ptr() as *const libc::c_char;
let ptr = data.as_ptr() as *const c_char;

let owned = unsafe { CStr::from_ptr(ptr).to_owned() };
assert_eq!(owned.as_bytes_with_nul(), data);
Expand All @@ -641,7 +634,7 @@ mod tests {
#[test]
fn equal_hash() {
let data = b"123\xE2\xFA\xA6\0";
let ptr = data.as_ptr() as *const libc::c_char;
let ptr = data.as_ptr() as *const c_char;
let cstr: &'static CStr = unsafe { CStr::from_ptr(ptr) };

let mut s = SipHasher::new_with_keys(0, 0);
Expand Down
8 changes: 6 additions & 2 deletions src/libstd/os/raw.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@

#![stable(feature = "raw_os", since = "1.1.0")]

#[cfg(any(target_arch = "aarch64", target_os = "android"))]
#[cfg(all(not(target_os = "ios"), not(target_os = "macos"),
any(target_arch = "aarch64", target_arch = "arm",
target_os = "android")))]
#[stable(feature = "raw_os", since = "1.1.0")] pub type c_char = u8;
#[cfg(not(any(target_arch = "aarch64", target_os = "android")))]
#[cfg(any(target_os = "ios", target_os = "macos",
not(any(target_arch = "aarch64", target_arch = "arm",
target_os = "android"))))]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope all this is correct... ugh.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least going by the current definition of liblibc, the type is unsigned for linux arm, linux aarch64, and android everywhere. Perhaps the ios/macos clause could be turned into a linux-specific clause?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, k, so if I have the logic right... everything is signed, exceptions are: linux aarch64 and arm, and android? The main thing I wasn't sure about were ARM BSDs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm not entirely sure about BSDs, but they can always be fixed once we gain support!

#[stable(feature = "raw_os", since = "1.1.0")] pub type c_char = i8;
#[stable(feature = "raw_os", since = "1.1.0")] pub type c_schar = i8;
#[stable(feature = "raw_os", since = "1.1.0")] pub type c_uchar = u8;
Expand Down