Add a null pointer check to CString::new

This also removes checks in other methods of `CString`

Breaking changes:
* `CString::new` now fails if `buf` is null. To avoid this add a check
before creatng a new `CString` .
* The `is_null` and `is_not_null` methods are deprecated, because a
`CString` cannot be null.
* Other methods which used to fail if the `CString` was null do not fail anymore

[breaking-change]
This commit is contained in:
Adolfo Ochagavía 2014-07-22 18:24:33 +02:00 committed by Alex Crichton
parent 6988bcd74c
commit 4ea1dd5494
2 changed files with 19 additions and 114 deletions

View file

@ -10,7 +10,6 @@
use libc::{c_char, c_int};
use libc;
use std::c_str::CString;
use std::mem;
use std::ptr::{null, mut_null};
use std::rt::rtio;
@ -27,8 +26,8 @@ impl GetAddrInfoRequest {
{
assert!(host.is_some() || servname.is_some());
let c_host = host.map_or(unsafe { CString::new(null(), true) }, |x| x.to_c_str());
let c_serv = servname.map_or(unsafe { CString::new(null(), true) }, |x| x.to_c_str());
let c_host = host.map(|x| x.to_c_str());
let c_serv = servname.map(|x| x.to_c_str());
let hint = hint.map(|hint| {
libc::addrinfo {
@ -50,8 +49,8 @@ impl GetAddrInfoRequest {
// Make the call
let s = unsafe {
let ch = if c_host.is_null() { null() } else { c_host.as_ptr() };
let cs = if c_serv.is_null() { null() } else { c_serv.as_ptr() };
let ch = if c_host.is_none() { null() } else { c_host.unwrap().as_ptr() };
let cs = if c_serv.is_none() { null() } else { c_serv.unwrap().as_ptr() };
getaddrinfo(ch, cs, hint_ptr, &mut res)
};
@ -104,6 +103,7 @@ fn get_error(_: c_int) -> IoError {
#[cfg(not(windows))]
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()

View file

@ -70,10 +70,10 @@ use core::prelude::*;
use alloc::libc_heap::malloc_raw;
use collections::string::String;
use collections::hash;
use core::fmt;
use core::kinds::marker;
use core::mem;
use core::ptr;
use core::fmt;
use core::raw::Slice;
use core::slice;
use core::str;
@ -93,23 +93,18 @@ impl Clone for CString {
/// reasons, this is always a deep clone, rather than the usual shallow
/// clone.
fn clone(&self) -> CString {
if self.buf.is_null() {
CString { buf: self.buf, owns_buffer_: self.owns_buffer_ }
} else {
let len = self.len() + 1;
let buf = unsafe { malloc_raw(len) } as *mut libc::c_char;
unsafe { ptr::copy_nonoverlapping_memory(buf, self.buf, len); }
CString { buf: buf as *const libc::c_char, owns_buffer_: true }
}
let len = self.len() + 1;
let buf = unsafe { malloc_raw(len) } as *mut libc::c_char;
unsafe { ptr::copy_nonoverlapping_memory(buf, self.buf, len); }
CString { buf: buf as *const libc::c_char, owns_buffer_: true }
}
}
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 {
true
} else if self.buf.is_null() || other.buf.is_null() {
false
} else {
unsafe {
libc::strcmp(self.buf, other.buf) == 0
@ -136,7 +131,12 @@ impl<S: hash::Writer> hash::Hash<S> for CString {
impl CString {
/// Create a C String from a pointer.
///
///# Failure
///
/// Fails if `buf` is null
pub unsafe fn new(buf: *const libc::c_char, owns_buffer: bool) -> CString {
assert!(!buf.is_null());
CString { buf: buf, owns_buffer_: owns_buffer }
}
@ -158,10 +158,6 @@ impl CString {
/// let p = foo.to_c_str().as_ptr();
/// ```
///
/// # Failure
///
/// Fails if the CString is null.
///
/// # Example
///
/// ```rust
@ -175,8 +171,6 @@ impl CString {
/// }
/// ```
pub fn as_ptr(&self) -> *const libc::c_char {
if self.buf.is_null() { fail!("CString is null!"); }
self.buf
}
@ -197,44 +191,30 @@ impl CString {
/// // wrong (the CString will be freed, invalidating `p`)
/// let p = foo.to_c_str().as_mut_ptr();
/// ```
///
/// # Failure
///
/// Fails if the CString is null.
pub fn as_mut_ptr(&mut self) -> *mut libc::c_char {
if self.buf.is_null() { fail!("CString is null!") }
self.buf as *mut _
}
/// Calls a closure with a reference to the underlying `*libc::c_char`.
///
/// # Failure
///
/// Fails if the CString is null.
#[deprecated="use `.as_ptr()`"]
pub fn with_ref<T>(&self, f: |*const libc::c_char| -> T) -> T {
if self.buf.is_null() { fail!("CString is null!"); }
f(self.buf)
}
/// Calls a closure with a mutable reference to the underlying `*libc::c_char`.
///
/// # Failure
///
/// Fails if the CString is null.
#[deprecated="use `.as_mut_ptr()`"]
pub fn with_mut_ref<T>(&mut self, f: |*mut libc::c_char| -> T) -> T {
if self.buf.is_null() { fail!("CString is null!"); }
f(self.buf as *mut libc::c_char)
}
/// Returns true if the CString is a null.
#[deprecated="a CString cannot be null"]
pub fn is_null(&self) -> bool {
self.buf.is_null()
}
/// Returns true if the CString is not null.
#[deprecated="a CString cannot be null"]
pub fn is_not_null(&self) -> bool {
self.buf.is_not_null()
}
@ -246,13 +226,8 @@ impl CString {
/// Converts the CString into a `&[u8]` without copying.
/// Includes the terminating NUL byte.
///
/// # Failure
///
/// Fails if the CString is null.
#[inline]
pub fn as_bytes<'a>(&'a self) -> &'a [u8] {
if self.buf.is_null() { fail!("CString is null!"); }
unsafe {
mem::transmute(Slice { data: self.buf, len: self.len() + 1 })
}
@ -260,13 +235,8 @@ impl CString {
/// Converts the CString into a `&[u8]` without copying.
/// Does not include the terminating NUL byte.
///
/// # Failure
///
/// Fails if the CString is null.
#[inline]
pub fn as_bytes_no_nul<'a>(&'a self) -> &'a [u8] {
if self.buf.is_null() { fail!("CString is null!"); }
unsafe {
mem::transmute(Slice { data: self.buf, len: self.len() })
}
@ -274,10 +244,6 @@ impl CString {
/// Converts the CString into a `&str` without copying.
/// Returns None if the CString is not UTF-8.
///
/// # Failure
///
/// Fails if the CString is null.
#[inline]
pub fn as_str<'a>(&'a self) -> Option<&'a str> {
let buf = self.as_bytes_no_nul();
@ -285,12 +251,7 @@ impl CString {
}
/// Return a CString iterator.
///
/// # Failure
///
/// Fails if the CString is null.
pub fn iter<'a>(&'a self) -> CChars<'a> {
if self.buf.is_null() { fail!("CString is null!"); }
CChars {
ptr: self.buf,
marker: marker::ContravariantLifetime,
@ -326,13 +287,8 @@ impl Drop for CString {
impl Collection for CString {
/// Return the number of bytes in the CString (not including the NUL terminator).
///
/// # Failure
///
/// Fails if the CString is null.
#[inline]
fn len(&self) -> uint {
if self.buf.is_null() { fail!("CString is null!"); }
let mut cur = self.buf;
let mut len = 0;
unsafe {
@ -631,13 +587,6 @@ mod tests {
}
}
#[test]
fn test_is_null() {
let c_str = unsafe { CString::new(ptr::null(), false) };
assert!(c_str.is_null());
assert!(!c_str.is_not_null());
}
#[test]
fn test_unwrap() {
let c_str = "hello".to_c_str();
@ -648,16 +597,8 @@ mod tests {
fn test_as_ptr() {
let c_str = "hello".to_c_str();
let len = unsafe { libc::strlen(c_str.as_ptr()) };
assert!(!c_str.is_null());
assert!(c_str.is_not_null());
assert_eq!(len, 5);
}
#[test]
#[should_fail]
fn test_as_ptr_empty_fail() {
let c_str = unsafe { CString::new(ptr::null(), false) };
c_str.as_ptr();
}
#[test]
fn test_iterator() {
@ -716,20 +657,6 @@ mod tests {
assert_eq!(c_str.as_bytes_no_nul(), b"foo\xFF");
}
#[test]
#[should_fail]
fn test_as_bytes_fail() {
let c_str = unsafe { CString::new(ptr::null(), false) };
c_str.as_bytes();
}
#[test]
#[should_fail]
fn test_as_bytes_no_nul_fail() {
let c_str = unsafe { CString::new(ptr::null(), false) };
c_str.as_bytes_no_nul();
}
#[test]
fn test_as_str() {
let c_str = "hello".to_c_str();
@ -742,23 +669,8 @@ mod tests {
#[test]
#[should_fail]
fn test_as_str_fail() {
fn test_new_fail() {
let c_str = unsafe { CString::new(ptr::null(), false) };
c_str.as_str();
}
#[test]
#[should_fail]
fn test_len_fail() {
let c_str = unsafe { CString::new(ptr::null(), false) };
c_str.len();
}
#[test]
#[should_fail]
fn test_iter_fail() {
let c_str = unsafe { CString::new(ptr::null(), false) };
c_str.iter();
}
#[test]
@ -791,13 +703,6 @@ mod tests {
// force a copy, reading the memory
c_.as_bytes().to_vec();
}
#[test]
fn test_clone_eq_null() {
let x = unsafe { CString::new(ptr::null(), false) };
let y = x.clone();
assert!(x == y);
}
}
#[cfg(test)]