Merge branch 'cstr_strchr' into 'master'

Add safe strchr method for CStr.

See merge request redox-os/relibc!723

src/c_str.rs

@@ -7,7 +7,10 @@ use alloc::{
     string::String,
 };
 
-use crate::{header::string::strlen, platform::types::c_char};
+use crate::{
+    header::string::{strchr, strlen},
+    platform::types::c_char,
+};
 
 /// C string wrapper, guaranteed to be
 #[derive(Clone, Copy)]
@@ -34,6 +37,30 @@ impl<'a> CStr<'a> {
             Some(Self::from_ptr(ptr))
         }
     }
+    #[doc(alias = "strchr")]
+    #[inline]
+    pub fn find(self, c: u8) -> Option<Self> {
+        unsafe {
+            // SAFETY: the only requirement is for self.as_ptr() to be valid up to and including
+            // the nearest NUL byte, which this type requires
+            let ret = strchr(self.as_ptr(), c.into());
+            // SAFETY: strchr must either return NULL (not found) or a substring of self, which can
+            // never exceed the nearest NUL byte of self
+            Self::from_nullable_ptr(ret)
+        }
+    }
+    #[inline]
+    pub fn contains(self, c: u8) -> bool {
+        self.find(c).is_some()
+    }
+    #[inline]
+    pub fn first(self) -> u8 {
+        unsafe {
+            // SAFETY: Self must be valid up to and including its nearest NUL byte, which certainly
+            // implies its readable length is nonzero (string is empty if this first byte is 0).
+            self.ptr.read() as u8
+        }
+    }
     pub fn to_bytes_with_nul(self) -> &'a [u8] {
         unsafe {
             // SAFETY: The string must be valid at least until (and including) the NUL byte.
@@ -77,6 +104,7 @@ impl<'a> CStr<'a> {
     pub fn borrow(string: &'a CString) -> Self {
         unsafe { Self::from_ptr(string.as_ptr()) }
     }
+    #[doc(alias = "strlen")]
     pub fn count_bytes(&self) -> usize {
         self.to_bytes().len()
     }

src/header/stdio/helpers.rs

@@ -2,8 +2,13 @@ use alloc::boxed::Box;
 
 use super::{constants::*, Buffer, FILE};
 use crate::{
+    c_str::CStr,
+    error::Errno,
     fs::File,
-    header::{errno, fcntl::*, string::strchr},
+    header::{
+        errno::{self, EINVAL},
+        fcntl::*,
+    },
     io::BufWriter,
     platform::{self, types::*},
     sync::Mutex,
@@ -11,26 +16,26 @@ use crate::{
 use alloc::vec::Vec;
 
 /// Parse mode flags as a string and output a mode flags integer
-pub unsafe fn parse_mode_flags(mode_str: *const c_char) -> i32 {
-    let mut flags = if !strchr(mode_str, b'+' as i32).is_null() {
+pub fn parse_mode_flags(mode_str: CStr) -> i32 {
+    let mut flags = if mode_str.contains(b'+') {
         O_RDWR
-    } else if (*mode_str) == b'r' as c_char {
+    } else if mode_str.first() == b'r' {
         O_RDONLY
     } else {
         O_WRONLY
     };
-    if !strchr(mode_str, b'x' as i32).is_null() {
+    if mode_str.contains(b'x') {
         flags |= O_EXCL;
     }
-    if !strchr(mode_str, b'e' as i32).is_null() {
+    if mode_str.contains(b'e') {
         flags |= O_CLOEXEC;
     }
-    if (*mode_str) != b'r' as c_char {
+    if mode_str.first() != b'r' {
         flags |= O_CREAT;
     }
-    if (*mode_str) == b'w' as c_char {
+    if mode_str.first() == b'w' {
         flags |= O_TRUNC;
-    } else if (*mode_str) == b'a' as c_char {
+    } else if mode_str.first() == b'a' {
         flags |= O_APPEND;
     }
 
@@ -38,37 +43,34 @@ pub unsafe fn parse_mode_flags(mode_str: *const c_char) -> i32 {
 }
 
 /// Open a file with the file descriptor `fd` in the mode `mode`
-pub unsafe fn _fdopen(fd: c_int, mode: *const c_char) -> Option<*mut FILE> {
-    if *mode != b'r' as c_char && *mode != b'w' as c_char && *mode != b'a' as c_char {
-        platform::ERRNO.set(errno::EINVAL);
-        return None;
+pub fn _fdopen(fd: c_int, mode: CStr) -> Result<Box<FILE>, Errno> {
+    if mode.first() != b'r' && mode.first() != b'w' && mode.first() != b'a' {
+        return Err(Errno(EINVAL));
     }
 
     let mut flags = 0;
-    if strchr(mode, b'+' as i32).is_null() {
-        flags |= if *mode == b'r' as c_char {
-            F_NOWR
-        } else {
-            F_NORD
-        };
+    if !mode.contains(b'+') {
+        flags |= if mode.first() == b'r' { F_NOWR } else { F_NORD };
     }
 
-    if !strchr(mode, b'e' as i32).is_null() {
-        fcntl(fd, F_SETFD, FD_CLOEXEC as c_ulonglong);
+    if mode.contains(b'e') {
+        unsafe {
+            fcntl(fd, F_SETFD, FD_CLOEXEC as c_ulonglong);
+        }
     }
 
-    if *mode == 'a' as c_char {
-        let f = fcntl(fd, F_GETFL, 0);
+    if mode.first() == b'a' {
+        let f = unsafe { fcntl(fd, F_GETFL, 0) };
         if (f & O_APPEND) == 0 {
-            fcntl(fd, F_SETFL, (f | O_APPEND) as c_ulonglong);
+            unsafe { fcntl(fd, F_SETFL, (f | O_APPEND) as c_ulonglong) };
         }
         flags |= F_APP;
     }
 
     let file = File::new(fd);
-    let writer = Box::new(BufWriter::new(file.get_ref()));
+    let writer = Box::new(BufWriter::new(unsafe { file.get_ref() }));
 
-    Some(Box::into_raw(Box::new(FILE {
+    Ok(Box::new(FILE {
         lock: Mutex::new(()),
 
         file,
@@ -82,5 +84,5 @@ pub unsafe fn _fdopen(fd: c_int, mode: *const c_char) -> Option<*mut FILE> {
         pid: None,
 
         orientation: 0,
-    })))
+    }))
 }

src/header/stdio/mod.rs

@@ -17,7 +17,7 @@ use core::{
 use crate::{
     c_str::CStr,
     c_vec::CVec,
-    error::ResultExt,
+    error::{ResultExt, ResultExtPtrMut},
     fs::File,
     header::{
         errno::{self, STR_ERROR},
@@ -347,11 +347,9 @@ pub unsafe extern "C" fn fclose(stream: *mut FILE) -> c_int {
 /// Open a file from a file descriptor
 #[no_mangle]
 pub unsafe extern "C" fn fdopen(fildes: c_int, mode: *const c_char) -> *mut FILE {
-    if let Some(f) = helpers::_fdopen(fildes, mode) {
-        f
-    } else {
-        ptr::null_mut()
-    }
+    helpers::_fdopen(fildes, CStr::from_ptr(mode))
+        .map(|f| Box::into_raw(f))
+        .or_errno_null_mut()
 }
 
 /// Check for EOF
@@ -514,7 +512,7 @@ pub unsafe extern "C" fn fopen(filename: *const c_char, mode: *const c_char) ->
         return ptr::null_mut();
     }
 
-    let flags = helpers::parse_mode_flags(mode);
+    let flags = helpers::parse_mode_flags(CStr::from_ptr(mode));
 
     let new_mode = if flags & fcntl::O_CREAT == fcntl::O_CREAT {
         0o666
@@ -531,12 +529,14 @@ pub unsafe extern "C" fn fopen(filename: *const c_char, mode: *const c_char) ->
         fcntl::fcntl(fd, fcntl::F_SETFD, fcntl::FD_CLOEXEC as c_ulonglong);
     }
 
-    if let Some(f) = helpers::_fdopen(fd, mode) {
-        f
-    } else {
-        Sys::close(fd);
-        ptr::null_mut()
-    }
+    helpers::_fdopen(fd, CStr::from_ptr(mode))
+        .map(|f| Box::into_raw(f))
+        .map_err(|err| {
+            // TODO: guard type
+            Sys::close(fd);
+            err
+        })
+        .or_errno_null_mut()
 }
 
 /// Clear the buffers of a stream
@@ -612,7 +612,7 @@ pub unsafe extern "C" fn freopen(
     mode: *const c_char,
     stream: &mut FILE,
 ) -> *mut FILE {
-    let mut flags = helpers::parse_mode_flags(mode);
+    let mut flags = helpers::parse_mode_flags(CStr::from_ptr(mode));
     flockfile(stream);
 
     let _ = stream.flush();
@@ -951,12 +951,12 @@ pub unsafe extern "C" fn popen(command: *const c_char, mode: *const c_char) -> *
             (pipes[0], if cloexec { c"re".into() } else { c"r".into() })
         };
 
-        if let Some(f) = helpers::_fdopen(fd, fd_mode.as_ptr()) {
-            (*f).pid = Some(child_pid);
-            f
-        } else {
-            ptr::null_mut()
-        }
+        helpers::_fdopen(fd, fd_mode)
+            .map(|mut f| {
+                f.pid = Some(child_pid);
+                Box::into_raw(f)
+            })
+            .or_errno_null_mut()
     } else {
         ptr::null_mut()
     }