Merge pull request #2679 from blyxxyz/uucore-inspection

uucore: Minor cleanup

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;
 

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())
+        OsStr::new(&$str)
-            .as_os_str()
             .encode_wide()
             .chain(Some(0))
             .collect::<Vec<u16>>()
@@ -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()) };
+    let len = buf.iter().position(|&n| n == 0).unwrap();
-    OsString::from_wide(&buf[..len as usize])
+    String::from_utf16(&buf[..len]).unwrap()
-        .into_string()
-        .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();
+            if let Ok(stat) = std::fs::metadata(&self.mount_dir) {
-            unsafe {
+                // Why do we cast this to i32?
-                let mut stat = mem::zeroed();
+                self.dev_id = (stat.dev() as i32).to_string()
-                if libc::stat(path.as_ptr(), &mut stat) == 0 {
+            } else {
-                    self.dev_id = (stat.st_dev as i32).to_string();
+                self.dev_id = "".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())
+                OsStr::new(&volume_name)
-                    .as_os_str()
                     .encode_wide()
                     .chain(Some(0))
                     .skip(4)
@@ -445,9 +436,11 @@ pub fn read_fs_list() -> Vec<MountInfo> {
             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 {
+            crash!(
-                GetLastError()
+                EXIT_ERR,
-            });
+                "FindFirstVolumeW failed: {}",
+                IOError::last_os_error()
+            );
         }
         let mut mounts = Vec::<MountInfo>::new();
         loop {
@@ -466,8 +459,9 @@ pub fn read_fs_list() -> Vec<MountInfo> {
                     volume_name_buf.len() as DWORD,
                 )
             } {
-                let err = unsafe { GetLastError() };
+                let err = IOError::last_os_error();
-                if err != winapi::shared::winerror::ERROR_NO_MORE_FILES {
+                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 {
+            // crash!(
-            //GetLastError()
+            //     EXIT_ERR,
-            //});
+            //     "GetDiskFreeSpaceW failed: {}",
+            //     IOError::last_os_error()
+            // );
         }
 
         let bytes_per_cluster = sectors_per_cluster as u64 * bytes_per_sector as u64;

src/uucore/src/lib/features/mode.rs

@@ -143,6 +143,13 @@ pub fn parse_mode(mode: &str) -> Result<mode_t, String> {
 }
 
 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

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.

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