unlink: Simplify, remove unsafe, move to core

This makes it no longer possible to pass multiple filenames, but every
other implementation I tried (GNU, busybox, FreeBSD, sbase) also
forbids that so I think it's for the best.

Cargo.lock

@@ -3197,7 +3197,6 @@ name = "uu_unlink"
 version = "0.0.7"
 dependencies = [
  "clap",
- "libc",
  "uucore",
  "uucore_procs",
 ]

Cargo.toml

@@ -98,6 +98,7 @@ feat_common_core = [
     "touch",
     "unexpand",
     "uniq",
+    "unlink",
     "wc",
     "yes",
 ]
@@ -182,7 +183,6 @@ feat_require_unix = [
     "timeout",
     "tty",
     "uname",
-    "unlink",
 ]
 # "feat_require_unix_utmpx" == set of utilities requiring unix utmp/utmpx support
 # * ref: <https://wiki.musl-libc.org/faq.html#Q:-Why-is-the-utmp/wtmp-functionality-only-implemented-as-stubs?>

src/uu/unlink/Cargo.toml

@@ -16,7 +16,6 @@ path = "src/unlink.rs"
 
 [dependencies]
 clap = { version = "2.33", features = ["wrap_help"] }
-libc = "0.2.42"
 uucore = { version=">=0.0.9", package="uucore", path="../../uucore" }
 uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" }
 

src/uu/unlink/src/unlink.rs

@@ -7,102 +7,32 @@
 
 /* last synced with: unlink (GNU coreutils) 8.21 */
 
-// spell-checker:ignore (ToDO) lstat IFLNK IFMT IFREG
-
 #[macro_use]
 extern crate uucore;
 
-use clap::{crate_version, App, Arg};
+use std::fs::remove_file;
-use libc::{lstat, stat, unlink};
+use std::path::Path;
-use libc::{S_IFLNK, S_IFMT, S_IFREG};
-use std::ffi::CString;
-use std::io::{Error, ErrorKind};
-use uucore::display::Quotable;
-use uucore::InvalidEncodingHandling;
 
-static ABOUT: &str = "Unlink the file at [FILE].";
+use clap::{crate_version, App, Arg};
+
+use uucore::display::Quotable;
+use uucore::error::{FromIo, UResult};
+
+static ABOUT: &str = "Unlink the file at FILE.";
 static OPT_PATH: &str = "FILE";
 
-fn usage() -> String {
+#[uucore_procs::gen_uumain]
-    format!("{} [OPTION]... FILE", uucore::execution_phrase())
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-}
+    let matches = uu_app().get_matches_from(args);
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+    let path: &Path = matches.value_of_os(OPT_PATH).unwrap().as_ref();
-    let args = args
-        .collect_str(InvalidEncodingHandling::ConvertLossy)
-        .accept_any();
 
-    let usage = usage();
+    remove_file(path).map_err_context(|| format!("cannot unlink {}", path.quote()))
-
-    let matches = uu_app().usage(&usage[..]).get_matches_from(args);
-
-    let paths: Vec<String> = matches
-        .values_of(OPT_PATH)
-        .map(|v| v.map(ToString::to_string).collect())
-        .unwrap_or_default();
-
-    if paths.is_empty() {
-        crash!(
-            1,
-            "missing operand\nTry '{0} --help' for more information.",
-            uucore::execution_phrase()
-        );
-    } else if paths.len() > 1 {
-        crash!(
-            1,
-            "extra operand: '{1}'\nTry '{0} --help' for more information.",
-            uucore::execution_phrase(),
-            paths[1]
-        );
-    }
-
-    let c_string = CString::new(paths[0].clone()).unwrap(); // unwrap() cannot fail, the string comes from argv so it cannot contain a \0.
-
-    let st_mode = {
-        #[allow(deprecated)]
-        let mut buf: stat = unsafe { std::mem::uninitialized() };
-        let result = unsafe { lstat(c_string.as_ptr(), &mut buf as *mut stat) };
-
-        if result < 0 {
-            crash!(
-                1,
-                "Cannot stat {}: {}",
-                paths[0].quote(),
-                Error::last_os_error()
-            );
-        }
-
-        buf.st_mode & S_IFMT
-    };
-
-    let result = if st_mode != S_IFREG && st_mode != S_IFLNK {
-        Err(Error::new(
-            ErrorKind::Other,
-            "Not a regular file or symlink",
-        ))
-    } else {
-        let result = unsafe { unlink(c_string.as_ptr()) };
-
-        if result < 0 {
-            Err(Error::last_os_error())
-        } else {
-            Ok(())
-        }
-    };
-
-    match result {
-        Ok(_) => (),
-        Err(e) => {
-            crash!(1, "cannot unlink '{0}': {1}", paths[0], e);
-        }
-    }
-
-    0
 }
 
 pub fn uu_app() -> App<'static, 'static> {
     App::new(uucore::util_name())
         .version(crate_version!())
         .about(ABOUT)
-        .arg(Arg::with_name(OPT_PATH).hidden(true).multiple(true))
+        .arg(Arg::with_name(OPT_PATH).required(true).hidden(true))
 }

tests/by-util/test_unlink.rs

@@ -23,23 +23,24 @@ fn test_unlink_multiple_files() {
     at.touch(file_a);
     at.touch(file_b);
 
-    ucmd.arg(file_a).arg(file_b).fails().stderr_is(&format!(
+    ucmd.arg(file_a)
-        "{0}: extra operand: 'test_unlink_multiple_file_b'\nTry '{1} {0} --help' for more information.",
+        .arg(file_b)
-        ts.util_name,
+        .fails()
-        ts.bin_path.to_string_lossy()
+        .stderr_contains("USAGE");
-    ));
 }
 
 #[test]
 fn test_unlink_directory() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_unlink_empty_directory";
+    let dir = "dir";
 
     at.mkdir(dir);
 
-    ucmd.arg(dir).fails().stderr_is(
+    let res = ucmd.arg(dir).fails();
-        "unlink: cannot unlink 'test_unlink_empty_directory': Not a regular file \
+    let stderr = res.stderr_str();
-         or symlink\n",
+    assert!(
+        stderr == "unlink: cannot unlink 'dir': Is a directory\n"
+            || stderr == "unlink: cannot unlink 'dir': Permission denied\n"
     );
 }
 
@@ -47,8 +48,21 @@ fn test_unlink_directory() {
 fn test_unlink_nonexistent() {
     let file = "test_unlink_nonexistent";
 
-    new_ucmd!().arg(file).fails().stderr_is(
+    new_ucmd!()
-        "unlink: Cannot stat 'test_unlink_nonexistent': No such file or directory \
+        .arg(file)
-         (os error 2)\n",
+        .fails()
-    );
+        .stderr_is("unlink: cannot unlink 'test_unlink_nonexistent': No such file or directory\n");
+}
+
+#[test]
+fn test_unlink_symlink() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.touch("foo");
+    at.symlink_file("foo", "bar");
+
+    ucmd.arg("bar").succeeds().no_stderr();
+
+    assert!(at.file_exists("foo"));
+    assert!(!at.file_exists("bar"));
 }