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]

diff --git a/src/libnative/io/addrinfo.rs b/src/libnative/io/addrinfo.rs
index 0977b55d8b9780d437f385fb473e1ecf49997943..343fe05ed42c132ff9ebf89bb0bd50fb150c4a53 100644 (file)
--- a/src/libnative/io/addrinfo.rs
+++ b/src/libnative/io/addrinfo.rs
@@ -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 @@ pub fn run(host: Option<&str>, servname: Option<&str>,
     {
         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 @@ pub fn run(host: Option<&str>, servname: Option<&str>,
 
         // 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()

diff --git a/src/librustrt/c_str.rs b/src/librustrt/c_str.rs
index c30b267219b090af5e18a522075a08ca48ddb4bb..5dd61c03d17e75b053eabff31b5baecde5e91ca9 100644 (file)
--- a/src/librustrt/c_str.rs
+++ b/src/librustrt/c_str.rs
@@ -70,10 +70,10 @@ fn main() {
 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 @@ fn hash(&self, state: &mut S) {
 
 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 @@ pub unsafe fn new(buf: *const libc::c_char, owns_buffer: bool) -> CString {
     /// let p = foo.to_c_str().as_ptr();
     /// ```
     ///
-    /// # Failure
-    ///
-    /// Fails if the CString is null.
-    ///
     /// # Example
     ///
     /// ```rust
@@ -175,8 +171,6 @@ pub unsafe fn new(buf: *const libc::c_char, owns_buffer: bool) -> 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 @@ pub fn as_ptr(&self) -> *const libc::c_char {
     /// // 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 @@ pub fn owns_buffer(&self) -> bool {
 
     /// 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 @@ pub fn as_bytes<'a>(&'a self) -> &'a [u8] {
 
     /// 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 @@ pub fn as_bytes_no_nul<'a>(&'a self) -> &'a [u8] {
 
     /// 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 @@ pub fn as_str<'a>(&'a self) -> Option<&'a str> {
     }
 
     /// 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 @@ fn drop(&mut self) {
 
 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 @@ fn test_vec_to_c_str() {
         }
     }
 
-    #[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 @@ fn test_unwrap() {
     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 @@ fn test_as_bytes_no_nul() {
         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 @@ fn test_as_str() {
 
     #[test]
     #[should_fail]
-    fn test_as_str_fail() {
-        let c_str = unsafe { CString::new(ptr::null(), false) };
-        c_str.as_str();
-    }
-
-    #[test]
-    #[should_fail]
-    fn test_len_fail() {
+    fn test_new_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 @@ fn foo(f: |c: &CString|) {
         // 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)]