From b7d697753c5cf72b62443e206ca99ac8b6ccad99 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sat, 28 Aug 2021 00:38:00 +0200 Subject: [PATCH] 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 | 1 - Cargo.toml | 2 +- src/uu/unlink/Cargo.toml | 1 - src/uu/unlink/src/unlink.rs | 98 ++++++------------------------------ tests/by-util/test_unlink.rs | 40 ++++++++++----- 5 files changed, 42 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 808f62e15..34b2dac52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3197,7 +3197,6 @@ name = "uu_unlink" version = "0.0.7" dependencies = [ "clap", - "libc", "uucore", "uucore_procs", ] diff --git a/Cargo.toml b/Cargo.toml index 3a2c5f12a..58b6aa52a 100644 --- a/Cargo.toml +++ b/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: diff --git a/src/uu/unlink/Cargo.toml b/src/uu/unlink/Cargo.toml index 3f13a7231..558d18422 100644 --- a/src/uu/unlink/Cargo.toml +++ b/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" } diff --git a/src/uu/unlink/src/unlink.rs b/src/uu/unlink/src/unlink.rs index 7c66708e0..1b4e4c998 100644 --- a/src/uu/unlink/src/unlink.rs +++ b/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 libc::{lstat, stat, unlink}; -use libc::{S_IFLNK, S_IFMT, S_IFREG}; -use std::ffi::CString; -use std::io::{Error, ErrorKind}; -use uucore::display::Quotable; -use uucore::InvalidEncodingHandling; +use std::fs::remove_file; +use std::path::Path; -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 { - format!("{} [OPTION]... FILE", uucore::execution_phrase()) -} +#[uucore_procs::gen_uumain] +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 args = args - .collect_str(InvalidEncodingHandling::ConvertLossy) - .accept_any(); + let path: &Path = matches.value_of_os(OPT_PATH).unwrap().as_ref(); - let usage = usage(); - - let matches = uu_app().usage(&usage[..]).get_matches_from(args); - - let paths: Vec = 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 + remove_file(path).map_err_context(|| format!("cannot unlink {}", path.quote())) } 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)) } diff --git a/tests/by-util/test_unlink.rs b/tests/by-util/test_unlink.rs index 36c978734..6b4fc41da 100644 --- a/tests/by-util/test_unlink.rs +++ b/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!( - "{0}: extra operand: 'test_unlink_multiple_file_b'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + ucmd.arg(file_a) + .arg(file_b) + .fails() + .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( - "unlink: cannot unlink 'test_unlink_empty_directory': Not a regular file \ - or symlink\n", + let res = ucmd.arg(dir).fails(); + let stderr = res.stderr_str(); + 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( - "unlink: Cannot stat 'test_unlink_nonexistent': No such file or directory \ - (os error 2)\n", - ); + new_ucmd!() + .arg(file) + .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")); }