Merge branch 'master' of github.com:uutils/coreutils into hbina-tr-reimplement-expansion

src/uu/expr/src/expr.rs

@@ -9,6 +9,7 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
+use uucore::error::{UResult, USimpleError};
 use uucore::InvalidEncodingHandling;
 
 mod syntax_tree;
@@ -23,7 +24,8 @@ pub fn uu_app() -> App<'static, 'static> {
         .arg(Arg::with_name(HELP).long(HELP))
 }
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let args = args
         .collect_str(InvalidEncodingHandling::ConvertLossy)
         .accept_any();
@@ -32,13 +34,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     // The following usage should work without escaping hyphens: `expr -15 = 1 +  2 \* \( 3 - -4 \)`
 
     if maybe_handle_help_or_version(&args) {
-        0
+        Ok(())
     } else {
         let token_strings = args[1..].to_vec();
 
         match process_expr(&token_strings) {
             Ok(expr_result) => print_expr_ok(&expr_result),
-            Err(expr_error) => print_expr_error(&expr_error),
+            Err(expr_error) => Err(USimpleError::new(2, &expr_error)),
         }
     }
 }
@@ -49,19 +51,15 @@ fn process_expr(token_strings: &[String]) -> Result<String, String> {
     evaluate_ast(maybe_ast)
 }
 
-fn print_expr_ok(expr_result: &str) -> i32 {
+fn print_expr_ok(expr_result: &str) -> UResult<()> {
     println!("{}", expr_result);
     if expr_result == "0" || expr_result.is_empty() {
-        1
+        Err(1.into())
     } else {
-        0
+        Ok(())
     }
 }
 
-fn print_expr_error(expr_error: &str) -> ! {
-    crash!(2, "{}", expr_error)
-}
-
 fn evaluate_ast(maybe_ast: Result<Box<syntax_tree::AstNode>, String>) -> Result<String, String> {
     maybe_ast.and_then(|ast| ast.evaluate())
 }

src/uu/kill/src/kill.rs

@@ -158,18 +158,13 @@ fn print_signal(signal_name_or_value: &str) -> UResult<()> {
 }
 
 fn print_signals() {
-    let mut pos = 0;
     for (idx, signal) in ALL_SIGNALS.iter().enumerate() {
-        pos += signal.len();
+        if idx > 0 {
-        print!("{}", signal);
-        if idx > 0 && pos > 73 {
-            println!();
-            pos = 0;
-        } else {
-            pos += 1;
             print!(" ");
         }
+        print!("{}", signal);
     }
+    println!();
 }
 
 fn list(arg: Option<String>) -> UResult<()> {

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

tests/by-util/test_kill.rs

@@ -56,6 +56,12 @@ fn test_kill_list_all_signals() {
         .stdout_contains("HUP");
 }
 
+#[test]
+fn test_kill_list_final_new_line() {
+    let re = Regex::new("\\n$").unwrap();
+    assert!(re.is_match(new_ucmd!().arg("-l").succeeds().stdout_str()));
+}
+
 #[test]
 fn test_kill_list_all_signals_as_table() {
     // Check for a few signals.  Do not try to be comprehensive.

tests/common/util.rs

@@ -1169,7 +1169,7 @@ pub fn check_coreutil_version(
                 if s.contains(&format!("(GNU coreutils) {}", version_expected)) {
                     Ok(format!("{}: {}", UUTILS_INFO, s.to_string()))
                 } else if s.contains("(GNU coreutils)") {
-                    let version_found = s.split_whitespace().last().unwrap()[..4].parse::<f32>().unwrap_or_default();
+                    let version_found = parse_coreutil_version(s);
                     let version_expected = version_expected.parse::<f32>().unwrap_or_default();
                     if version_found > version_expected {
                     Ok(format!("{}: version for the reference coreutil '{}' is higher than expected; expected: {}, found: {}", UUTILS_INFO, util_name, version_expected, version_found))
@@ -1182,6 +1182,20 @@ pub fn check_coreutil_version(
         )
 }
 
+// simple heuristic to parse the coreutils SemVer string, e.g. "id (GNU coreutils) 8.32.263-0475"
+fn parse_coreutil_version(version_string: &str) -> f32 {
+    version_string
+        .split_whitespace()
+        .last()
+        .unwrap()
+        .split('.')
+        .take(2)
+        .collect::<Vec<_>>()
+        .join(".")
+        .parse::<f32>()
+        .unwrap_or_default()
+}
+
 /// This runs the GNU coreutils `util_name` binary in `$PATH` in order to
 /// dynamically gather reference values on the system.
 /// If the `util_name` in `$PATH` doesn't include a coreutils version string,
@@ -1474,6 +1488,36 @@ mod tests {
         res.normalized_newlines_stdout_is("A\r\nB\nC\n");
     }
 
+    #[test]
+    #[cfg(unix)]
+    fn test_parse_coreutil_version() {
+        use std::assert_eq;
+        assert_eq!(
+            parse_coreutil_version("id (GNU coreutils) 9.0.123-0123").to_string(),
+            "9"
+        );
+        assert_eq!(
+            parse_coreutil_version("id (GNU coreutils) 8.32.263-0475").to_string(),
+            "8.32"
+        );
+        assert_eq!(
+            parse_coreutil_version("id (GNU coreutils) 8.25.123-0123").to_string(),
+            "8.25"
+        );
+        assert_eq!(
+            parse_coreutil_version("id (GNU coreutils) 9.0").to_string(),
+            "9"
+        );
+        assert_eq!(
+            parse_coreutil_version("id (GNU coreutils) 8.32").to_string(),
+            "8.32"
+        );
+        assert_eq!(
+            parse_coreutil_version("id (GNU coreutils) 8.25").to_string(),
+            "8.25"
+        );
+    }
+
     #[test]
     #[cfg(unix)]
     fn test_check_coreutil_version() {