From cb3ab63815cc38eb16d9629c02b033731308801e Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Mon, 12 Jun 2023 05:59:50 -0700 Subject: [PATCH] Replace all CStr usage to Utf8CStr --- native/src/base/cxx_extern.rs | 19 +++++++++--- native/src/base/files.rs | 20 ++++++------ native/src/base/misc.rs | 58 +++++++++++++++++++++++++++-------- native/src/base/xwrap.rs | 40 ++++++++++-------------- native/src/boot/cpio.rs | 4 +-- native/src/boot/payload.rs | 14 ++++----- native/src/core/lib.rs | 4 +-- 7 files changed, 97 insertions(+), 62 deletions(-) diff --git a/native/src/base/cxx_extern.rs b/native/src/base/cxx_extern.rs index 2070c9a20..b84fa28d9 100644 --- a/native/src/base/cxx_extern.rs +++ b/native/src/base/cxx_extern.rs @@ -1,6 +1,5 @@ // Functions listed here are just to export to C++ -use std::ffi::CStr; use std::io; use std::os::fd::{BorrowedFd, OwnedFd, RawFd}; @@ -10,6 +9,7 @@ use libc::mode_t; use crate::{ fd_path, map_fd, map_file, mkdirs, realpath, rm_rf, slice_from_ptr_mut, Directory, ResultExt, + Utf8CStr, }; pub(crate) fn fd_path_for_cxx(fd: RawFd, buf: &mut [u8]) -> isize { @@ -21,17 +21,26 @@ pub(crate) fn fd_path_for_cxx(fd: RawFd, buf: &mut [u8]) -> isize { #[no_mangle] unsafe extern "C" fn canonical_path(path: *const c_char, buf: *mut u8, bufsz: usize) -> isize { - realpath(CStr::from_ptr(path), slice_from_ptr_mut(buf, bufsz)).map_or(-1, |v| v as isize) + match Utf8CStr::from_ptr(path) { + Ok(p) => realpath(p, slice_from_ptr_mut(buf, bufsz)).map_or(-1, |v| v as isize), + Err(_) => -1, + } } #[export_name = "mkdirs"] unsafe extern "C" fn mkdirs_for_cxx(path: *const c_char, mode: mode_t) -> i32 { - mkdirs(CStr::from_ptr(path), mode).map_or(-1, |_| 0) + match Utf8CStr::from_ptr(path) { + Ok(p) => mkdirs(p, mode).map_or(-1, |_| 0), + Err(_) => -1, + } } #[export_name = "rm_rf"] unsafe extern "C" fn rm_rf_for_cxx(path: *const c_char) -> bool { - rm_rf(CStr::from_ptr(path)).map_or(false, |_| true) + match Utf8CStr::from_ptr(path) { + Ok(p) => rm_rf(p).map_or(false, |_| true), + Err(_) => false, + } } #[no_mangle] @@ -44,7 +53,7 @@ unsafe extern "C" fn frm_rf(fd: OwnedFd) -> bool { pub(crate) fn map_file_for_cxx(path: &[u8], rw: bool) -> &'static mut [u8] { unsafe { - map_file(CStr::from_bytes_with_nul_unchecked(path), rw) + map_file(Utf8CStr::from_bytes_unchecked(path), rw) .log() .unwrap_or(&mut []) } diff --git a/native/src/base/files.rs b/native/src/base/files.rs index fbd535270..a2717049c 100644 --- a/native/src/base/files.rs +++ b/native/src/base/files.rs @@ -10,9 +10,9 @@ use std::{io, mem, ptr, slice}; use libc::{c_char, c_uint, dirent, mode_t, EEXIST, ENOENT, O_CLOEXEC, O_PATH, O_RDONLY, O_RDWR}; -use crate::{bfmt_cstr, copy_cstr, cstr, errno, error, LibcReturn}; +use crate::{bfmt_cstr, copy_cstr, cstr, errno, error, LibcReturn, Utf8CStr}; -pub fn __open_fd_impl(path: &CStr, flags: i32, mode: mode_t) -> io::Result { +pub fn __open_fd_impl(path: &Utf8CStr, flags: i32, mode: mode_t) -> io::Result { unsafe { let fd = libc::open(path.as_ptr(), flags, mode as c_uint).check_os_err()?; Ok(OwnedFd::from_raw_fd(fd)) @@ -37,7 +37,7 @@ pub unsafe fn readlink_unsafe(path: *const c_char, buf: *mut u8, bufsz: usize) - r } -pub fn readlink(path: &CStr, data: &mut [u8]) -> io::Result { +pub fn readlink(path: &Utf8CStr, data: &mut [u8]) -> io::Result { let r = unsafe { readlink_unsafe(path.as_ptr(), data.as_mut_ptr(), data.len()) }.check_os_err()?; Ok(r as usize) @@ -50,7 +50,7 @@ pub fn fd_path(fd: RawFd, buf: &mut [u8]) -> io::Result { } // Inspired by https://android.googlesource.com/platform/bionic/+/master/libc/bionic/realpath.cpp -pub fn realpath(path: &CStr, buf: &mut [u8]) -> io::Result { +pub fn realpath(path: &Utf8CStr, buf: &mut [u8]) -> io::Result { let fd = open_fd!(path, O_PATH | O_CLOEXEC)?; let mut st1: libc::stat; let mut st2: libc::stat; @@ -75,7 +75,7 @@ pub fn realpath(path: &CStr, buf: &mut [u8]) -> io::Result { Ok(len) } -pub fn mkdirs(path: &CStr, mode: mode_t) -> io::Result<()> { +pub fn mkdirs(path: &Utf8CStr, mode: mode_t) -> io::Result<()> { let mut buf = [0_u8; 4096]; let len = copy_cstr(&mut buf, path); let buf = &mut buf[..len]; @@ -277,7 +277,7 @@ pub enum WalkResult { } impl Directory { - pub fn open(path: &CStr) -> io::Result { + pub fn open(path: &Utf8CStr) -> io::Result { let dirp = unsafe { libc::opendir(path.as_ptr()) }.check_os_err()?; Ok(Directory { dirp }) } @@ -420,7 +420,7 @@ impl Drop for Directory { } } -pub fn rm_rf(path: &CStr) -> io::Result<()> { +pub fn rm_rf(path: &Utf8CStr) -> io::Result<()> { unsafe { let mut stat: libc::stat = mem::zeroed(); libc::lstat(path.as_ptr(), &mut stat).check_os_err()?; @@ -490,11 +490,11 @@ impl FdExt for T { pub struct MappedFile(&'static mut [u8]); impl MappedFile { - pub fn open(path: &CStr) -> io::Result { + pub fn open(path: &Utf8CStr) -> io::Result { Ok(MappedFile(map_file(path, false)?)) } - pub fn open_rw(path: &CStr) -> io::Result { + pub fn open_rw(path: &Utf8CStr) -> io::Result { Ok(MappedFile(map_file(path, true)?)) } @@ -524,7 +524,7 @@ impl Drop for MappedFile { } // We mark the returned slice static because it is valid until explicitly unmapped -pub(crate) fn map_file(path: &CStr, rw: bool) -> io::Result<&'static mut [u8]> { +pub(crate) fn map_file(path: &Utf8CStr, rw: bool) -> io::Result<&'static mut [u8]> { let flag = if rw { O_RDWR } else { O_RDONLY }; let fd = open_fd!(path, flag | O_CLOEXEC)?; map_fd(fd.as_fd(), fd.size()?, rw) diff --git a/native/src/base/misc.rs b/native/src/base/misc.rs index a91550c54..b432dc5f4 100644 --- a/native/src/base/misc.rs +++ b/native/src/base/misc.rs @@ -17,8 +17,8 @@ pub fn copy_str>(dest: &mut [u8], src: T) -> usize { len } -pub fn copy_cstr(dest: &mut [u8], src: &CStr) -> usize { - let src = src.to_bytes_with_nul(); +pub fn copy_cstr + ?Sized>(dest: &mut [u8], src: &T) -> usize { + let src = src.as_ref().to_bytes_with_nul(); let len = min(src.len(), dest.len()); dest[..len].copy_from_slice(&src[..len]); len @@ -69,7 +69,9 @@ macro_rules! bfmt_cstr { ($buf:expr, $($args:tt)*) => {{ let len = $crate::fmt_to_buf($buf, format_args!($($args)*)); #[allow(unused_unsafe)] - unsafe { std::ffi::CStr::from_bytes_with_nul_unchecked(&$buf[..(len + 1)]) } + unsafe { + $crate::Utf8CStr::from_bytes_unchecked($buf.get_unchecked(..(len + 1))) + } }}; } @@ -84,7 +86,7 @@ macro_rules! cstr { ); #[allow(unused_unsafe)] unsafe { - std::ffi::CStr::from_bytes_with_nul_unchecked(concat!($str, "\0").as_bytes()) + $crate::Utf8CStr::from_bytes_unchecked(concat!($str, "\0").as_bytes()) } }}; } @@ -135,6 +137,7 @@ impl Utf8CStr { } } + #[inline] pub unsafe fn from_bytes_unchecked(buf: &[u8]) -> &Utf8CStr { &*(buf as *const [u8] as *const Utf8CStr) } @@ -146,35 +149,46 @@ impl Utf8CStr { Self::from_cstr(unsafe { CStr::from_ptr(ptr) }) } + #[inline] pub fn as_bytes(&self) -> &[u8] { // The length of the slice is at least 1 due to null termination check unsafe { self.inner.get_unchecked(..self.inner.len() - 1) } } + #[inline] pub fn as_bytes_with_nul(&self) -> &[u8] { &self.inner } + #[inline] pub fn as_ptr(&self) -> *const c_char { self.inner.as_ptr().cast() } + + #[inline] + pub fn as_cstr(&self) -> &CStr { + self.as_ref() + } } impl Deref for Utf8CStr { type Target = str; + #[inline] fn deref(&self) -> &str { self.as_ref() } } impl Display for Utf8CStr { + #[inline] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { Display::fmt(self.deref(), f) } } impl Debug for Utf8CStr { + #[inline] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { Debug::fmt(self.deref(), f) } @@ -199,24 +213,42 @@ impl AsRef for Utf8CStr { impl AsRef for Utf8CStr { #[inline] fn as_ref(&self) -> &OsStr { - OsStr::new(self as &str) + OsStr::new(self.deref()) } } impl AsRef for Utf8CStr { #[inline] fn as_ref(&self) -> &Path { - Path::new(self) + Path::new(self.deref()) } } -pub fn ptr_to_str_result<'a, T>(ptr: *const T) -> Result<&'a str, StrErr> { - if ptr.is_null() { - Err(StrErr::NullPointerError) - } else { - unsafe { CStr::from_ptr(ptr.cast()) } - .to_str() - .map_err(StrErr::from) +impl PartialEq for Utf8CStr { + #[inline] + fn eq(&self, other: &CStr) -> bool { + self.as_cstr() == other + } +} + +impl PartialEq for Utf8CStr { + #[inline] + fn eq(&self, other: &str) -> bool { + self.deref() == other + } +} + +impl PartialEq for CStr { + #[inline] + fn eq(&self, other: &Utf8CStr) -> bool { + self == other.as_cstr() + } +} + +impl PartialEq for str { + #[inline] + fn eq(&self, other: &Utf8CStr) -> bool { + self == other.deref() } } diff --git a/native/src/base/xwrap.rs b/native/src/base/xwrap.rs index c24cc65bd..31b012360 100644 --- a/native/src/base/xwrap.rs +++ b/native/src/base/xwrap.rs @@ -1,4 +1,3 @@ -use std::ffi::CStr; use std::os::unix::io::RawFd; use std::ptr; @@ -8,10 +7,9 @@ use libc::{ ssize_t, SYS_dup3, }; -use crate::{cstr, errno, error, mkdirs, perror, ptr_to_str, raw_cstr, ResultExt}; +use crate::{cstr, errno, error, mkdirs, perror, ptr_to_str, raw_cstr, ResultExt, Utf8CStr}; mod unsafe_impl { - use std::ffi::CStr; use std::os::unix::io::RawFd; use anyhow::Context; @@ -19,8 +17,8 @@ mod unsafe_impl { use libc::{c_char, nfds_t, off_t, pollfd}; use crate::{ - perror, ptr_to_str, readlink, readlink_unsafe, slice_from_ptr, slice_from_ptr_mut, - ResultExt, + perror, ptr_to_str, readlink_unsafe, realpath, slice_from_ptr, slice_from_ptr_mut, + ResultExt, Utf8CStr, }; #[no_mangle] @@ -40,10 +38,13 @@ mod unsafe_impl { #[no_mangle] unsafe extern "C" fn xrealpath(path: *const c_char, buf: *mut u8, bufsz: usize) -> isize { - readlink(CStr::from_ptr(path), slice_from_ptr_mut(buf, bufsz)) - .with_context(|| format!("realpath {} failed", ptr_to_str(path))) - .log() - .map_or(-1, |v| v as isize) + match Utf8CStr::from_ptr(path) { + Ok(p) => realpath(p, slice_from_ptr_mut(buf, bufsz)) + .with_context(|| format!("realpath {} failed", p)) + .log() + .map_or(-1, |v| v as isize), + Err(_) => -1, + } } #[no_mangle] @@ -475,16 +476,6 @@ pub extern "C" fn xdup3(oldfd: RawFd, newfd: RawFd, flags: i32) -> RawFd { } } -#[inline] -pub fn xreadlink(path: &CStr, data: &mut [u8]) -> isize { - unsafe { unsafe_impl::xreadlink(path.as_ptr(), data.as_mut_ptr(), data.len()) } -} - -#[inline] -pub fn xreadlinkat(dirfd: RawFd, path: &CStr, data: &mut [u8]) -> isize { - unsafe { unsafe_impl::xreadlinkat(dirfd, path.as_ptr(), data.as_mut_ptr(), data.len()) } -} - #[no_mangle] pub unsafe extern "C" fn xsymlink(target: *const c_char, linkpath: *const c_char) -> i32 { let r = libc::symlink(target, linkpath); @@ -579,10 +570,13 @@ pub unsafe extern "C" fn xmkdir(path: *const c_char, mode: mode_t) -> i32 { #[no_mangle] pub unsafe extern "C" fn xmkdirs(path: *const c_char, mode: mode_t) -> i32 { - mkdirs(CStr::from_ptr(path), mode) - .with_context(|| format!("mkdirs {}", ptr_to_str(path))) - .log() - .map_or(-1, |_| 0) + match Utf8CStr::from_ptr(path) { + Ok(p) => mkdirs(p, mode) + .with_context(|| format!("mkdirs {} failed", p)) + .log() + .map_or(-1, |_| 0), + Err(_) => -1, + } } #[no_mangle] diff --git a/native/src/boot/cpio.rs b/native/src/boot/cpio.rs index 4411d5fb5..43892fe2c 100644 --- a/native/src/boot/cpio.rs +++ b/native/src/boot/cpio.rs @@ -155,8 +155,8 @@ impl Cpio { pub(crate) fn load_from_file(path: &Utf8CStr) -> anyhow::Result { eprintln!("Loading cpio: [{}]", path); - let data = MappedFile::open(path.as_ref())?; - Self::load_from_data(data.as_ref()) + let file = MappedFile::open(path)?; + Self::load_from_data(file.as_ref()) } fn dump(&self, path: &str) -> anyhow::Result<()> { diff --git a/native/src/boot/payload.rs b/native/src/boot/payload.rs index 9cb2d5249..ac75f7811 100644 --- a/native/src/boot/payload.rs +++ b/native/src/boot/payload.rs @@ -7,7 +7,7 @@ use byteorder::{BigEndian, ReadBytesExt}; use protobuf::{EnumFull, Message}; use base::libc::c_char; -use base::{ptr_to_str_result, ReadSeekExt, StrErr}; +use base::{ReadSeekExt, StrErr, Utf8CStr}; use base::{ResultExt, WriteExt}; use crate::ffi; @@ -26,9 +26,9 @@ macro_rules! bad_payload { const PAYLOAD_MAGIC: &str = "CrAU"; fn do_extract_boot_from_payload( - in_path: &str, - partition_name: Option<&str>, - out_path: Option<&str>, + in_path: &Utf8CStr, + partition_name: Option<&Utf8CStr>, + out_path: Option<&Utf8CStr>, ) -> anyhow::Result<()> { let mut reader = BufReader::new(if in_path == "-" { unsafe { File::from_raw_fd(0) } @@ -193,13 +193,13 @@ pub fn extract_boot_from_payload( partition: *const c_char, out_path: *const c_char, ) -> anyhow::Result<()> { - let in_path = ptr_to_str_result(in_path)?; - let partition = match ptr_to_str_result(partition) { + let in_path = unsafe { Utf8CStr::from_ptr(in_path) }?; + let partition = match unsafe { Utf8CStr::from_ptr(partition) } { Ok(s) => Some(s), Err(StrErr::NullPointerError) => None, Err(e) => Err(e)?, }; - let out_path = match ptr_to_str_result(out_path) { + let out_path = match unsafe { Utf8CStr::from_ptr(out_path) } { Ok(s) => Some(s), Err(StrErr::NullPointerError) => None, Err(e) => Err(e)?, diff --git a/native/src/core/lib.rs b/native/src/core/lib.rs index 7a1a0ec26..757a033bc 100644 --- a/native/src/core/lib.rs +++ b/native/src/core/lib.rs @@ -1,8 +1,8 @@ #![allow(clippy::missing_safety_doc)] +use base::Utf8CStr; use daemon::*; use logging::*; -use std::ffi::CStr; #[path = "../include/consts.rs"] mod consts; @@ -39,6 +39,6 @@ pub mod ffi { fn rust_test_entry() {} -pub fn get_prop(name: &CStr, persist: bool) -> String { +pub fn get_prop(name: &Utf8CStr, persist: bool) -> String { unsafe { ffi::get_prop_rs(name.as_ptr(), persist) } }