diff --git a/src/uucore/src/lib/features.rs b/src/uucore/src/lib/features.rs index 42d8ffbe7..546cbea87 100644 --- a/src/uucore/src/lib/features.rs +++ b/src/uucore/src/lib/features.rs @@ -10,7 +10,7 @@ pub mod fsext; pub mod ringbuffer; // * (platform-specific) feature-gated modules -// ** non-windows +// ** non-windows (i.e. Unix + Fuchsia) #[cfg(all(not(windows), feature = "mode"))] pub mod mode; diff --git a/src/uucore/src/lib/features/fsext.rs b/src/uucore/src/lib/features/fsext.rs index 7525d9e41..97c1da79c 100644 --- a/src/uucore/src/lib/features/fsext.rs +++ b/src/uucore/src/lib/features/fsext.rs @@ -26,16 +26,12 @@ const MAX_PATH: usize = 266; static EXIT_ERR: i32 = 1; #[cfg(windows)] -use std::ffi::OsString; +use std::ffi::OsStr; #[cfg(windows)] use std::os::windows::ffi::OsStrExt; #[cfg(windows)] -use std::os::windows::ffi::OsStringExt; -#[cfg(windows)] use winapi::shared::minwindef::DWORD; #[cfg(windows)] -use winapi::um::errhandlingapi::GetLastError; -#[cfg(windows)] use winapi::um::fileapi::GetDiskFreeSpaceW; #[cfg(windows)] use winapi::um::fileapi::{ @@ -47,11 +43,12 @@ use winapi::um::handleapi::INVALID_HANDLE_VALUE; #[cfg(windows)] use winapi::um::winbase::DRIVE_REMOTE; +// Warning: the pointer has to be used *immediately* or the Vec +// it points to will be dropped! #[cfg(windows)] macro_rules! String2LPWSTR { ($str: expr) => { - OsString::from($str.clone()) - .as_os_str() + OsStr::new(&$str) .encode_wide() .chain(Some(0)) .collect::>() @@ -62,10 +59,8 @@ macro_rules! String2LPWSTR { #[cfg(windows)] #[allow(non_snake_case)] fn LPWSTR2String(buf: &[u16]) -> String { - let len = unsafe { libc::wcslen(buf.as_ptr()) }; - OsString::from_wide(&buf[..len as usize]) - .into_string() - .unwrap() + let len = buf.iter().position(|&n| n == 0).unwrap(); + String::from_utf16(&buf[..len]).unwrap() } use self::time::Timespec; @@ -77,7 +72,6 @@ use std::borrow::Cow; use std::convert::{AsRef, From}; #[cfg(unix)] use std::ffi::CString; -#[cfg(unix)] use std::io::Error as IOError; #[cfg(unix)] use std::mem; @@ -157,16 +151,14 @@ impl MountInfo { fn set_missing_fields(&mut self) { #[cfg(unix)] { + use std::os::unix::fs::MetadataExt; // We want to keep the dev_id on Windows // but set dev_id - let path = CString::new(self.mount_dir.clone()).unwrap(); - unsafe { - let mut stat = mem::zeroed(); - if libc::stat(path.as_ptr(), &mut stat) == 0 { - self.dev_id = (stat.st_dev as i32).to_string(); - } else { - self.dev_id = "".to_string(); - } + if let Ok(stat) = std::fs::metadata(&self.mount_dir) { + // Why do we cast this to i32? + self.dev_id = (stat.dev() as i32).to_string() + } else { + self.dev_id = "".to_string(); } } // set MountInfo::dummy @@ -247,8 +239,7 @@ impl MountInfo { volume_name.pop(); unsafe { QueryDosDeviceW( - OsString::from(volume_name.clone()) - .as_os_str() + OsStr::new(&volume_name) .encode_wide() .chain(Some(0)) .skip(4) @@ -445,9 +436,11 @@ pub fn read_fs_list() -> Vec { FindFirstVolumeW(volume_name_buf.as_mut_ptr(), volume_name_buf.len() as DWORD) }; if INVALID_HANDLE_VALUE == find_handle { - crash!(EXIT_ERR, "FindFirstVolumeW failed: {}", unsafe { - GetLastError() - }); + crash!( + EXIT_ERR, + "FindFirstVolumeW failed: {}", + IOError::last_os_error() + ); } let mut mounts = Vec::::new(); loop { @@ -466,8 +459,9 @@ pub fn read_fs_list() -> Vec { volume_name_buf.len() as DWORD, ) } { - let err = unsafe { GetLastError() }; - if err != winapi::shared::winerror::ERROR_NO_MORE_FILES { + let err = IOError::last_os_error(); + if err.raw_os_error() != Some(winapi::shared::winerror::ERROR_NO_MORE_FILES as i32) + { crash!(EXIT_ERR, "FindNextVolumeW failed: {}", err); } break; @@ -527,7 +521,7 @@ impl FsUsage { crash!( EXIT_ERR, "GetVolumePathNamesForVolumeNameW failed: {}", - unsafe { GetLastError() } + IOError::last_os_error() ); } @@ -547,9 +541,11 @@ impl FsUsage { }; if 0 == success { // Fails in case of CD for example - //crash!(EXIT_ERR, "GetDiskFreeSpaceW failed: {}", unsafe { - //GetLastError() - //}); + // crash!( + // EXIT_ERR, + // "GetDiskFreeSpaceW failed: {}", + // IOError::last_os_error() + // ); } let bytes_per_cluster = sectors_per_cluster as u64 * bytes_per_sector as u64; diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 02e78ba9b..72083e799 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -143,6 +143,13 @@ pub fn parse_mode(mode: &str) -> Result { } pub fn get_umask() -> u32 { + // There's no portable way to read the umask without changing it. + // We have to replace it and then quickly set it back, hopefully before + // some other thread is affected. + // On modern Linux kernels the current umask could instead be read + // from /proc/self/status. But that's a lot of work. + // SAFETY: umask always succeeds and doesn't operate on memory. Races are + // possible but it can't violate Rust's guarantees. let mask = unsafe { umask(0) }; unsafe { umask(mask) }; mask as u32 diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index fc4b7ed48..d0f530a5a 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -19,6 +19,8 @@ use std::process::ExitStatus as StdExitStatus; use std::thread; use std::time::{Duration, Instant}; +// SAFETY: These functions always succeed and return simple integers. + /// `geteuid()` returns the effective user ID of the calling process. pub fn geteuid() -> uid_t { unsafe { libc::geteuid() } @@ -96,6 +98,9 @@ impl fmt::Display for ExitStatus { /// Missing methods for Child objects pub trait ChildExt { /// Send a signal to a Child process. + /// + /// Caller beware: if the process already exited then you may accidentally + /// send the signal to an unrelated process that recycled the PID. fn send_signal(&mut self, signal: usize) -> io::Result<()>; /// Wait for a process to finish or return after the specified duration. diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 79cc2afc3..3d2d867bd 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -41,7 +41,7 @@ pub use crate::features::fsext; pub use crate::features::ringbuffer; // * (platform-specific) feature-gated modules -// ** non-windows +// ** non-windows (i.e. Unix + Fuchsia) #[cfg(all(not(windows), feature = "mode"))] pub use crate::features::mode; // ** unix-only