Skip to content

Split CString::new into CString::new and CString::new_owned #18125

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/libnative/io/addrinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn get_error(s: c_int) -> IoError {
use std::c_str::CString;

let err_str = unsafe {
CString::new(gai_strerror(s), false).as_str().unwrap().to_string()
CString::new(gai_strerror(s)).as_str().unwrap().to_string()
};
IoError {
code: s as uint,
Expand Down
4 changes: 2 additions & 2 deletions src/libnative/io/file_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ pub fn readdir(p: &CString) -> IoResult<Vec<CString>> {
use libc::{opendir, readdir_r, closedir};

fn prune(root: &CString, dirs: Vec<Path>) -> Vec<CString> {
let root = unsafe { CString::new(root.as_ptr(), false) };
let root = unsafe { CString::new(root.as_ptr()) };
let root = Path::new(root);

dirs.into_iter().filter(|path| {
Expand All @@ -376,7 +376,7 @@ pub fn readdir(p: &CString) -> IoResult<Vec<CString>> {
while unsafe { readdir_r(dir_ptr, ptr, &mut entry_ptr) == 0 } {
if entry_ptr.is_null() { break }
let cstr = unsafe {
CString::new(rust_list_dir_val(entry_ptr), false)
CString::new(rust_list_dir_val(entry_ptr))
};
paths.push(Path::new(cstr));
}
Expand Down
4 changes: 2 additions & 2 deletions src/libnative/io/file_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ pub fn mkdir(p: &CString, _mode: uint) -> IoResult<()> {

pub fn readdir(p: &CString) -> IoResult<Vec<CString>> {
fn prune(root: &CString, dirs: Vec<Path>) -> Vec<CString> {
let root = unsafe { CString::new(root.as_ptr(), false) };
let root = unsafe { CString::new(root.as_ptr()) };
let root = Path::new(root);

dirs.into_iter().filter(|path| {
Expand All @@ -356,7 +356,7 @@ pub fn readdir(p: &CString) -> IoResult<Vec<CString>> {
}

let star = Path::new(unsafe {
CString::new(p.as_ptr(), false)
CString::new(p.as_ptr())
}).join("*");
let path = try!(to_utf16(&star.to_c_str()));

Expand Down
6 changes: 3 additions & 3 deletions src/librustc/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ pub enum OutputType {
pub fn llvm_err(handler: &diagnostic::Handler, msg: String) -> ! {
unsafe {
let cstr = llvm::LLVMRustGetLastError();
if cstr == ptr::null() {
if cstr as *const i8 == ptr::null() {
handler.fatal(msg.as_slice());
} else {
let err = CString::new(cstr, true);
let err = CString::new_owned(cstr);
let err = String::from_utf8_lossy(err.as_bytes());
handler.fatal(format!("{}: {}",
msg.as_slice(),
Expand Down Expand Up @@ -373,7 +373,7 @@ unsafe extern "C" fn diagnostic_handler(info: DiagnosticInfoRef, user: *mut c_vo

match llvm::diagnostic::Diagnostic::unpack(info) {
llvm::diagnostic::Optimization(opt) => {
let pass_name = CString::new(opt.pass_name, false);
let pass_name = CString::new(opt.pass_name);
let pass_name = pass_name.as_str().expect("got a non-UTF8 pass name from LLVM");
let enabled = match cgcx.remark {
AllPasses => true,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2988,7 +2988,7 @@ fn internalize_symbols(cx: &SharedCrateContext, reachable: &HashSet<String>) {
continue
}

let name = CString::new(llvm::LLVMGetValueName(val), false);
let name = CString::new(llvm::LLVMGetValueName(val));
declared.insert(name);
}
}
Expand All @@ -3004,7 +3004,7 @@ fn internalize_symbols(cx: &SharedCrateContext, reachable: &HashSet<String>) {
continue
}

let name = CString::new(llvm::LLVMGetValueName(val), false);
let name = CString::new(llvm::LLVMGetValueName(val));
if !declared.contains(&name) &&
!reachable.contains_equiv(name.as_str().unwrap()) {
llvm::SetLinkage(val, llvm::InternalLinkage);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,7 @@ extern {

/** Returns a string describing the last error caused by an LLVMRust*
call. */
pub fn LLVMRustGetLastError() -> *const c_char;
pub fn LLVMRustGetLastError() -> *mut c_char;

/// Print the pass timings since static dtors aren't picking them up.
pub fn LLVMRustPrintPassTimings();
Expand Down
92 changes: 53 additions & 39 deletions src/librustrt/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,6 @@ strings can validly contain a null-byte in the middle of the string (0 is a
valid Unicode codepoint). This means that not all Rust strings can actually be
translated to C strings.

# Creation of a C string

A C string is managed through the `CString` type defined in this module. It
"owns" the internal buffer of characters and will automatically deallocate the
buffer when the string is dropped. The `ToCStr` trait is implemented for `&str`
and `&[u8]`, but the conversions can fail due to some of the limitations
explained above.

This also means that currently whenever a C string is created, an allocation
must be performed to place the data elsewhere (the lifetime of the C string is
not tied to the lifetime of the original string/data buffer). If C strings are
heavily used in applications, then caching may be advisable to prevent
unnecessary amounts of allocations.

Be carefull to remember that the memory is managed by C allocator API and not
by Rust allocator API.
That means that the CString pointers should be freed with C allocator API
Expand Down Expand Up @@ -85,13 +71,17 @@ use core::slice;
use core::str;
use libc;

enum CStringInner {
Owned(*mut libc::c_char),
NotOwned(*const libc::c_char),
}

/// The representation of a C String.
///
/// This structure wraps a `*libc::c_char`, and will automatically free the
/// memory it is pointing to when it goes out of scope.
pub struct CString {
buf: *const libc::c_char,
owns_buffer_: bool,
inner: CStringInner,
}

impl Clone for CString {
Expand All @@ -102,19 +92,19 @@ impl Clone for CString {
let len = self.len() + 1;
let buf = unsafe { libc::malloc(len as libc::size_t) } as *mut libc::c_char;
if buf.is_null() { ::alloc::oom() }
unsafe { ptr::copy_nonoverlapping_memory(buf, self.buf, len); }
CString { buf: buf as *const libc::c_char, owns_buffer_: true }
unsafe { ptr::copy_nonoverlapping_memory(buf, self.as_ptr(), len); }
CString { inner: Owned(buf) }
}
}

impl PartialEq for CString {
fn eq(&self, other: &CString) -> bool {
// Check if the two strings share the same buffer
if self.buf as uint == other.buf as uint {
if self.as_ptr() as uint == other.as_ptr() as uint {
true
} else {
unsafe {
libc::strcmp(self.buf, other.buf) == 0
libc::strcmp(self.as_ptr(), other.as_ptr()) == 0
}
}
}
Expand Down Expand Up @@ -144,9 +134,20 @@ impl CString {
///# Failure
///
/// Fails if `buf` is null
pub unsafe fn new(buf: *const libc::c_char, owns_buffer: bool) -> CString {
pub unsafe fn new(buf: *const libc::c_char) -> CString {
assert!(!buf.is_null());
CString { buf: buf, owns_buffer_: owns_buffer }
CString { inner: NotOwned(buf) }
}

/// Like CString::new except that `buf` will be freed when the value goes
/// out of scope.
///
///# Failure
///
/// Fails if `buf` is null
pub unsafe fn new_owned(buf: *mut libc::c_char) -> CString {
assert!(!buf.is_null());
CString { inner: Owned(buf) }
}

/// Return a pointer to the NUL-terminated string data.
Expand Down Expand Up @@ -179,8 +180,12 @@ impl CString {
/// }
/// }
/// ```
#[inline]
pub fn as_ptr(&self) -> *const libc::c_char {
self.buf
match self.inner {
Owned(buf) => buf as *const libc::c_char,
NotOwned(buf) => buf,
}
}

/// Return a mutable pointer to the NUL-terminated string data.
Expand All @@ -200,21 +205,28 @@ impl CString {
/// // wrong (the CString will be freed, invalidating `p`)
/// let p = foo.to_c_str().as_mut_ptr();
/// ```
#[inline]
pub fn as_mut_ptr(&mut self) -> *mut libc::c_char {
self.buf as *mut _
match self.inner {
Owned(buf) => buf,
NotOwned(buf) => buf as *mut libc::c_char,
}
}

/// Returns whether or not the `CString` owns the buffer.
pub fn owns_buffer(&self) -> bool {
self.owns_buffer_
match self.inner {
Owned(_) => true,
NotOwned(_) => false,
}
}

/// Converts the CString into a `&[u8]` without copying.
/// Includes the terminating NUL byte.
#[inline]
pub fn as_bytes<'a>(&'a self) -> &'a [u8] {
unsafe {
mem::transmute(Slice { data: self.buf, len: self.len() + 1 })
mem::transmute(Slice { data: self.as_ptr(), len: self.len() + 1 })
}
}

Expand All @@ -223,7 +235,7 @@ impl CString {
#[inline]
pub fn as_bytes_no_nul<'a>(&'a self) -> &'a [u8] {
unsafe {
mem::transmute(Slice { data: self.buf, len: self.len() })
mem::transmute(Slice { data: self.as_ptr(), len: self.len() })
}
}

Expand All @@ -238,7 +250,7 @@ impl CString {
/// Return a CString iterator.
pub fn iter<'a>(&'a self) -> CChars<'a> {
CChars {
ptr: self.buf,
ptr: self.as_ptr(),
marker: marker::ContravariantLifetime,
}
}
Expand All @@ -255,15 +267,16 @@ impl CString {
/// Prefer `.as_ptr()` when just retrieving a pointer to the
/// string data, as that does not relinquish ownership.
pub unsafe fn unwrap(mut self) -> *const libc::c_char {
self.owns_buffer_ = false;
self.buf
let ret = self.as_ptr();
self.inner = NotOwned(ret);
ret
}

/// Return the number of bytes in the CString (not including the NUL
/// terminator).
#[inline]
pub fn len(&self) -> uint {
unsafe { libc::strlen(self.buf) as uint }
unsafe { libc::strlen(self.as_ptr()) as uint }
}

/// Returns if there are no bytes in this string
Expand All @@ -273,10 +286,11 @@ impl CString {

impl Drop for CString {
fn drop(&mut self) {
if self.owns_buffer_ {
unsafe {
libc::free(self.buf as *mut libc::c_void)
}
match self.inner {
Owned(buf) => unsafe {
libc::free(buf as *mut libc::c_void)
},
_ => {},
}
}
}
Expand Down Expand Up @@ -393,7 +407,7 @@ impl ToCStr for [u8] {
ptr::copy_memory(buf, self.as_ptr(), self_len);
*buf.offset(self_len as int) = 0;

CString::new(buf as *const libc::c_char, true)
CString::new_owned(buf as *mut libc::c_char)
}

fn with_c_str<T>(&self, f: |*const libc::c_char| -> T) -> T {
Expand Down Expand Up @@ -500,7 +514,7 @@ pub unsafe fn from_c_multistring(buf: *const libc::c_char,
};
while ((limited_count && ctr < limit) || !limited_count)
&& *(curr_ptr as *const libc::c_char) != 0 as libc::c_char {
let cstr = CString::new(curr_ptr as *const libc::c_char, false);
let cstr = CString::new(curr_ptr as *const libc::c_char);
f(&cstr);
curr_ptr += cstr.len() + 1;
ctr += 1;
Expand Down Expand Up @@ -665,7 +679,7 @@ mod tests {
#[test]
#[should_fail]
fn test_new_fail() {
let _c_str = unsafe { CString::new(ptr::null(), false) };
let _c_str = unsafe { CString::new(ptr::null()) };
}

#[test]
Expand All @@ -681,7 +695,7 @@ mod tests {
let s = "test".to_string();
let c = s.to_c_str();
// give the closure a non-owned CString
let mut c_ = unsafe { CString::new(c.as_ptr(), false) };
let mut c_ = unsafe { CString::new(c.as_ptr()) };
f(&c_);
// muck with the buffer for later printing
unsafe { *c_.as_mut_ptr() = 'X' as libc::c_char }
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/dynamic_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ pub mod dl {
let ret = if ptr::null() == last_error {
Ok(result)
} else {
Err(String::from_str(CString::new(last_error, false).as_str()
Err(String::from_str(CString::new(last_error).as_str()
.unwrap()))
};

Expand Down
10 changes: 5 additions & 5 deletions src/libstd/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub fn getcwd() -> Path {
if libc::getcwd(buf.as_mut_ptr(), buf.len() as libc::size_t).is_null() {
panic!()
}
Path::new(CString::new(buf.as_ptr(), false))
Path::new(CString::new(buf.as_ptr()))
}
}

Expand Down Expand Up @@ -288,7 +288,7 @@ pub fn env_as_bytes() -> Vec<(Vec<u8>,Vec<u8>)> {
let mut result = Vec::new();
while *environ != 0 as *const _ {
let env_pair =
CString::new(*environ, false).as_bytes_no_nul().to_vec();
CString::new(*environ).as_bytes_no_nul().to_vec();
result.push(env_pair);
environ = environ.offset(1);
}
Expand Down Expand Up @@ -355,7 +355,7 @@ pub fn getenv_as_bytes(n: &str) -> Option<Vec<u8>> {
if s.is_null() {
None
} else {
Some(CString::new(s as *const i8, false).as_bytes_no_nul().to_vec())
Some(CString::new(s as *const i8).as_bytes_no_nul().to_vec())
}
})
}
Expand Down Expand Up @@ -1103,7 +1103,7 @@ unsafe fn load_argc_and_argv(argc: int,
use c_str::CString;

Vec::from_fn(argc as uint, |i| {
CString::new(*argv.offset(i as int), false).as_bytes_no_nul().to_vec()
CString::new(*argv.offset(i as int)).as_bytes_no_nul().to_vec()
})
}

Expand Down Expand Up @@ -1170,7 +1170,7 @@ fn real_args_as_bytes() -> Vec<Vec<u8>> {
let tmp = objc_msgSend(args, objectAtSel, i);
let utf_c_str: *const libc::c_char =
mem::transmute(objc_msgSend(tmp, utf8Sel));
let s = CString::new(utf_c_str, false);
let s = CString::new(utf_c_str);
res.push(s.as_bytes_no_nul().to_vec())
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/libstd/rt/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ mod imp {
output(w, idx,addr, None)
} else {
output(w, idx, addr, Some(unsafe {
CString::new(info.dli_sname, false)
CString::new(info.dli_sname)
}))
}
}
Expand Down Expand Up @@ -516,7 +516,7 @@ mod imp {
if ret == 0 || data.is_null() {
output(w, idx, addr, None)
} else {
output(w, idx, addr, Some(unsafe { CString::new(data, false) }))
output(w, idx, addr, Some(unsafe { CString::new(data) }))
}
}

Expand Down Expand Up @@ -990,7 +990,7 @@ mod imp {

if ret == libc::TRUE {
try!(write!(w, " - "));
let cstr = unsafe { CString::new(info.Name.as_ptr(), false) };
let cstr = unsafe { CString::new(info.Name.as_ptr()) };
let bytes = cstr.as_bytes();
match cstr.as_str() {
Some(s) => try!(super::demangle(w, s)),
Expand Down
Loading