diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 42c448561..a8ed1b704 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -5,7 +5,7 @@ name: CICD # spell-checker:ignore (jargon) SHAs deps softprops toolchain # spell-checker:ignore (names) CodeCOV MacOS MinGW Peltoche rivy # spell-checker:ignore (shell/tools) choco clippy dmake dpkg esac fakeroot gmake grcov halium lcov libssl mkdir popd printf pushd rustc rustfmt rustup shopt xargs -# spell-checker:ignore (misc) aarch alnum armhf bindir busytest coreutils gnueabihf issuecomment maint nullglob onexitbegin onexitend tempfile testsuite uutils +# spell-checker:ignore (misc) aarch alnum armhf bindir busytest coreutils gnueabihf issuecomment maint nullglob onexitbegin onexitend runtest tempfile testsuite uutils env: PROJECT_NAME: coreutils @@ -32,12 +32,12 @@ jobs: shell: bash run: | ## VARs setup + outputs() { for var in "$@" ; do echo steps.vars.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # target-specific options # * CARGO_FEATURES_OPTION CARGO_FEATURES_OPTION='' ; if [ -n "${{ matrix.job.features }}" ]; then CARGO_FEATURES_OPTION='--features "${{ matrix.job.features }}"' ; fi - echo set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} - echo ::set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} + outputs CARGO_FEATURES_OPTION - name: Install `rust` toolchain uses: actions-rs/toolchain@v1 with: @@ -93,12 +93,12 @@ jobs: shell: bash run: | ## VARs setup + outputs() { for var in "$@" ; do echo steps.vars.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # target-specific options # * CARGO_FEATURES_OPTION CARGO_FEATURES_OPTION='' ; if [ -n "${{ matrix.job.features }}" ]; then CARGO_FEATURES_OPTION='--features "${{ matrix.job.features }}"' ; fi - echo set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} - echo ::set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} + outputs CARGO_FEATURES_OPTION - name: Install `rust` toolchain uses: actions-rs/toolchain@v1 with: @@ -197,7 +197,11 @@ jobs: run: | bindir=$(pwd)/target/debug cd tmp/busybox-*/testsuite - S=$(bindir=$bindir ./runtest) && printf "%s\n" "$S" || { printf "%s\n" "$S" | grep "FAIL:" | sed -e "s/FAIL: /::warning ::Test failure:/g" ; } + ## S=$(bindir=$bindir ./runtest) && printf "%s\n" "$S" || { printf "%s\n" "$S" | grep "FAIL:" | sed -e "s/FAIL: /::warning ::Test failure:/g" ; } + output=$(bindir=$bindir ./runtest 2>&1 || true) + printf "%s\n" "${output}" + n_fails=$(echo "$output" | grep "^FAIL:\s" | wc --lines) + if [ $n_fails -gt 0 ] ; then echo "::warning ::${n_fails}+ test failures" ; fi makefile_build: name: Test the build target of the Makefile @@ -261,22 +265,20 @@ jobs: shell: bash run: | ## VARs setup + outputs() { for var in "$@" ; do echo steps.vars.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # toolchain TOOLCHAIN="stable" ## default to "stable" toolchain # * specify alternate/non-default TOOLCHAIN for *-pc-windows-gnu targets; gnu targets on Windows are broken for the standard *-pc-windows-msvc toolchain (refs: GH:rust-lang/rust#47048, GH:rust-lang/rust#53454, GH:rust-lang/cargo#6754) case ${{ matrix.job.target }} in *-pc-windows-gnu) TOOLCHAIN="stable-${{ matrix.job.target }}" ;; esac; # * use requested TOOLCHAIN if specified if [ -n "${{ matrix.job.toolchain }}" ]; then TOOLCHAIN="${{ matrix.job.toolchain }}" ; fi - echo set-output name=TOOLCHAIN::${TOOLCHAIN:-/false} - echo ::set-output name=TOOLCHAIN::${TOOLCHAIN} + outputs TOOLCHAIN # staging directory STAGING='_staging' - echo set-output name=STAGING::${STAGING} - echo ::set-output name=STAGING::${STAGING} + outputs STAGING # determine EXE suffix EXE_suffix="" ; case '${{ matrix.job.target }}' in *-pc-windows-*) EXE_suffix=".exe" ;; esac; - echo set-output name=EXE_suffix::${EXE_suffix} - echo ::set-output name=EXE_suffix::${EXE_suffix} + outputs EXE_suffix # parse commit reference info echo GITHUB_REF=${GITHUB_REF} echo GITHUB_SHA=${GITHUB_SHA} @@ -284,14 +286,7 @@ jobs: unset REF_BRANCH ; case "${GITHUB_REF}" in refs/heads/*) REF_BRANCH=${GITHUB_REF#refs/heads/} ;; esac; unset REF_TAG ; case "${GITHUB_REF}" in refs/tags/*) REF_TAG=${GITHUB_REF#refs/tags/} ;; esac; REF_SHAS=${GITHUB_SHA:0:8} - echo set-output name=REF_NAME::${REF_NAME} - echo set-output name=REF_BRANCH::${REF_BRANCH} - echo set-output name=REF_TAG::${REF_TAG} - echo set-output name=REF_SHAS::${REF_SHAS} - echo ::set-output name=REF_NAME::${REF_NAME} - echo ::set-output name=REF_BRANCH::${REF_BRANCH} - echo ::set-output name=REF_TAG::${REF_TAG} - echo ::set-output name=REF_SHAS::${REF_SHAS} + outputs REF_NAME REF_BRANCH REF_TAG REF_SHAS # parse target unset TARGET_ARCH case '${{ matrix.job.target }}' in @@ -301,68 +296,50 @@ jobs: i686-*) TARGET_ARCH=i686 ;; x86_64-*) TARGET_ARCH=x86_64 ;; esac; - echo set-output name=TARGET_ARCH::${TARGET_ARCH} - echo ::set-output name=TARGET_ARCH::${TARGET_ARCH} unset TARGET_OS ; case '${{ matrix.job.target }}' in *-linux-*) TARGET_OS=linux ;; *-apple-*) TARGET_OS=macos ;; *-windows-*) TARGET_OS=windows ;; esac; - echo set-output name=TARGET_OS::${TARGET_OS} - echo ::set-output name=TARGET_OS::${TARGET_OS} + outputs TARGET_ARCH TARGET_OS # package name PKG_suffix=".tar.gz" ; case '${{ matrix.job.target }}' in *-pc-windows-*) PKG_suffix=".zip" ;; esac; PKG_BASENAME=${PROJECT_NAME}-${REF_TAG:-$REF_SHAS}-${{ matrix.job.target }} PKG_NAME=${PKG_BASENAME}${PKG_suffix} - echo set-output name=PKG_suffix::${PKG_suffix} - echo set-output name=PKG_BASENAME::${PKG_BASENAME} - echo set-output name=PKG_NAME::${PKG_NAME} - echo ::set-output name=PKG_suffix::${PKG_suffix} - echo ::set-output name=PKG_BASENAME::${PKG_BASENAME} - echo ::set-output name=PKG_NAME::${PKG_NAME} + outputs PKG_suffix PKG_BASENAME PKG_NAME # deployable tag? (ie, leading "vM" or "M"; M == version number) unset DEPLOY ; if [[ $REF_TAG =~ ^[vV]?[0-9].* ]]; then DEPLOY='true' ; fi - echo set-output name=DEPLOY::${DEPLOY:-/false} - echo ::set-output name=DEPLOY::${DEPLOY} + outputs DEPLOY # DPKG architecture? unset DPKG_ARCH case ${{ matrix.job.target }} in x86_64-*-linux-*) DPKG_ARCH=amd64 ;; *-linux-*) DPKG_ARCH=${TARGET_ARCH} ;; esac - echo set-output name=DPKG_ARCH::${DPKG_ARCH} - echo ::set-output name=DPKG_ARCH::${DPKG_ARCH} + outputs DPKG_ARCH # DPKG version? unset DPKG_VERSION ; if [[ $REF_TAG =~ ^[vV]?[0-9].* ]]; then DPKG_VERSION=${REF_TAG/#[vV]/} ; fi - echo set-output name=DPKG_VERSION::${DPKG_VERSION} - echo ::set-output name=DPKG_VERSION::${DPKG_VERSION} + outputs DPKG_VERSION # DPKG base name/conflicts? DPKG_BASENAME=${PROJECT_NAME} DPKG_CONFLICTS=${PROJECT_NAME}-musl case ${{ matrix.job.target }} in *-musl) DPKG_BASENAME=${PROJECT_NAME}-musl ; DPKG_CONFLICTS=${PROJECT_NAME} ;; esac; - echo set-output name=DPKG_BASENAME::${DPKG_BASENAME} - echo set-output name=DPKG_CONFLICTS::${DPKG_CONFLICTS} - echo ::set-output name=DPKG_BASENAME::${DPKG_BASENAME} - echo ::set-output name=DPKG_CONFLICTS::${DPKG_CONFLICTS} + outputs DPKG_BASENAME DPKG_CONFLICTS # DPKG name unset DPKG_NAME; if [[ -n $DPKG_ARCH && -n $DPKG_VERSION ]]; then DPKG_NAME="${DPKG_BASENAME}_${DPKG_VERSION}_${DPKG_ARCH}.deb" ; fi - echo set-output name=DPKG_NAME::${DPKG_NAME} - echo ::set-output name=DPKG_NAME::${DPKG_NAME} + outputs DPKG_NAME # target-specific options # * CARGO_FEATURES_OPTION CARGO_FEATURES_OPTION='' ; if [ -n "${{ matrix.job.features }}" ]; then CARGO_FEATURES_OPTION='--features "${{ matrix.job.features }}"' ; fi - echo set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} - echo ::set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} + outputs CARGO_FEATURES_OPTION # * CARGO_USE_CROSS (truthy) CARGO_USE_CROSS='true' ; case '${{ matrix.job.use-cross }}' in ''|0|f|false|n|no) unset CARGO_USE_CROSS ;; esac; - echo set-output name=CARGO_USE_CROSS::${CARGO_USE_CROSS:-/false} - echo ::set-output name=CARGO_USE_CROSS::${CARGO_USE_CROSS} + outputs CARGO_USE_CROSS # ** pass needed environment into `cross` container (iff `cross` not already configured via "Cross.toml") if [ -n "${CARGO_USE_CROSS}" ] && [ ! -e "Cross.toml" ] ; then printf "[build.env]\npassthrough = [\"CI\"]\n" > Cross.toml fi # * test only library and/or binaries for arm-type targets unset CARGO_TEST_OPTIONS ; case '${{ matrix.job.target }}' in aarch64-* | arm-*) CARGO_TEST_OPTIONS="--bins" ;; esac; - echo set-output name=CARGO_TEST_OPTIONS::${CARGO_TEST_OPTIONS} - echo ::set-output name=CARGO_TEST_OPTIONS::${CARGO_TEST_OPTIONS} + outputs CARGO_TEST_OPTIONS # * executable for `strip`? STRIP="strip" case ${{ matrix.job.target }} in @@ -370,8 +347,7 @@ jobs: arm-*-linux-gnueabihf) STRIP="arm-linux-gnueabihf-strip" ;; *-pc-windows-msvc) STRIP="" ;; esac; - echo set-output name=STRIP::${STRIP:-/false} - echo ::set-output name=STRIP::${STRIP} + outputs STRIP - name: Create all needed build/work directories shell: bash run: | @@ -395,11 +371,12 @@ jobs: shell: bash run: | ## Dependent VARs setup + outputs() { for var in "$@" ; do echo steps.vars.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # * determine sub-crate utility list UTILITY_LIST="$(./util/show-utils.sh ${CARGO_FEATURES_OPTION})" + echo UTILITY_LIST=${UTILITY_LIST} CARGO_UTILITY_LIST_OPTIONS="$(for u in ${UTILITY_LIST}; do echo "-puu_${u}"; done;)" - echo set-output name=UTILITY_LIST::${UTILITY_LIST} - echo ::set-output name=CARGO_UTILITY_LIST_OPTIONS::${CARGO_UTILITY_LIST_OPTIONS} + outputs CARGO_UTILITY_LIST_OPTIONS - name: Install `cargo-tree` # for dependency information uses: actions-rs/install@v0.1 with: @@ -524,34 +501,31 @@ jobs: id: vars shell: bash run: | + ## VARs setup + outputs() { for var in "$@" ; do echo steps.vars.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # toolchain TOOLCHAIN="nightly-${{ env.RUST_COV_SRV }}" ## default to "nightly" toolchain (required for certain required unstable compiler flags) ## !maint: refactor when stable channel has needed support # * specify gnu-type TOOLCHAIN for windows; `grcov` requires gnu-style code coverage data files case ${{ matrix.job.os }} in windows-*) TOOLCHAIN="$TOOLCHAIN-x86_64-pc-windows-gnu" ;; esac; # * use requested TOOLCHAIN if specified if [ -n "${{ matrix.job.toolchain }}" ]; then TOOLCHAIN="${{ matrix.job.toolchain }}" ; fi - echo set-output name=TOOLCHAIN::${TOOLCHAIN} - echo ::set-output name=TOOLCHAIN::${TOOLCHAIN} + outputs TOOLCHAIN # staging directory STAGING='_staging' - echo set-output name=STAGING::${STAGING} - echo ::set-output name=STAGING::${STAGING} + outputs STAGING ## # check for CODECOV_TOKEN availability (work-around for inaccessible 'secrets' object for 'if'; see ) ## # note: CODECOV_TOKEN / HAS_CODECOV_TOKEN is not needed for public repositories when using AppVeyor, Azure Pipelines, CircleCI, GitHub Actions, Travis (see ) ## unset HAS_CODECOV_TOKEN ## if [ -n $CODECOV_TOKEN ]; then HAS_CODECOV_TOKEN='true' ; fi - ## echo set-output name=HAS_CODECOV_TOKEN::${HAS_CODECOV_TOKEN} - ## echo ::set-output name=HAS_CODECOV_TOKEN::${HAS_CODECOV_TOKEN} + ## outputs HAS_CODECOV_TOKEN # target-specific options # * CARGO_FEATURES_OPTION CARGO_FEATURES_OPTION='--all-features' ; ## default to '--all-features' for code coverage if [ -n "${{ matrix.job.features }}" ]; then CARGO_FEATURES_OPTION='--features "${{ matrix.job.features }}"' ; fi - echo set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} - echo ::set-output name=CARGO_FEATURES_OPTION::${CARGO_FEATURES_OPTION} + outputs CARGO_FEATURES_OPTION # * CODECOV_FLAGS CODECOV_FLAGS=$( echo "${{ matrix.job.os }}" | sed 's/[^[:alnum:]]/_/g' ) - echo set-output name=CODECOV_FLAGS::${CODECOV_FLAGS} - echo ::set-output name=CODECOV_FLAGS::${CODECOV_FLAGS} + outputs CODECOV_FLAGS - name: rust toolchain ~ install uses: actions-rs/toolchain@v1 with: @@ -563,12 +537,11 @@ jobs: shell: bash run: | ## Dependent VARs setup + outputs() { for var in "$@" ; do echo steps.vars.outputs.${var}="${!var}"; echo ::set-output name=${var}::${!var}; done; } # * determine sub-crate utility list - # UTILITY_LIST="$(./util/show-utils.sh ${CARGO_FEATURES_OPTION})" - UTILITY_LIST="id" # TODO: remove after debugging + UTILITY_LIST="$(./util/show-utils.sh ${CARGO_FEATURES_OPTION})" CARGO_UTILITY_LIST_OPTIONS="$(for u in ${UTILITY_LIST}; do echo "-puu_${u}"; done;)" - echo set-output name=UTILITY_LIST::${UTILITY_LIST} - echo ::set-output name=CARGO_UTILITY_LIST_OPTIONS::${CARGO_UTILITY_LIST_OPTIONS} + outputs CARGO_UTILITY_LIST_OPTIONS - name: Test uucore uses: actions-rs/cargo@v1 with: @@ -607,7 +580,7 @@ jobs: with: crate: grcov version: latest - use-tool-cache: true + use-tool-cache: false - name: Generate coverage data (via `grcov`) id: coverage shell: bash diff --git a/.vscode/cspell.dictionaries/people.wordlist.txt b/.vscode/cspell.dictionaries/people.wordlist.txt index 01cfa4a3e..0a091633f 100644 --- a/.vscode/cspell.dictionaries/people.wordlist.txt +++ b/.vscode/cspell.dictionaries/people.wordlist.txt @@ -97,6 +97,9 @@ Michael Debertol Michael Gehring Michael Gehring +Mitchell Mebane + Mitchell + Mebane Morten Olsen Lysgaard Morten Olsen diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index e2a864f9c..ed634dffb 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -7,6 +7,7 @@ advapi advapi32-sys aho-corasick backtrace +blake2b_simd bstr byteorder chacha @@ -47,6 +48,7 @@ xattr # * rust/rustc RUSTDOCFLAGS RUSTFLAGS +bitor # BitOr trait function bitxor # BitXor trait function clippy concat diff --git a/Cargo.lock b/Cargo.lock index 43d491cef..089d30554 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6,16 +6,6 @@ version = "0.11.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fe438c63458706e03479442743baae6c88256498e6431708f6dfc520a26515d3" -[[package]] -name = "advapi32-sys" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e06588080cb19d0acb6739808aafa5f26bfb2ca015b2b6370028b44cf7cb8a9a" -dependencies = [ - "winapi 0.2.8", - "winapi-build", -] - [[package]] name = "aho-corasick" version = "0.7.18" @@ -1772,6 +1762,7 @@ dependencies = [ name = "uu_cat" version = "0.0.6" dependencies = [ + "atty", "clap", "nix 0.20.0", "thiserror", @@ -1784,6 +1775,7 @@ dependencies = [ name = "uu_chgrp" version = "0.0.6" dependencies = [ + "clap", "uucore", "uucore_procs", "walkdir", @@ -1872,6 +1864,7 @@ dependencies = [ name = "uu_cut" version = "0.0.6" dependencies = [ + "atty", "bstr", "clap", "memchr 2.4.0", @@ -2262,6 +2255,7 @@ dependencies = [ name = "uu_nohup" version = "0.0.6" dependencies = [ + "atty", "clap", "libc", "uucore", @@ -2420,6 +2414,7 @@ dependencies = [ "uucore", "uucore_procs", "walkdir", + "winapi 0.3.9", ] [[package]] @@ -2601,7 +2596,6 @@ name = "uu_timeout" version = "0.0.6" dependencies = [ "clap", - "getopts", "libc", "uucore", "uucore_procs", @@ -2659,6 +2653,7 @@ dependencies = [ name = "uu_tty" version = "0.0.6" dependencies = [ + "atty", "clap", "libc", "uucore", @@ -2750,7 +2745,6 @@ dependencies = [ name = "uu_whoami" version = "0.0.6" dependencies = [ - "advapi32-sys", "clap", "uucore", "uucore_procs", diff --git a/Cargo.toml b/Cargo.toml index 19ebca511..804c5f978 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -351,7 +351,7 @@ time = "0.1" unindent = "0.1" uucore = { version=">=0.0.8", package="uucore", path="src/uucore", features=["entries", "process"] } walkdir = "2.2" -atty = "0.2.14" +atty = "0.2" [target.'cfg(unix)'.dev-dependencies] rlimit = "0.4.0" diff --git a/GNUmakefile b/GNUmakefile index 102856b66..e5ad01340 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -268,11 +268,11 @@ test: ${CARGO} test ${CARGOFLAGS} --features "$(TESTS) $(TEST_SPEC_FEATURE)" --no-default-features $(TEST_NO_FAIL_FAST) busybox-src: - if [ ! -e $(BUSYBOX_SRC) ]; then \ - mkdir -p $(BUSYBOX_ROOT); \ - wget https://busybox.net/downloads/busybox-$(BUSYBOX_VER).tar.bz2 -P $(BUSYBOX_ROOT); \ - tar -C $(BUSYBOX_ROOT) -xf $(BUSYBOX_ROOT)/busybox-$(BUSYBOX_VER).tar.bz2; \ - fi; \ + if [ ! -e "$(BUSYBOX_SRC)" ] ; then \ + mkdir -p "$(BUSYBOX_ROOT)" ; \ + wget "https://busybox.net/downloads/busybox-$(BUSYBOX_VER).tar.bz2" -P "$(BUSYBOX_ROOT)" ; \ + tar -C "$(BUSYBOX_ROOT)" -xf "$(BUSYBOX_ROOT)/busybox-$(BUSYBOX_VER).tar.bz2" ; \ + fi ; # This is a busybox-specific config file their test suite wants to parse. $(BUILDDIR)/.config: $(BASEDIR)/.busybox-config @@ -280,10 +280,12 @@ $(BUILDDIR)/.config: $(BASEDIR)/.busybox-config # Test under the busybox test suite $(BUILDDIR)/busybox: busybox-src build-coreutils $(BUILDDIR)/.config - cp $(BUILDDIR)/coreutils $(BUILDDIR)/busybox; \ - chmod +x $@; + cp "$(BUILDDIR)/coreutils" "$(BUILDDIR)/busybox" + chmod +x $@ prepare-busytest: $(BUILDDIR)/busybox + # disable inapplicable tests + -( cd "$(BUSYBOX_SRC)/testsuite" ; if [ -e "busybox.tests" ] ; then mv busybox.tests busybox.tests- ; fi ; ) ifeq ($(EXES),) busytest: diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index f0e187c31..e6a01cb34 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -38,18 +38,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let config_result: Result = base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - - if config_result.is_err() { - match config_result { - Ok(_) => panic!(), - Err(s) => crash!(BASE_CMD_PARSE_ERROR, "{}", s), - } - } + let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let config = config_result.unwrap(); let mut input: Box = base_common::get_input(&config, &stdin_raw); base_common::handle_input( diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index ee5fe8675..256b674e2 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -54,15 +54,13 @@ impl Config { None => None, }; - let cols = match options.value_of(options::WRAP) { - Some(num) => match num.parse::() { - Ok(n) => Some(n), - Err(e) => { - return Err(format!("Invalid wrap size: ‘{}’: {}", num, e)); - } - }, - None => None, - }; + let cols = options + .value_of(options::WRAP) + .map(|num| { + num.parse::() + .map_err(|e| format!("Invalid wrap size: ‘{}’: {}", num, e)) + }) + .transpose()?; Ok(Config { decode: options.is_present(options::DECODE), diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index 810df4fe8..0dd831027 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -38,18 +38,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let name = executable!(); let config_result: Result = base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - - if config_result.is_err() { - match config_result { - Ok(_) => panic!(), - Err(s) => crash!(BASE_CMD_PARSE_ERROR, "{}", s), - } - } + let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let config = config_result.unwrap(); let mut input: Box = base_common::get_input(&config, &stdin_raw); base_common::handle_input( diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index a0eed93f1..098a3e2b2 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -110,7 +110,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let line_ending = if opt_zero { "\0" } else { "\n" }; for path in paths { - print!("{}{}", basename(&path, &suffix), line_ending); + print!("{}{}", basename(path, suffix), line_ending); } 0 @@ -118,14 +118,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { fn basename(fullname: &str, suffix: &str) -> String { // Remove all platform-specific path separators from the end - let mut path: String = fullname - .chars() - .rev() - .skip_while(|&ch| is_separator(ch)) - .collect(); - - // Undo reverse - path = path.chars().rev().collect(); + let path = fullname.trim_end_matches(is_separator); // Convert to path buffer and get last path component let pb = PathBuf::from(path); diff --git a/src/uu/cat/Cargo.toml b/src/uu/cat/Cargo.toml index 09b289253..9218e84fe 100644 --- a/src/uu/cat/Cargo.toml +++ b/src/uu/cat/Cargo.toml @@ -17,6 +17,7 @@ path = "src/cat.rs" [dependencies] clap = "2.33" thiserror = "1.0" +atty = "0.2" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 1f2f441d8..889ba424a 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -20,7 +20,6 @@ use clap::{crate_version, App, Arg}; use std::fs::{metadata, File}; use std::io::{self, Read, Write}; use thiserror::Error; -use uucore::fs::is_stdin_interactive; /// Linux splice support #[cfg(any(target_os = "linux", target_os = "android"))] @@ -295,7 +294,7 @@ fn cat_handle( if options.can_write_fast() { write_fast(handle) } else { - write_lines(handle, &options, state) + write_lines(handle, options, state) } } @@ -306,9 +305,9 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat #[cfg(any(target_os = "linux", target_os = "android"))] file_descriptor: stdin.as_raw_fd(), reader: stdin, - is_interactive: is_stdin_interactive(), + is_interactive: atty::is(atty::Stream::Stdin), }; - return cat_handle(&mut handle, &options, state); + return cat_handle(&mut handle, options, state); } match get_input_type(path)? { InputType::Directory => Err(CatError::IsDirectory), @@ -322,7 +321,7 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat reader: socket, is_interactive: false, }; - cat_handle(&mut handle, &options, state) + cat_handle(&mut handle, options, state) } _ => { let file = File::open(path)?; @@ -332,7 +331,7 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat reader: file, is_interactive: false, }; - cat_handle(&mut handle, &options, state) + cat_handle(&mut handle, options, state) } } } @@ -345,7 +344,7 @@ fn cat_files(files: Vec, options: &OutputOptions) -> Result<(), u32> { }; for path in &files { - if let Err(err) = cat_path(path, &options, &mut state) { + if let Err(err) = cat_path(path, options, &mut state) { show_error!("{}: {}", path, err); error_count += 1; } diff --git a/src/uu/chgrp/Cargo.toml b/src/uu/chgrp/Cargo.toml index 9424ad35e..0e43f7c02 100644 --- a/src/uu/chgrp/Cargo.toml +++ b/src/uu/chgrp/Cargo.toml @@ -15,6 +15,7 @@ edition = "2018" path = "src/chgrp.rs" [dependencies] +clap = "2.33" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "fs", "perms"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } walkdir = "2.2" diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index f6afc2805..454a0386c 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -14,6 +14,8 @@ use uucore::fs::resolve_relative_path; use uucore::libc::gid_t; use uucore::perms::{wrap_chgrp, Verbosity}; +use clap::{App, Arg}; + extern crate walkdir; use walkdir::WalkDir; @@ -24,76 +26,194 @@ use std::os::unix::fs::MetadataExt; use std::path::Path; use uucore::InvalidEncodingHandling; -static SYNTAX: &str = - "chgrp [OPTION]... GROUP FILE...\n or : chgrp [OPTION]... --reference=RFILE FILE..."; -static SUMMARY: &str = "Change the group of each FILE to GROUP."; +static ABOUT: &str = "Change the group of each FILE to GROUP."; +static VERSION: &str = env!("CARGO_PKG_VERSION"); + +pub mod options { + pub mod verbosity { + pub static CHANGES: &str = "changes"; + pub static QUIET: &str = "quiet"; + pub static SILENT: &str = "silent"; + pub static VERBOSE: &str = "verbose"; + } + pub mod preserve_root { + pub static PRESERVE: &str = "preserve-root"; + pub static NO_PRESERVE: &str = "no-preserve-root"; + } + pub mod dereference { + pub static DEREFERENCE: &str = "dereference"; + pub static NO_DEREFERENCE: &str = "no-dereference"; + } + pub static RECURSIVE: &str = "recursive"; + pub mod traverse { + pub static TRAVERSE: &str = "H"; + pub static NO_TRAVERSE: &str = "P"; + pub static EVERY: &str = "L"; + } + pub static REFERENCE: &str = "reference"; + pub static ARG_GROUP: &str = "GROUP"; + pub static ARG_FILES: &str = "FILE"; +} const FTS_COMFOLLOW: u8 = 1; const FTS_PHYSICAL: u8 = 1 << 1; const FTS_LOGICAL: u8 = 1 << 2; +fn get_usage() -> String { + format!( + "{0} [OPTION]... GROUP FILE...\n {0} [OPTION]... --reference=RFILE FILE...", + executable!() + ) +} + pub fn uumain(args: impl uucore::Args) -> i32 { let args = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); - let mut opts = app!(SYNTAX, SUMMARY, ""); - opts.optflag("c", - "changes", - "like verbose but report only when a change is made") - .optflag("f", "silent", "") - .optflag("", "quiet", "suppress most error messages") - .optflag("v", - "verbose", - "output a diagnostic for every file processed") - .optflag("", "dereference", "affect the referent of each symbolic link (this is the default), rather than the symbolic link itself") - .optflag("h", "no-dereference", "affect symbolic links instead of any referenced file (useful only on systems that can change the ownership of a symlink)") - .optflag("", - "no-preserve-root", - "do not treat '/' specially (the default)") - .optflag("", "preserve-root", "fail to operate recursively on '/'") - .optopt("", - "reference", - "use RFILE's owner and group rather than specifying OWNER:GROUP values", - "RFILE") - .optflag("R", - "recursive", - "operate on files and directories recursively") - .optflag("H", - "", - "if a command line argument is a symbolic link to a directory, traverse it") - .optflag("L", - "", - "traverse every symbolic link to a directory encountered") - .optflag("P", "", "do not traverse any symbolic links (default)"); + let usage = get_usage(); - let mut bit_flag = FTS_PHYSICAL; - let mut preserve_root = false; - let mut derefer = -1; - let flags: &[char] = &['H', 'L', 'P']; - for opt in &args { - match opt.as_str() { - // If more than one is specified, only the final one takes effect. - s if s.contains(flags) => { - if let Some(idx) = s.rfind(flags) { - match s.chars().nth(idx).unwrap() { - 'H' => bit_flag = FTS_COMFOLLOW | FTS_PHYSICAL, - 'L' => bit_flag = FTS_LOGICAL, - 'P' => bit_flag = FTS_PHYSICAL, - _ => (), - } - } - } - "--no-preserve-root" => preserve_root = false, - "--preserve-root" => preserve_root = true, - "--dereference" => derefer = 1, - "--no-dereference" => derefer = 0, - _ => (), + let mut app = App::new(executable!()) + .version(VERSION) + .about(ABOUT) + .usage(&usage[..]) + .arg( + Arg::with_name(options::verbosity::CHANGES) + .short("c") + .long(options::verbosity::CHANGES) + .help("like verbose but report only when a change is made"), + ) + .arg( + Arg::with_name(options::verbosity::SILENT) + .short("f") + .long(options::verbosity::SILENT), + ) + .arg( + Arg::with_name(options::verbosity::QUIET) + .long(options::verbosity::QUIET) + .help("suppress most error messages"), + ) + .arg( + Arg::with_name(options::verbosity::VERBOSE) + .short("v") + .long(options::verbosity::VERBOSE) + .help("output a diagnostic for every file processed"), + ) + .arg( + Arg::with_name(options::dereference::DEREFERENCE) + .long(options::dereference::DEREFERENCE), + ) + .arg( + Arg::with_name(options::dereference::NO_DEREFERENCE) + .short("h") + .long(options::dereference::NO_DEREFERENCE) + .help( + "affect symbolic links instead of any referenced file (useful only on systems that can change the ownership of a symlink)", + ), + ) + .arg( + Arg::with_name(options::preserve_root::PRESERVE) + .long(options::preserve_root::PRESERVE) + .help("fail to operate recursively on '/'"), + ) + .arg( + Arg::with_name(options::preserve_root::NO_PRESERVE) + .long(options::preserve_root::NO_PRESERVE) + .help("do not treat '/' specially (the default)"), + ) + .arg( + Arg::with_name(options::REFERENCE) + .long(options::REFERENCE) + .value_name("RFILE") + .help("use RFILE's group rather than specifying GROUP values") + .takes_value(true) + .multiple(false), + ) + .arg( + Arg::with_name(options::RECURSIVE) + .short("R") + .long(options::RECURSIVE) + .help("operate on files and directories recursively"), + ) + .arg( + Arg::with_name(options::traverse::TRAVERSE) + .short(options::traverse::TRAVERSE) + .help("if a command line argument is a symbolic link to a directory, traverse it"), + ) + .arg( + Arg::with_name(options::traverse::NO_TRAVERSE) + .short(options::traverse::NO_TRAVERSE) + .help("do not traverse any symbolic links (default)") + .overrides_with_all(&[options::traverse::TRAVERSE, options::traverse::EVERY]), + ) + .arg( + Arg::with_name(options::traverse::EVERY) + .short(options::traverse::EVERY) + .help("traverse every symbolic link to a directory encountered"), + ); + + // we change the positional args based on whether + // --reference was used. + let mut reference = false; + let mut help = false; + // stop processing options on -- + for arg in args.iter().take_while(|s| *s != "--") { + if arg.starts_with("--reference=") || arg == "--reference" { + reference = true; + } else if arg == "--help" { + // we stop processing once we see --help, + // as it doesn't matter if we've seen reference or not + help = true; + break; } } - let matches = opts.parse(args); - let recursive = matches.opt_present("recursive"); + if help || !reference { + // add both positional arguments + app = app.arg( + Arg::with_name(options::ARG_GROUP) + .value_name(options::ARG_GROUP) + .required(true) + .takes_value(true) + .multiple(false), + ) + } + app = app.arg( + Arg::with_name(options::ARG_FILES) + .value_name(options::ARG_FILES) + .multiple(true) + .takes_value(true) + .required(true) + .min_values(1), + ); + + let matches = app.get_matches_from(args); + + /* Get the list of files */ + let files: Vec = matches + .values_of(options::ARG_FILES) + .map(|v| v.map(ToString::to_string).collect()) + .unwrap_or_default(); + + let preserve_root = matches.is_present(options::preserve_root::PRESERVE); + + let mut derefer = if matches.is_present(options::dereference::DEREFERENCE) { + 1 + } else if matches.is_present(options::dereference::NO_DEREFERENCE) { + 0 + } else { + -1 + }; + + let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) { + FTS_COMFOLLOW | FTS_PHYSICAL + } else if matches.is_present(options::traverse::EVERY) { + FTS_LOGICAL + } else { + FTS_PHYSICAL + }; + + let recursive = matches.is_present(options::RECURSIVE); if recursive { if bit_flag == FTS_PHYSICAL { if derefer == 1 { @@ -106,27 +226,20 @@ pub fn uumain(args: impl uucore::Args) -> i32 { bit_flag = FTS_PHYSICAL; } - let verbosity = if matches.opt_present("changes") { + let verbosity = if matches.is_present(options::verbosity::CHANGES) { Verbosity::Changes - } else if matches.opt_present("silent") || matches.opt_present("quiet") { + } else if matches.is_present(options::verbosity::SILENT) + || matches.is_present(options::verbosity::QUIET) + { Verbosity::Silent - } else if matches.opt_present("verbose") { + } else if matches.is_present(options::verbosity::VERBOSE) { Verbosity::Verbose } else { Verbosity::Normal }; - if matches.free.is_empty() { - show_usage_error!("missing operand"); - return 1; - } else if matches.free.len() < 2 && !matches.opt_present("reference") { - show_usage_error!("missing operand after ‘{}’", matches.free[0]); - return 1; - } - - let dest_gid: gid_t; - let mut files; - if let Some(file) = matches.opt_str("reference") { + let dest_gid: u32; + if let Some(file) = matches.value_of(options::REFERENCE) { match fs::metadata(&file) { Ok(meta) => { dest_gid = meta.gid(); @@ -136,19 +249,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { return 1; } } - files = matches.free; } else { - match entries::grp2gid(&matches.free[0]) { + let group = matches.value_of(options::ARG_GROUP).unwrap_or_default(); + match entries::grp2gid(group) { Ok(g) => { dest_gid = g; } _ => { - show_error!("invalid group: {}", matches.free[0].as_str()); + show_error!("invalid group: {}", group); return 1; } } - files = matches.free; - files.remove(0); } let executor = Chgrper { diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 9cdabc7d6..2d5787099 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -127,31 +127,32 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let verbose = matches.is_present(options::VERBOSE); let preserve_root = matches.is_present(options::PRESERVE_ROOT); let recursive = matches.is_present(options::RECURSIVE); - let fmode = - matches - .value_of(options::REFERENCE) - .and_then(|ref fref| match fs::metadata(fref) { - Ok(meta) => Some(meta.mode()), - Err(err) => crash!(1, "cannot stat attributes of '{}': {}", fref, err), - }); + let fmode = matches + .value_of(options::REFERENCE) + .and_then(|fref| match fs::metadata(fref) { + Ok(meta) => Some(meta.mode()), + Err(err) => crash!(1, "cannot stat attributes of '{}': {}", fref, err), + }); let modes = matches.value_of(options::MODE).unwrap(); // should always be Some because required - let mut cmode = if mode_had_minus_prefix { + let cmode = if mode_had_minus_prefix { // clap parsing is finished, now put prefix back - Some(format!("-{}", modes)) + format!("-{}", modes) } else { - Some(modes.to_string()) + modes.to_string() }; let mut files: Vec = matches .values_of(options::FILE) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); - if fmode.is_some() { + let cmode = if fmode.is_some() { // "--reference" and MODE are mutually exclusive // if "--reference" was used MODE needs to be interpreted as another FILE // it wasn't possible to implement this behavior directly with clap - files.push(cmode.unwrap()); - cmode = None; - } + files.push(cmode); + None + } else { + Some(cmode) + }; let chmoder = Chmoder { changes, @@ -230,11 +231,11 @@ impl Chmoder { return Err(1); } if !self.recursive { - r = self.chmod_file(&file).and(r); + r = self.chmod_file(file).and(r); } else { for entry in WalkDir::new(&filename).into_iter().filter_map(|e| e.ok()) { let file = entry.path(); - r = self.chmod_file(&file).and(r); + r = self.chmod_file(file).and(r); } } } diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 2bb5133fe..ab9f10dba 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -220,7 +220,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; let filter = if let Some(spec) = matches.value_of(options::FROM) { - match parse_spec(&spec) { + match parse_spec(spec) { Ok((Some(uid), None)) => IfFrom::User(uid), Ok((None, Some(gid))) => IfFrom::Group(gid), Ok((Some(uid), Some(gid))) => IfFrom::UserGroup(uid, gid), @@ -248,7 +248,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } } else { - match parse_spec(&owner) { + match parse_spec(owner) { Ok((u, g)) => { dest_uid = u; dest_gid = g; @@ -278,37 +278,25 @@ fn parse_spec(spec: &str) -> Result<(Option, Option), String> { let usr_only = args.len() == 1 && !args[0].is_empty(); let grp_only = args.len() == 2 && args[0].is_empty(); let usr_grp = args.len() == 2 && !args[0].is_empty() && !args[1].is_empty(); - - if usr_only { - Ok(( - Some(match Passwd::locate(args[0]) { - Ok(v) => v.uid(), - _ => return Err(format!("invalid user: ‘{}’", spec)), - }), - None, - )) - } else if grp_only { - Ok(( - None, - Some(match Group::locate(args[1]) { - Ok(v) => v.gid(), - _ => return Err(format!("invalid group: ‘{}’", spec)), - }), - )) - } else if usr_grp { - Ok(( - Some(match Passwd::locate(args[0]) { - Ok(v) => v.uid(), - _ => return Err(format!("invalid user: ‘{}’", spec)), - }), - Some(match Group::locate(args[1]) { - Ok(v) => v.gid(), - _ => return Err(format!("invalid group: ‘{}’", spec)), - }), - )) + let uid = if usr_only || usr_grp { + Some( + Passwd::locate(args[0]) + .map_err(|_| format!("invalid user: ‘{}’", spec))? + .uid(), + ) } else { - Ok((None, None)) - } + None + }; + let gid = if grp_only || usr_grp { + Some( + Group::locate(args[1]) + .map_err(|_| format!("invalid group: ‘{}’", spec))? + .gid(), + ) + } else { + None + }; + Ok((uid, gid)) } enum IfFrom { @@ -497,3 +485,17 @@ impl Chowner { } } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_parse_spec() { + assert_eq!(parse_spec(":"), Ok((None, None))); + assert!(parse_spec("::") + .err() + .unwrap() + .starts_with("invalid group: ")); + } +} diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index a05bd4494..86d4a4900 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -28,6 +28,7 @@ mod options { pub const GROUP: &str = "group"; pub const GROUPS: &str = "groups"; pub const USERSPEC: &str = "userspec"; + pub const COMMAND: &str = "command"; } pub fn uumain(args: impl uucore::Args) -> i32 { @@ -39,7 +40,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .version(crate_version!()) .about(ABOUT) .usage(SYNTAX) - .arg(Arg::with_name(options::NEWROOT).hidden(true).required(true)) + .arg( + Arg::with_name(options::NEWROOT) + .hidden(true) + .required(true) + .index(1), + ) .arg( Arg::with_name(options::USER) .short("u") @@ -71,6 +77,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ) .value_name("USER:GROUP"), ) + .arg( + Arg::with_name(options::COMMAND) + .hidden(true) + .multiple(true) + .index(2), + ) .get_matches_from(args); let default_shell: &'static str = "/bin/sh"; @@ -94,7 +106,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ); } - let command: Vec<&str> = match matches.args.len() { + let commands = match matches.values_of(options::COMMAND) { + Some(v) => v.collect(), + None => vec![], + }; + + // TODO: refactor the args and command matching + // See: https://github.com/uutils/coreutils/pull/2365#discussion_r647849967 + let command: Vec<&str> = match commands.len() { 1 => { let shell: &str = match user_shell { Err(_) => default_shell, @@ -102,17 +121,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; vec![shell, default_option] } - _ => { - let mut vector: Vec<&str> = Vec::new(); - for (&k, v) in matches.args.iter() { - vector.push(k); - vector.push(&v.vals[0].to_str().unwrap()); - } - vector - } + _ => commands, }; - set_context(&newroot, &matches); + set_context(newroot, &matches); let pstatus = Command::new(command[0]) .args(&command[1..]) @@ -132,7 +144,7 @@ fn set_context(root: &Path, options: &clap::ArgMatches) { let group_str = options.value_of(options::GROUP).unwrap_or_default(); let groups_str = options.value_of(options::GROUPS).unwrap_or_default(); let userspec = match userspec_str { - Some(ref u) => { + Some(u) => { let s: Vec<&str> = u.split(':').collect(); if s.len() != 2 || s.iter().any(|&spec| spec.is_empty()) { crash!(1, "invalid userspec: `{}`", u) diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index 49c0536f5..6a812c186 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -160,18 +160,14 @@ fn cksum(fname: &str) -> io::Result<(u32, usize)> { let mut bytes = init_byte_array(); loop { - match rd.read(&mut bytes) { - Ok(num_bytes) => { - if num_bytes == 0 { - return Ok((crc_final(crc, size), size)); - } - for &b in bytes[..num_bytes].iter() { - crc = crc_update(crc, b); - } - size += num_bytes; - } - Err(err) => return Err(err), + let num_bytes = rd.read(&mut bytes)?; + if num_bytes == 0 { + return Ok((crc_final(crc, size), size)); } + for &b in bytes[..num_bytes].iter() { + crc = crc_update(crc, b); + } + size += num_bytes; } } diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index f7190fb73..7a6086bb5 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -50,9 +50,8 @@ fn mkdelim(col: usize, opts: &ArgMatches) -> String { } fn ensure_nl(line: &mut String) { - match line.chars().last() { - Some('\n') => (), - _ => line.push('\n'), + if !line.ends_with('\n') { + line.push('\n'); } } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 6a114cf44..a87e86b98 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -709,27 +709,26 @@ fn parse_path_args(path_args: &[String], options: &Options) -> CopyResult<(Vec { // All path args are sources, and the target dir was // specified separately - (paths, PathBuf::from(target)) + PathBuf::from(target) } None => { // If there was no explicit target-dir, then use the last // path_arg - let target = paths.pop().unwrap(); - (paths, target) + paths.pop().unwrap() } }; if options.strip_trailing_slashes { - for source in sources.iter_mut() { + for source in paths.iter_mut() { *source = source.components().as_path().to_owned() } } - Ok((sources, target)) + Ok((paths, target)) } fn preserve_hardlinks( @@ -1088,7 +1087,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu } #[cfg(not(windows))] -#[allow(clippy::unnecessary_unwrap)] // needed for windows version +#[allow(clippy::unnecessary_wraps)] // needed for windows version fn symlink_file(source: &Path, dest: &Path, context: &str) -> CopyResult<()> { match std::os::unix::fs::symlink(source, dest).context(context) { Ok(_) => Ok(()), @@ -1271,15 +1270,15 @@ fn copy_on_write_linux(source: &Path, dest: &Path, mode: ReflinkMode) -> CopyRes ReflinkMode::Always => unsafe { let result = ficlone(dst_file.as_raw_fd(), src_file.as_raw_fd() as *const i32); if result != 0 { - return Err(format!( + Err(format!( "failed to clone {:?} from {:?}: {}", source, dest, std::io::Error::last_os_error() ) - .into()); + .into()) } else { - return Ok(()); + Ok(()) } }, ReflinkMode::Auto => unsafe { @@ -1287,11 +1286,10 @@ fn copy_on_write_linux(source: &Path, dest: &Path, mode: ReflinkMode) -> CopyRes if result != 0 { fs::copy(source, dest).context(&*context_for(source, dest))?; } + Ok(()) }, ReflinkMode::Never => unreachable!(), } - - Ok(()) } /// Copies `source` to `dest` using copy-on-write if possible. diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index e3b2069ab..d69254a3a 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -92,7 +92,7 @@ where T: BufRead, { let mut input_iter = InputSplitter::new(input.lines().enumerate()); - let mut split_writer = SplitWriter::new(&options); + let mut split_writer = SplitWriter::new(options); let ret = do_csplit(&mut split_writer, patterns, &mut input_iter); // consume the rest diff --git a/src/uu/csplit/src/patterns.rs b/src/uu/csplit/src/patterns.rs index 5621d18a3..4ab7862ac 100644 --- a/src/uu/csplit/src/patterns.rs +++ b/src/uu/csplit/src/patterns.rs @@ -133,20 +133,12 @@ fn extract_patterns(args: &[String]) -> Result, CsplitError> { Some(m) => m.as_str().parse().unwrap(), }; if let Some(up_to_match) = captures.name("UPTO") { - let pattern = match Regex::new(up_to_match.as_str()) { - Err(_) => { - return Err(CsplitError::InvalidPattern(arg.to_string())); - } - Ok(reg) => reg, - }; + let pattern = Regex::new(up_to_match.as_str()) + .map_err(|_| CsplitError::InvalidPattern(arg.to_string()))?; patterns.push(Pattern::UpToMatch(pattern, offset, execute_ntimes)); } else if let Some(skip_to_match) = captures.name("SKIPTO") { - let pattern = match Regex::new(skip_to_match.as_str()) { - Err(_) => { - return Err(CsplitError::InvalidPattern(arg.to_string())); - } - Ok(reg) => reg, - }; + let pattern = Regex::new(skip_to_match.as_str()) + .map_err(|_| CsplitError::InvalidPattern(arg.to_string()))?; patterns.push(Pattern::SkipToMatch(pattern, offset, execute_ntimes)); } } else if let Ok(line_number) = arg.parse::() { diff --git a/src/uu/csplit/src/split_name.rs b/src/uu/csplit/src/split_name.rs index 6db781e9b..758216414 100644 --- a/src/uu/csplit/src/split_name.rs +++ b/src/uu/csplit/src/split_name.rs @@ -33,13 +33,13 @@ impl SplitName { // get the prefix let prefix = prefix_opt.unwrap_or_else(|| "xx".to_string()); // the width for the split offset - let n_digits = match n_digits_opt { - None => 2, - Some(opt) => match opt.parse::() { - Ok(digits) => digits, - Err(_) => return Err(CsplitError::InvalidNumber(opt)), - }, - }; + let n_digits = n_digits_opt + .map(|opt| { + opt.parse::() + .map_err(|_| CsplitError::InvalidNumber(opt)) + }) + .transpose()? + .unwrap_or(2); // translate the custom format into a function let fn_split_name: Box String> = match format_opt { None => Box::new(move |n: usize| -> String { diff --git a/src/uu/cut/Cargo.toml b/src/uu/cut/Cargo.toml index c863c1772..47b8223c5 100644 --- a/src/uu/cut/Cargo.toml +++ b/src/uu/cut/Cargo.toml @@ -20,6 +20,7 @@ uucore = { version=">=0.0.8", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } memchr = "2" bstr = "0.2" +atty = "0.2" [[bin]] name = "cut" diff --git a/src/uu/cut/src/cut.rs b/src/uu/cut/src/cut.rs index 819cbb989..af4a27d8a 100644 --- a/src/uu/cut/src/cut.rs +++ b/src/uu/cut/src/cut.rs @@ -17,7 +17,6 @@ use std::io::{stdin, stdout, BufReader, BufWriter, Read, Write}; use std::path::Path; use self::searcher::Searcher; -use uucore::fs::is_stdout_interactive; use uucore::ranges::Range; use uucore::InvalidEncodingHandling; @@ -127,7 +126,7 @@ enum Mode { } fn stdout_writer() -> Box { - if is_stdout_interactive() { + if atty::is(atty::Stream::Stdout) { Box::new(stdout()) } else { Box::new(BufWriter::new(stdout())) as Box diff --git a/src/uu/du/Cargo.toml b/src/uu/du/Cargo.toml index 023c0a021..dcd1f720e 100644 --- a/src/uu/du/Cargo.toml +++ b/src/uu/du/Cargo.toml @@ -19,6 +19,8 @@ clap = "2.33" chrono = "0.4" uucore = { version=">=0.0.8", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } + +[target.'cfg(target_os = "windows")'.dependencies] winapi = { version="0.3", features=[] } [[bin]] diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index c5fff2ed7..b7c53eb72 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -1,9 +1,9 @@ -// This file is part of the uutils coreutils package. -// -// (c) Derek Chiang -// -// For the full copyright and license information, please view the LICENSE -// file that was distributed with this source code. +// * This file is part of the uutils coreutils package. +// * +// * (c) Derek Chiang +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. #[macro_use] extern crate uucore; @@ -12,6 +12,7 @@ use chrono::prelude::DateTime; use chrono::Local; use clap::{crate_version, App, Arg}; use std::collections::HashSet; +use std::convert::TryFrom; use std::env; use std::fs; #[cfg(not(windows))] @@ -24,8 +25,11 @@ use std::os::unix::fs::MetadataExt; use std::os::windows::fs::MetadataExt; #[cfg(windows)] use std::os::windows::io::AsRawHandle; +#[cfg(windows)] +use std::path::Path; use std::path::PathBuf; use std::time::{Duration, UNIX_EPOCH}; +use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::InvalidEncodingHandling; #[cfg(windows)] use winapi::shared::minwindef::{DWORD, LPVOID}; @@ -42,7 +46,7 @@ mod options { pub const NULL: &str = "0"; pub const ALL: &str = "all"; pub const APPARENT_SIZE: &str = "apparent-size"; - pub const BLOCK_SIZE: &str = "B"; + pub const BLOCK_SIZE: &str = "block-size"; pub const BYTES: &str = "b"; pub const TOTAL: &str = "c"; pub const MAX_DEPTH: &str = "d"; @@ -55,6 +59,7 @@ mod options { pub const SI: &str = "si"; pub const TIME: &str = "time"; pub const TIME_STYLE: &str = "time-style"; + pub const ONE_FILE_SYSTEM: &str = "one-file-system"; pub const FILE: &str = "FILE"; } @@ -79,6 +84,7 @@ struct Options { max_depth: Option, total: bool, separate_dirs: bool, + one_file_system: bool, } #[derive(PartialEq, Eq, Hash, Clone, Copy)] @@ -159,7 +165,7 @@ fn birth_u64(meta: &Metadata) -> Option { } #[cfg(windows)] -fn get_size_on_disk(path: &PathBuf) -> u64 { +fn get_size_on_disk(path: &Path) -> u64 { let mut size_on_disk = 0; // bind file so it stays in scope until end of function @@ -191,7 +197,7 @@ fn get_size_on_disk(path: &PathBuf) -> u64 { } #[cfg(windows)] -fn get_file_info(path: &PathBuf) -> Option { +fn get_file_info(path: &Path) -> Option { let mut result = None; let file = match fs::File::open(path) { @@ -223,64 +229,22 @@ fn get_file_info(path: &PathBuf) -> Option { result } -fn unit_string_to_number(s: &str) -> Option { - let mut offset = 0; - let mut s_chars = s.chars().rev(); - - let (mut ch, multiple) = match s_chars.next() { - Some('B') | Some('b') => ('B', 1000u64), - Some(ch) => (ch, 1024u64), - None => return None, - }; - if ch == 'B' { - ch = s_chars.next()?; - offset += 1; - } - ch = ch.to_ascii_uppercase(); - - let unit = UNITS - .iter() - .rev() - .find(|&&(unit_ch, _)| unit_ch == ch) - .map(|&(_, val)| { - // we found a match, so increment offset - offset += 1; - val - }) - .or_else(|| if multiple == 1024 { Some(0) } else { None })?; - - let number = s[..s.len() - offset].parse::().ok()?; - - Some(number * multiple.pow(unit)) -} - -fn translate_to_pure_number(s: &Option<&str>) -> Option { - match *s { - Some(ref s) => unit_string_to_number(s), - None => None, - } -} - -fn read_block_size(s: Option<&str>) -> u64 { - match translate_to_pure_number(&s) { - Some(v) => v, - None => { - if let Some(value) = s { - show_error!("invalid --block-size argument '{}'", value); - }; - - for env_var in &["DU_BLOCK_SIZE", "BLOCK_SIZE", "BLOCKSIZE"] { - let env_size = env::var(env_var).ok(); - if let Some(quantity) = translate_to_pure_number(&env_size.as_deref()) { - return quantity; +fn read_block_size(s: Option<&str>) -> usize { + if let Some(s) = s { + parse_size(s) + .unwrap_or_else(|e| crash!(1, "{}", format_error_message(e, s, options::BLOCK_SIZE))) + } else { + for env_var in &["DU_BLOCK_SIZE", "BLOCK_SIZE", "BLOCKSIZE"] { + if let Ok(env_size) = env::var(env_var) { + if let Ok(v) = parse_size(&env_size) { + return v; } } - - if env::var("POSIXLY_CORRECT").is_ok() { - 512 - } else { - 1024 - } + } + if env::var("POSIXLY_CORRECT").is_ok() { + 512 + } else { + 1024 } } } @@ -316,10 +280,18 @@ fn du( Ok(entry) => match Stat::new(entry.path()) { Ok(this_stat) => { if this_stat.is_dir { + if options.one_file_system { + if let (Some(this_inode), Some(my_inode)) = + (this_stat.inode, my_stat.inode) + { + if this_inode.dev_id != my_inode.dev_id { + continue; + } + } + } futures.push(du(this_stat, options, depth + 1, inodes)); } else { - if this_stat.inode.is_some() { - let inode = this_stat.inode.unwrap(); + if let Some(inode) = this_stat.inode { if inodes.contains(&inode) { continue; } @@ -358,7 +330,9 @@ fn du( my_stat.size += stat.size; my_stat.blocks += stat.blocks; } - options.max_depth == None || depth < options.max_depth.unwrap() + options + .max_depth + .map_or(true, |max_depth| depth < max_depth) })); stats.push(my_stat); Box::new(stats.into_iter()) @@ -431,12 +405,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { although the apparent size is usually smaller, it may be larger due to holes \ in ('sparse') files, internal fragmentation, indirect blocks, and the like" ) - .alias("app") // The GNU testsuite uses this alias + .alias("app") // The GNU test suite uses this alias ) .arg( Arg::with_name(options::BLOCK_SIZE) .short("B") - .long("block-size") + .long(options::BLOCK_SIZE) .value_name("SIZE") .help( "scale sizes by SIZE before printing them. \ @@ -530,12 +504,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::SI) .help("like -h, but use powers of 1000 not 1024") ) - // .arg( - // Arg::with_name("one-file-system") - // .short("x") - // .long("one-file-system") - // .help("skip directories on different file systems") - // ) + .arg( + Arg::with_name(options::ONE_FILE_SYSTEM) + .short("x") + .long(options::ONE_FILE_SYSTEM) + .help("skip directories on different file systems") + ) // .arg( // Arg::with_name("") // .short("x") @@ -583,12 +557,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let max_depth_str = matches.value_of(options::MAX_DEPTH); let max_depth = max_depth_str.as_ref().and_then(|s| s.parse::().ok()); match (max_depth_str, max_depth) { - (Some(ref s), _) if summarize => { - show_error!("summarizing conflicts with --max-depth={}", *s); + (Some(s), _) if summarize => { + show_error!("summarizing conflicts with --max-depth={}", s); return 1; } - (Some(ref s), None) => { - show_error!("invalid maximum depth '{}'", *s); + (Some(s), None) => { + show_error!("invalid maximum depth '{}'", s); return 1; } (Some(_), Some(_)) | (None, _) => { /* valid */ } @@ -600,6 +574,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { max_depth, total: matches.is_present(options::TOTAL), separate_dirs: matches.is_present(options::SEPARATE_DIRS), + one_file_system: matches.is_present(options::ONE_FILE_SYSTEM), }; let files = match matches.value_of(options::FILE) { @@ -609,7 +584,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } }; - let block_size = read_block_size(matches.value_of(options::BLOCK_SIZE)); + let block_size = u64::try_from(read_block_size(matches.value_of(options::BLOCK_SIZE))).unwrap(); let multiplier: u64 = if matches.is_present(options::SI) { 1000 @@ -745,31 +720,27 @@ Try '{} --help' for more information.", 0 } +fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String { + // NOTE: + // GNU's du echos affected flag, -B or --block-size (-t or --threshold), depending user's selection + // GNU's du does distinguish between "invalid (suffix in) argument" + match error { + ParseSizeError::ParseFailure(_) => format!("invalid --{} argument '{}'", option, s), + ParseSizeError::SizeTooBig(_) => format!("--{} argument '{}' too large", option, s), + } +} + #[cfg(test)] mod test_du { #[allow(unused_imports)] use super::*; - #[test] - fn test_translate_to_pure_number() { - let test_data = [ - (Some("10".to_string()), Some(10)), - (Some("10K".to_string()), Some(10 * 1024)), - (Some("5M".to_string()), Some(5 * 1024 * 1024)), - (Some("900KB".to_string()), Some(900 * 1000)), - (Some("BAD_STRING".to_string()), None), - ]; - for it in test_data.iter() { - assert_eq!(translate_to_pure_number(&it.0.as_deref()), it.1); - } - } - #[test] fn test_read_block_size() { let test_data = [ - (Some("10".to_string()), 10), + (Some("1024".to_string()), 1024), + (Some("K".to_string()), 1024), (None, 1024), - (Some("BAD_STRING".to_string()), 1024), ]; for it in test_data.iter() { assert_eq!(read_block_size(it.0.as_deref()), it.1); diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 56cd967f4..d83a4fe06 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -181,7 +181,7 @@ fn execute(no_newline: bool, escaped: bool, free: Vec) -> io::Result<()> write!(output, " ")?; } if escaped { - let should_stop = print_escaped(&input, &mut output)?; + let should_stop = print_escaped(input, &mut output)?; if should_stop { break; } diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index 50a327260..0ea66d7e9 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -82,13 +82,10 @@ fn load_config_file(opts: &mut Options) -> Result<(), i32> { Ini::load_from_file(file) }; - let conf = match conf { - Ok(config) => config, - Err(error) => { - eprintln!("env: error: \"{}\": {}", file, error); - return Err(1); - } - }; + let conf = conf.map_err(|error| { + eprintln!("env: error: \"{}\": {}", file, error); + 1 + })?; for (_, prop) in &conf { // ignore all INI section lines (treat them as comments) @@ -245,7 +242,7 @@ fn run_env(args: impl uucore::Args) -> Result<(), i32> { } // set specified env vars - for &(ref name, ref val) in &opts.sets { + for &(name, val) in &opts.sets { // FIXME: set_var() panics if name is an empty string env::set_var(name, val); } @@ -256,13 +253,10 @@ fn run_env(args: impl uucore::Args) -> Result<(), i32> { // FIXME: this should just use execvp() (no fork()) on Unix-like systems match Command::new(&*prog).args(args).status() { - Ok(exit) => { - if !exit.success() { - return Err(exit.code().unwrap()); - } - } + Ok(exit) if !exit.success() => return Err(exit.code().unwrap()), Err(ref err) if err.kind() == io::ErrorKind::NotFound => return Err(127), Err(_) => return Err(126), + Ok(_) => (), } } else { // no program provided, so just dump all env vars to stdout diff --git a/src/uu/expand/src/expand.rs b/src/uu/expand/src/expand.rs index 08a514dbf..d9d669e7c 100644 --- a/src/uu/expand/src/expand.rs +++ b/src/uu/expand/src/expand.rs @@ -15,7 +15,6 @@ extern crate uucore; use clap::{crate_version, App, Arg, ArgMatches}; use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; -use std::iter::repeat; use std::str::from_utf8; use unicode_width::UnicodeWidthChar; @@ -90,7 +89,7 @@ impl Options { }) .max() .unwrap(); // length of tabstops is guaranteed >= 1 - let tspaces = repeat(' ').take(nspaces).collect(); + let tspaces = " ".repeat(nspaces); let files: Vec = match matches.values_of(options::FILES) { Some(s) => s.map(|v| v.to_string()).collect(), @@ -236,7 +235,7 @@ fn expand(options: Options) { // now dump out either spaces if we're expanding, or a literal tab if we're not if init || !options.iflag { - safe_unwrap!(output.write_all(&options.tspaces[..nts].as_bytes())); + safe_unwrap!(output.write_all(options.tspaces[..nts].as_bytes())); } else { safe_unwrap!(output.write_all(&buf[byte..byte + nbytes])); } diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs index 5d63bed80..8238917f7 100644 --- a/src/uu/expr/src/expr.rs +++ b/src/uu/expr/src/expr.rs @@ -37,7 +37,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } fn process_expr(token_strings: &[String]) -> Result { - let maybe_tokens = tokens::strings_to_tokens(&token_strings); + let maybe_tokens = tokens::strings_to_tokens(token_strings); let maybe_ast = syntax_tree::tokens_to_ast(maybe_tokens); evaluate_ast(maybe_ast) } @@ -56,11 +56,7 @@ fn print_expr_error(expr_error: &str) -> ! { } fn evaluate_ast(maybe_ast: Result, String>) -> Result { - if maybe_ast.is_err() { - Err(maybe_ast.err().unwrap()) - } else { - maybe_ast.ok().unwrap().evaluate() - } + maybe_ast.and_then(|ast| ast.evaluate()) } fn maybe_handle_help_or_version(args: &[String]) -> bool { diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index b72d78729..ff49ea57e 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -160,10 +160,8 @@ impl AstNode { if let AstNode::Node { operands, .. } = self { let mut out = Vec::with_capacity(operands.len()); for operand in operands { - match operand.evaluate() { - Ok(value) => out.push(value), - Err(reason) => return Err(reason), - } + let value = operand.evaluate()?; + out.push(value); } Ok(out) } else { @@ -175,23 +173,14 @@ impl AstNode { pub fn tokens_to_ast( maybe_tokens: Result, String>, ) -> Result, String> { - if maybe_tokens.is_err() { - Err(maybe_tokens.err().unwrap()) - } else { - let tokens = maybe_tokens.ok().unwrap(); + maybe_tokens.and_then(|tokens| { let mut out_stack: TokenStack = Vec::new(); let mut op_stack: TokenStack = Vec::new(); for (token_idx, token) in tokens { - if let Err(reason) = - push_token_to_either_stack(token_idx, &token, &mut out_stack, &mut op_stack) - { - return Err(reason); - } - } - if let Err(reason) = move_rest_of_ops_to_out(&mut out_stack, &mut op_stack) { - return Err(reason); + push_token_to_either_stack(token_idx, &token, &mut out_stack, &mut op_stack)?; } + move_rest_of_ops_to_out(&mut out_stack, &mut op_stack)?; assert!(op_stack.is_empty()); maybe_dump_rpn(&out_stack); @@ -205,7 +194,7 @@ pub fn tokens_to_ast( maybe_dump_ast(&result); result } - } + }) } fn maybe_dump_ast(result: &Result, String>) { @@ -261,10 +250,8 @@ fn maybe_ast_node( ) -> Result, String> { let mut operands = Vec::with_capacity(arity); for _ in 0..arity { - match ast_from_rpn(rpn) { - Err(reason) => return Err(reason), - Ok(operand) => operands.push(operand), - } + let operand = ast_from_rpn(rpn)?; + operands.push(operand); } operands.reverse(); Ok(AstNode::new_node(token_idx, op_type, operands)) @@ -408,10 +395,12 @@ fn move_till_match_paren( op_stack: &mut TokenStack, ) -> Result<(), String> { loop { - match op_stack.pop() { - None => return Err("syntax error (Mismatched close-parenthesis)".to_string()), - Some((_, Token::ParOpen)) => return Ok(()), - Some(other) => out_stack.push(other), + let op = op_stack + .pop() + .ok_or_else(|| "syntax error (Mismatched close-parenthesis)".to_string())?; + match op { + (_, Token::ParOpen) => return Ok(()), + other => out_stack.push(other), } } } @@ -471,22 +460,17 @@ fn infix_operator_and(values: &[String]) -> String { fn operator_match(values: &[String]) -> Result { assert!(values.len() == 2); - let re = match Regex::with_options(&values[1], RegexOptions::REGEX_OPTION_NONE, Syntax::grep()) - { - Ok(m) => m, - Err(err) => return Err(err.description().to_string()), - }; - if re.captures_len() > 0 { - Ok(match re.captures(&values[0]) { - Some(captures) => captures.at(1).unwrap().to_string(), - None => "".to_string(), - }) + let re = Regex::with_options(&values[1], RegexOptions::REGEX_OPTION_NONE, Syntax::grep()) + .map_err(|err| err.description().to_string())?; + Ok(if re.captures_len() > 0 { + re.captures(&values[0]) + .map(|captures| captures.at(1).unwrap()) + .unwrap_or("") + .to_string() } else { - Ok(match re.find(&values[0]) { - Some((start, end)) => (end - start).to_string(), - None => "0".to_string(), - }) - } + re.find(&values[0]) + .map_or("0".to_string(), |(start, end)| (end - start).to_string()) + }) } fn prefix_operator_length(values: &[String]) -> String { diff --git a/src/uu/expr/src/tokens.rs b/src/uu/expr/src/tokens.rs index 6f2795588..748960bc3 100644 --- a/src/uu/expr/src/tokens.rs +++ b/src/uu/expr/src/tokens.rs @@ -78,27 +78,27 @@ pub fn strings_to_tokens(strings: &[String]) -> Result, Stri "(" => Token::ParOpen, ")" => Token::ParClose, - "^" => Token::new_infix_op(&s, false, 7), + "^" => Token::new_infix_op(s, false, 7), - ":" => Token::new_infix_op(&s, true, 6), + ":" => Token::new_infix_op(s, true, 6), - "*" => Token::new_infix_op(&s, true, 5), - "/" => Token::new_infix_op(&s, true, 5), - "%" => Token::new_infix_op(&s, true, 5), + "*" => Token::new_infix_op(s, true, 5), + "/" => Token::new_infix_op(s, true, 5), + "%" => Token::new_infix_op(s, true, 5), - "+" => Token::new_infix_op(&s, true, 4), - "-" => Token::new_infix_op(&s, true, 4), + "+" => Token::new_infix_op(s, true, 4), + "-" => Token::new_infix_op(s, true, 4), - "=" => Token::new_infix_op(&s, true, 3), - "!=" => Token::new_infix_op(&s, true, 3), - "<" => Token::new_infix_op(&s, true, 3), - ">" => Token::new_infix_op(&s, true, 3), - "<=" => Token::new_infix_op(&s, true, 3), - ">=" => Token::new_infix_op(&s, true, 3), + "=" => Token::new_infix_op(s, true, 3), + "!=" => Token::new_infix_op(s, true, 3), + "<" => Token::new_infix_op(s, true, 3), + ">" => Token::new_infix_op(s, true, 3), + "<=" => Token::new_infix_op(s, true, 3), + ">=" => Token::new_infix_op(s, true, 3), - "&" => Token::new_infix_op(&s, true, 2), + "&" => Token::new_infix_op(s, true, 2), - "|" => Token::new_infix_op(&s, true, 1), + "|" => Token::new_infix_op(s, true, 1), "match" => Token::PrefixOp { arity: 2, @@ -117,9 +117,9 @@ pub fn strings_to_tokens(strings: &[String]) -> Result, Stri value: s.clone(), }, - _ => Token::new_value(&s), + _ => Token::new_value(s), }; - push_token_if_not_escaped(&mut tokens_acc, tok_idx, token_if_not_escaped, &s); + push_token_if_not_escaped(&mut tokens_acc, tok_idx, token_if_not_escaped, s); tok_idx += 1; } maybe_dump_tokens_acc(&tokens_acc); diff --git a/src/uu/fold/src/fold.rs b/src/uu/fold/src/fold.rs index e476fed5b..118f7f5f9 100644 --- a/src/uu/fold/src/fold.rs +++ b/src/uu/fold/src/fold.rs @@ -98,7 +98,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { fn handle_obsolete(args: &[String]) -> (Vec, Option) { for (i, arg) in args.iter().enumerate() { let slice = &arg; - if slice.starts_with('-') && slice.len() > 1 && slice.chars().nth(1).unwrap().is_digit(10) { + if slice.starts_with('-') && slice.chars().nth(1).map_or(false, |c| c.is_digit(10)) { let mut v = args.to_vec(); v.remove(i); return (v, Some(slice[1..].to_owned())); @@ -109,7 +109,7 @@ fn handle_obsolete(args: &[String]) -> (Vec, Option) { fn fold(filenames: Vec, bytes: bool, spaces: bool, width: usize) { for filename in &filenames { - let filename: &str = &filename; + let filename: &str = filename; let mut stdin_buf; let mut file_buf; let buffer = BufReader::new(if filename == "-" { diff --git a/src/uu/groups/Cargo.toml b/src/uu/groups/Cargo.toml index 1a56bc2ab..4a5a537e5 100644 --- a/src/uu/groups/Cargo.toml +++ b/src/uu/groups/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" path = "src/groups.rs" [dependencies] -uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries"] } +uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "process"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } clap = "2.33" diff --git a/src/uu/hashsum/src/digest.rs b/src/uu/hashsum/src/digest.rs index 25bc7f4c3..9093d94a7 100644 --- a/src/uu/hashsum/src/digest.rs +++ b/src/uu/hashsum/src/digest.rs @@ -59,7 +59,7 @@ impl Digest for blake2b_simd::State { fn result(&mut self, out: &mut [u8]) { let hash_result = &self.finalize(); - out.copy_from_slice(&hash_result.as_bytes()); + out.copy_from_slice(hash_result.as_bytes()); } fn reset(&mut self) { diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs index 9822ca3fa..a007473ab 100644 --- a/src/uu/hashsum/src/hashsum.rs +++ b/src/uu/hashsum/src/hashsum.rs @@ -90,7 +90,7 @@ fn detect_algo<'a>( 512, ), "sha3sum" => match matches.value_of("bits") { - Some(bits_str) => match (&bits_str).parse::() { + Some(bits_str) => match (bits_str).parse::() { Ok(224) => ( "SHA3-224", Box::new(Sha3_224::new()) as Box, @@ -140,7 +140,7 @@ fn detect_algo<'a>( 512, ), "shake128sum" => match matches.value_of("bits") { - Some(bits_str) => match (&bits_str).parse::() { + Some(bits_str) => match (bits_str).parse::() { Ok(bits) => ( "SHAKE128", Box::new(Shake128::new()) as Box, @@ -151,7 +151,7 @@ fn detect_algo<'a>( None => crash!(1, "--bits required for SHAKE-128"), }, "shake256sum" => match matches.value_of("bits") { - Some(bits_str) => match (&bits_str).parse::() { + Some(bits_str) => match (bits_str).parse::() { Ok(bits) => ( "SHAKE256", Box::new(Shake256::new()) as Box, @@ -194,7 +194,7 @@ fn detect_algo<'a>( } if matches.is_present("sha3") { match matches.value_of("bits") { - Some(bits_str) => match (&bits_str).parse::() { + Some(bits_str) => match (bits_str).parse::() { Ok(224) => set_or_crash( "SHA3-224", Box::new(Sha3_224::new()) as Box, @@ -238,7 +238,7 @@ fn detect_algo<'a>( } if matches.is_present("shake128") { match matches.value_of("bits") { - Some(bits_str) => match (&bits_str).parse::() { + Some(bits_str) => match (bits_str).parse::() { Ok(bits) => set_or_crash("SHAKE128", Box::new(Shake128::new()), bits), Err(err) => crash!(1, "{}", err), }, @@ -247,7 +247,7 @@ fn detect_algo<'a>( } if matches.is_present("shake256") { match matches.value_of("bits") { - Some(bits_str) => match (&bits_str).parse::() { + Some(bits_str) => match (bits_str).parse::() { Ok(bits) => set_or_crash("SHAKE256", Box::new(Shake256::new()), bits), Err(err) => crash!(1, "{}", err), }, @@ -255,10 +255,8 @@ fn detect_algo<'a>( } } } - if alg.is_none() { - crash!(1, "You must specify hash algorithm!") - }; - (name, alg.unwrap(), output_bits) + let alg = alg.unwrap_or_else(|| crash!(1, "You must specify hash algorithm!")); + (name, alg, output_bits) } } } diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 28710e1fe..aceecd941 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -1,3 +1,8 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + // spell-checker:ignore (vars) zlines use clap::{crate_version, App, Arg}; @@ -75,7 +80,7 @@ fn app<'a>() -> App<'a, 'a> { .arg( Arg::with_name(options::QUIET_NAME) .short("q") - .long("--quiet") + .long("quiet") .visible_alias("silent") .help("never print headers giving file names") .overrides_with_all(&[options::VERBOSE_NAME, options::QUIET_NAME]), @@ -108,12 +113,7 @@ where { match parse::parse_num(src) { Ok((n, last)) => Ok((closure(n), last)), - Err(reason) => match reason { - parse::ParseError::Syntax => Err(format!("'{}'", src)), - parse::ParseError::Overflow => { - Err(format!("'{}': Value too large for defined datatype", src)) - } - }, + Err(e) => Err(e.to_string()), } } @@ -176,19 +176,11 @@ impl HeadOptions { options.zeroed = matches.is_present(options::ZERO_NAME); let mode_and_from_end = if let Some(v) = matches.value_of(options::BYTES_NAME) { - match parse_mode(v, Modes::Bytes) { - Ok(v) => v, - Err(err) => { - return Err(format!("invalid number of bytes: {}", err)); - } - } + parse_mode(v, Modes::Bytes) + .map_err(|err| format!("invalid number of bytes: {}", err))? } else if let Some(v) = matches.value_of(options::LINES_NAME) { - match parse_mode(v, Modes::Lines) { - Ok(v) => v, - Err(err) => { - return Err(format!("invalid number of lines: {}", err)); - } - } + parse_mode(v, Modes::Lines) + .map_err(|err| format!("invalid number of lines: {}", err))? } else { (Modes::Lines(10), false) }; @@ -474,7 +466,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let args = match HeadOptions::get_from(args) { Ok(o) => o, Err(s) => { - crash!(EXIT_FAILURE, "head: {}", s); + crash!(EXIT_FAILURE, "{}", s); } }; match uu_head(&args) { diff --git a/src/uu/head/src/parse.rs b/src/uu/head/src/parse.rs index f1c97561d..f6f291814 100644 --- a/src/uu/head/src/parse.rs +++ b/src/uu/head/src/parse.rs @@ -1,5 +1,10 @@ -use std::convert::TryFrom; +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + use std::ffi::OsString; +use uucore::parse_size::{parse_size, ParseSizeError}; #[derive(PartialEq, Debug)] pub enum ParseError { @@ -92,92 +97,25 @@ pub fn parse_obsolete(src: &str) -> Option } /// Parses an -c or -n argument, /// the bool specifies whether to read from the end -pub fn parse_num(src: &str) -> Result<(usize, bool), ParseError> { - let mut num_start = 0; - let mut chars = src.char_indices(); - let (mut chars, all_but_last) = match chars.next() { - Some((_, c)) => { +pub fn parse_num(src: &str) -> Result<(usize, bool), ParseSizeError> { + let mut size_string = src.trim(); + let mut all_but_last = false; + + if let Some(c) = size_string.chars().next() { + if c == '+' || c == '-' { + // head: '+' is not documented (8.32 man pages) + size_string = &size_string[1..]; if c == '-' { - num_start += 1; - (chars, true) - } else { - (src.char_indices(), false) + all_but_last = true; } } - None => return Err(ParseError::Syntax), - }; - let mut num_end = 0usize; - let mut last_char = 0 as char; - let mut num_count = 0usize; - for (n, c) in &mut chars { - if c.is_numeric() { - num_end = n; - num_count += 1; - } else { - last_char = c; - break; - } + } else { + return Err(ParseSizeError::ParseFailure(src.to_string())); } - let num = if num_count > 0 { - match src[num_start..=num_end].parse::() { - Ok(n) => Some(n), - Err(_) => return Err(ParseError::Overflow), - } - } else { - None - }; - - if last_char == 0 as char { - if let Some(n) = num { - Ok((n, all_but_last)) - } else { - Err(ParseError::Syntax) - } - } else { - let base: u128 = match chars.next() { - Some((_, c)) => { - let b = match c { - 'B' if last_char != 'b' => 1000, - 'i' if last_char != 'b' => { - if let Some((_, 'B')) = chars.next() { - 1024 - } else { - return Err(ParseError::Syntax); - } - } - _ => return Err(ParseError::Syntax), - }; - if chars.next().is_some() { - return Err(ParseError::Syntax); - } else { - b - } - } - None => 1024, - }; - let mul = match last_char.to_lowercase().next().unwrap() { - 'b' => 512, - 'k' => base.pow(1), - 'm' => base.pow(2), - 'g' => base.pow(3), - 't' => base.pow(4), - 'p' => base.pow(5), - 'e' => base.pow(6), - 'z' => base.pow(7), - 'y' => base.pow(8), - _ => return Err(ParseError::Syntax), - }; - let mul = match usize::try_from(mul) { - Ok(n) => n, - Err(_) => return Err(ParseError::Overflow), - }; - match num.unwrap_or(1).checked_mul(mul) { - Some(n) => Ok((n, all_but_last)), - None => Err(ParseError::Overflow), - } - } + parse_size(size_string).map(|n| (n, all_but_last)) } + #[cfg(test)] mod tests { use super::*; @@ -195,44 +133,6 @@ mod tests { Some(Ok(src.iter().map(|s| s.to_string()).collect())) } #[test] - #[cfg(not(target_pointer_width = "128"))] - fn test_parse_overflow_x64() { - assert_eq!(parse_num("1Y"), Err(ParseError::Overflow)); - assert_eq!(parse_num("1Z"), Err(ParseError::Overflow)); - assert_eq!(parse_num("100E"), Err(ParseError::Overflow)); - assert_eq!(parse_num("100000P"), Err(ParseError::Overflow)); - assert_eq!(parse_num("1000000000T"), Err(ParseError::Overflow)); - assert_eq!( - parse_num("10000000000000000000000"), - Err(ParseError::Overflow) - ); - } - #[test] - #[cfg(target_pointer_width = "32")] - fn test_parse_overflow_x32() { - assert_eq!(parse_num("1T"), Err(ParseError::Overflow)); - assert_eq!(parse_num("1000G"), Err(ParseError::Overflow)); - } - #[test] - fn test_parse_bad_syntax() { - assert_eq!(parse_num("5MiB nonsense"), Err(ParseError::Syntax)); - assert_eq!(parse_num("Nonsense string"), Err(ParseError::Syntax)); - assert_eq!(parse_num("5mib"), Err(ParseError::Syntax)); - assert_eq!(parse_num("biB"), Err(ParseError::Syntax)); - assert_eq!(parse_num("-"), Err(ParseError::Syntax)); - assert_eq!(parse_num(""), Err(ParseError::Syntax)); - } - #[test] - fn test_parse_numbers() { - assert_eq!(parse_num("k"), Ok((1024, false))); - assert_eq!(parse_num("MiB"), Ok((1024 * 1024, false))); - assert_eq!(parse_num("-5"), Ok((5, true))); - assert_eq!(parse_num("b"), Ok((512, false))); - assert_eq!(parse_num("-2GiB"), Ok((2 * 1024 * 1024 * 1024, true))); - assert_eq!(parse_num("5M"), Ok((5 * 1024 * 1024, false))); - assert_eq!(parse_num("5MB"), Ok((5 * 1000 * 1000, false))); - } - #[test] fn test_parse_numbers_obsolete() { assert_eq!(obsolete("-5"), obsolete_result(&["-n", "5"])); assert_eq!(obsolete("-100"), obsolete_result(&["-n", "100"])); diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 7aa6f95ff..ad5ea694c 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -299,29 +299,17 @@ fn behavior(matches: &ArgMatches) -> Result { let considering_dir: bool = MainFunction::Directory == main_function; let specified_mode: Option = if matches.is_present(OPT_MODE) { - match matches.value_of(OPT_MODE) { - Some(x) => match mode::parse(x, considering_dir) { - Ok(y) => Some(y), - Err(err) => { - show_error!("Invalid mode string: {}", err); - return Err(1); - } - }, - None => { - return Err(1); - } - } + let x = matches.value_of(OPT_MODE).ok_or(1)?; + Some(mode::parse(x, considering_dir).map_err(|err| { + show_error!("Invalid mode string: {}", err); + 1 + })?) } else { None }; let backup_suffix = if matches.is_present(OPT_SUFFIX) { - match matches.value_of(OPT_SUFFIX) { - Some(x) => x, - None => { - return Err(1); - } - } + matches.value_of(OPT_SUFFIX).ok_or(1)? } else { "~" }; @@ -379,7 +367,7 @@ fn directory(paths: Vec, b: Behavior) -> i32 { } } - if mode::chmod(&path, b.mode()).is_err() { + if mode::chmod(path, b.mode()).is_err() { all_successful = false; continue; } @@ -422,7 +410,7 @@ fn standard(paths: Vec, b: Behavior) -> i32 { return 1; } - if mode::chmod(&parent, b.mode()).is_err() { + if mode::chmod(parent, b.mode()).is_err() { show_error!("failed to chmod {}", parent.display()); return 1; } @@ -501,7 +489,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3 /// _target_ must be a non-directory /// fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 { - if copy(file, &target, b).is_err() { + if copy(file, target, b).is_err() { 1 } else { 0 @@ -563,7 +551,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { } } - if mode::chmod(&to, b.mode()).is_err() { + if mode::chmod(to, b.mode()).is_err() { return Err(()); } diff --git a/src/uu/join/src/join.rs b/src/uu/join/src/join.rs index 7a044789f..4cdfe2141 100644 --- a/src/uu/join/src/join.rs +++ b/src/uu/join/src/join.rs @@ -328,8 +328,8 @@ impl<'a> State<'a> { }); } else { repr.print_field(key); - repr.print_fields(&line1, self.key, self.max_fields); - repr.print_fields(&line2, other.key, other.max_fields); + repr.print_fields(line1, self.key, self.max_fields); + repr.print_fields(line2, other.key, other.max_fields); } println!(); @@ -611,7 +611,7 @@ fn exec(file1: &str, file2: &str, settings: &Settings) -> i32 { let mut state1 = State::new( FileNum::File1, - &file1, + file1, &stdin, settings.key1, settings.print_unpaired, @@ -619,7 +619,7 @@ fn exec(file1: &str, file2: &str, settings: &Settings) -> i32 { let mut state2 = State::new( FileNum::File2, - &file2, + file2, &stdin, settings.key2, settings.print_unpaired, diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index 6c2464c92..a49acaa05 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -111,7 +111,7 @@ fn handle_obsolete(mut args: Vec) -> (Vec, Option) { while i < args.len() { // this is safe because slice is valid when it is referenced let slice = &args[i].clone(); - if slice.starts_with('-') && slice.len() > 1 && slice.chars().nth(1).unwrap().is_digit(10) { + if slice.starts_with('-') && slice.chars().nth(1).map_or(false, |c| c.is_digit(10)) { let val = &slice[1..]; match val.parse() { Ok(num) => { diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index cd5eef842..ce1dd15b0 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -27,7 +27,6 @@ use uucore::fs::{canonicalize, CanonicalizeMode}; pub struct Settings { overwrite: OverwriteMode, backup: BackupMode, - force: bool, suffix: String, symbolic: bool, relative: bool, @@ -54,7 +53,7 @@ pub enum BackupMode { fn get_usage() -> String { format!( - "{0} [OPTION]... [-T] TARGET LINK_executable!() (1st form) + "{0} [OPTION]... [-T] TARGET LINK_NAME (1st form) {0} [OPTION]... TARGET (2nd form) {0} [OPTION]... TARGET... DIRECTORY (3rd form) {0} [OPTION]... -t DIRECTORY TARGET... (4th form)", @@ -64,7 +63,7 @@ fn get_usage() -> String { fn get_long_usage() -> String { String::from( - " In the 1st form, create a link to TARGET with the name LINK_executable!(). + " In the 1st form, create a link to TARGET with the name LINK_NAME. In the 2nd form, create a link to TARGET in the current directory. In the 3rd and 4th forms, create links to each TARGET in DIRECTORY. Create hard links by default, symbolic links with --symbolic. @@ -78,17 +77,19 @@ fn get_long_usage() -> String { static ABOUT: &str = "change file owner and group"; -static OPT_B: &str = "b"; -static OPT_BACKUP: &str = "backup"; -static OPT_FORCE: &str = "force"; -static OPT_INTERACTIVE: &str = "interactive"; -static OPT_NO_DEREFERENCE: &str = "no-dereference"; -static OPT_SYMBOLIC: &str = "symbolic"; -static OPT_SUFFIX: &str = "suffix"; -static OPT_TARGET_DIRECTORY: &str = "target-directory"; -static OPT_NO_TARGET_DIRECTORY: &str = "no-target-directory"; -static OPT_RELATIVE: &str = "relative"; -static OPT_VERBOSE: &str = "verbose"; +mod options { + pub const B: &str = "b"; + pub const BACKUP: &str = "backup"; + pub const FORCE: &str = "force"; + pub const INTERACTIVE: &str = "interactive"; + pub const NO_DEREFERENCE: &str = "no-dereference"; + pub const SYMBOLIC: &str = "symbolic"; + pub const SUFFIX: &str = "suffix"; + pub const TARGET_DIRECTORY: &str = "target-directory"; + pub const NO_TARGET_DIRECTORY: &str = "no-target-directory"; + pub const RELATIVE: &str = "relative"; + pub const VERBOSE: &str = "verbose"; +} static ARG_FILES: &str = "files"; @@ -101,49 +102,44 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .about(ABOUT) .usage(&usage[..]) .after_help(&long_usage[..]) - .arg(Arg::with_name(OPT_B).short(OPT_B).help( + .arg(Arg::with_name(options::B).short(options::B).help( "make a backup of each file that would otherwise be overwritten or \ removed", )) .arg( - Arg::with_name(OPT_BACKUP) - .long(OPT_BACKUP) + Arg::with_name(options::BACKUP) + .long(options::BACKUP) .help( "make a backup of each file that would otherwise be overwritten \ or removed", ) .takes_value(true) - .possible_value("simple") - .possible_value("never") - .possible_value("numbered") - .possible_value("t") - .possible_value("existing") - .possible_value("nil") - .possible_value("none") - .possible_value("off") + .possible_values(&[ + "simple", "never", "numbered", "t", "existing", "nil", "none", "off", + ]) .value_name("METHOD"), ) // TODO: opts.arg( // Arg::with_name(("d", "directory", "allow users with appropriate privileges to attempt \ // to make hard links to directories"); .arg( - Arg::with_name(OPT_FORCE) + Arg::with_name(options::FORCE) .short("f") - .long(OPT_FORCE) + .long(options::FORCE) .help("remove existing destination files"), ) .arg( - Arg::with_name(OPT_INTERACTIVE) + Arg::with_name(options::INTERACTIVE) .short("i") - .long(OPT_INTERACTIVE) + .long(options::INTERACTIVE) .help("prompt whether to remove existing destination files"), ) .arg( - Arg::with_name(OPT_NO_DEREFERENCE) + Arg::with_name(options::NO_DEREFERENCE) .short("n") - .long(OPT_NO_DEREFERENCE) + .long(options::NO_DEREFERENCE) .help( - "treat LINK_executable!() as a normal file if it is a \ + "treat LINK_NAME as a normal file if it is a \ symbolic link to a directory", ), ) @@ -153,43 +149,46 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // TODO: opts.arg( // Arg::with_name(("P", "physical", "make hard links directly to symbolic links"); .arg( - Arg::with_name(OPT_SYMBOLIC) + Arg::with_name(options::SYMBOLIC) .short("s") .long("symbolic") - .help("make symbolic links instead of hard links"), + .help("make symbolic links instead of hard links") + // override added for https://github.com/uutils/coreutils/issues/2359 + .overrides_with(options::SYMBOLIC), ) .arg( - Arg::with_name(OPT_SUFFIX) + Arg::with_name(options::SUFFIX) .short("S") - .long(OPT_SUFFIX) + .long(options::SUFFIX) .help("override the usual backup suffix") .value_name("SUFFIX") .takes_value(true), ) .arg( - Arg::with_name(OPT_TARGET_DIRECTORY) + Arg::with_name(options::TARGET_DIRECTORY) .short("t") - .long(OPT_TARGET_DIRECTORY) + .long(options::TARGET_DIRECTORY) .help("specify the DIRECTORY in which to create the links") .value_name("DIRECTORY") - .conflicts_with(OPT_NO_TARGET_DIRECTORY), + .conflicts_with(options::NO_TARGET_DIRECTORY), ) .arg( - Arg::with_name(OPT_NO_TARGET_DIRECTORY) + Arg::with_name(options::NO_TARGET_DIRECTORY) .short("T") - .long(OPT_NO_TARGET_DIRECTORY) - .help("treat LINK_executable!() as a normal file always"), + .long(options::NO_TARGET_DIRECTORY) + .help("treat LINK_NAME as a normal file always"), ) .arg( - Arg::with_name(OPT_RELATIVE) + Arg::with_name(options::RELATIVE) .short("r") - .long(OPT_RELATIVE) - .help("create symbolic links relative to link location"), + .long(options::RELATIVE) + .help("create symbolic links relative to link location") + .requires(options::SYMBOLIC), ) .arg( - Arg::with_name(OPT_VERBOSE) + Arg::with_name(options::VERBOSE) .short("v") - .long(OPT_VERBOSE) + .long(options::VERBOSE) .help("print name of each linked file"), ) .arg( @@ -209,18 +208,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .map(PathBuf::from) .collect(); - let overwrite_mode = if matches.is_present(OPT_FORCE) { + let overwrite_mode = if matches.is_present(options::FORCE) { OverwriteMode::Force - } else if matches.is_present(OPT_INTERACTIVE) { + } else if matches.is_present(options::INTERACTIVE) { OverwriteMode::Interactive } else { OverwriteMode::NoClobber }; - let backup_mode = if matches.is_present(OPT_B) { + let backup_mode = if matches.is_present(options::B) { BackupMode::ExistingBackup - } else if matches.is_present(OPT_BACKUP) { - match matches.value_of(OPT_BACKUP) { + } else if matches.is_present(options::BACKUP) { + match matches.value_of(options::BACKUP) { None => BackupMode::ExistingBackup, Some(mode) => match mode { "simple" | "never" => BackupMode::SimpleBackup, @@ -234,8 +233,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { BackupMode::NoBackup }; - let backup_suffix = if matches.is_present(OPT_SUFFIX) { - matches.value_of(OPT_SUFFIX).unwrap() + let backup_suffix = if matches.is_present(options::SUFFIX) { + matches.value_of(options::SUFFIX).unwrap() } else { "~" }; @@ -243,14 +242,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let settings = Settings { overwrite: overwrite_mode, backup: backup_mode, - force: matches.is_present(OPT_FORCE), suffix: backup_suffix.to_string(), - symbolic: matches.is_present(OPT_SYMBOLIC), - relative: matches.is_present(OPT_RELATIVE), - target_dir: matches.value_of(OPT_TARGET_DIRECTORY).map(String::from), - no_target_dir: matches.is_present(OPT_NO_TARGET_DIRECTORY), - no_dereference: matches.is_present(OPT_NO_DEREFERENCE), - verbose: matches.is_present(OPT_VERBOSE), + symbolic: matches.is_present(options::SYMBOLIC), + relative: matches.is_present(options::RELATIVE), + target_dir: matches + .value_of(options::TARGET_DIRECTORY) + .map(String::from), + no_target_dir: matches.is_present(options::NO_TARGET_DIRECTORY), + no_dereference: matches.is_present(options::NO_DEREFERENCE), + verbose: matches.is_present(options::VERBOSE), }; exec(&paths[..], &settings) @@ -260,17 +260,17 @@ fn exec(files: &[PathBuf], settings: &Settings) -> i32 { // Handle cases where we create links in a directory first. if let Some(ref name) = settings.target_dir { // 4th form: a directory is specified by -t. - return link_files_in_dir(files, &PathBuf::from(name), &settings); + return link_files_in_dir(files, &PathBuf::from(name), settings); } if !settings.no_target_dir { if files.len() == 1 { // 2nd form: the target directory is the current directory. - return link_files_in_dir(files, &PathBuf::from("."), &settings); + return link_files_in_dir(files, &PathBuf::from("."), settings); } let last_file = &PathBuf::from(files.last().unwrap()); if files.len() > 2 || last_file.is_dir() { // 3rd form: create links in the last argument. - return link_files_in_dir(&files[0..files.len() - 1], last_file, &settings); + return link_files_in_dir(&files[0..files.len() - 1], last_file, settings); } } @@ -310,47 +310,48 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) let mut all_successful = true; for srcpath in files.iter() { - let targetpath = if settings.no_dereference && settings.force { - // In that case, we don't want to do link resolution - // We need to clean the target - if is_symlink(target_dir) { - if target_dir.is_file() { - if let Err(e) = fs::remove_file(target_dir) { - show_error!("Could not update {}: {}", target_dir.display(), e) - }; - } - if target_dir.is_dir() { - // Not sure why but on Windows, the symlink can be - // considered as a dir - // See test_ln::test_symlink_no_deref_dir - if let Err(e) = fs::remove_dir(target_dir) { - show_error!("Could not update {}: {}", target_dir.display(), e) - }; - } - } - target_dir.to_path_buf() - } else { - match srcpath.as_os_str().to_str() { - Some(name) => { - match Path::new(name).file_name() { - Some(basename) => target_dir.join(basename), - // This can be None only for "." or "..". Trying - // to create a link with such name will fail with - // EEXIST, which agrees with the behavior of GNU - // coreutils. - None => target_dir.join(name), + let targetpath = + if settings.no_dereference && matches!(settings.overwrite, OverwriteMode::Force) { + // In that case, we don't want to do link resolution + // We need to clean the target + if is_symlink(target_dir) { + if target_dir.is_file() { + if let Err(e) = fs::remove_file(target_dir) { + show_error!("Could not update {}: {}", target_dir.display(), e) + }; + } + if target_dir.is_dir() { + // Not sure why but on Windows, the symlink can be + // considered as a dir + // See test_ln::test_symlink_no_deref_dir + if let Err(e) = fs::remove_dir(target_dir) { + show_error!("Could not update {}: {}", target_dir.display(), e) + }; } } - None => { - show_error!( - "cannot stat '{}': No such file or directory", - srcpath.display() - ); - all_successful = false; - continue; + target_dir.to_path_buf() + } else { + match srcpath.as_os_str().to_str() { + Some(name) => { + match Path::new(name).file_name() { + Some(basename) => target_dir.join(basename), + // This can be None only for "." or "..". Trying + // to create a link with such name will fail with + // EEXIST, which agrees with the behavior of GNU + // coreutils. + None => target_dir.join(name), + } + } + None => { + show_error!( + "cannot stat '{}': No such file or directory", + srcpath.display() + ); + all_successful = false; + continue; + } } - } - }; + }; if let Err(e) = link(srcpath, &targetpath, settings) { show_error!( @@ -371,7 +372,8 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) fn relative_path<'a>(src: &Path, dst: &Path) -> Result> { let src_abs = canonicalize(src, CanonicalizeMode::Normal)?; - let dst_abs = canonicalize(dst, CanonicalizeMode::Normal)?; + let mut dst_abs = canonicalize(dst.parent().unwrap(), CanonicalizeMode::Normal)?; + dst_abs.push(dst.components().last().unwrap()); let suffix_pos = src_abs .components() .zip(dst_abs.components()) @@ -392,7 +394,7 @@ fn relative_path<'a>(src: &Path, dst: &Path) -> Result> { fn link(src: &Path, dst: &Path, settings: &Settings) -> Result<()> { let mut backup_path = None; let source: Cow<'_, Path> = if settings.relative { - relative_path(&src, dst)? + relative_path(src, dst)? } else { src.into() }; @@ -421,10 +423,6 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> Result<()> { } } - if settings.no_dereference && settings.force && dst.exists() { - fs::remove_file(dst)?; - } - if settings.symbolic { symlink(&source, dst)?; } else { diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 3c7b22360..dc67d5738 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1243,7 +1243,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { Sort::Time => entries.sort_by_key(|k| { Reverse( k.md() - .and_then(|md| get_system_time(&md, config)) + .and_then(|md| get_system_time(md, config)) .unwrap_or(UNIX_EPOCH), ) }), @@ -1323,7 +1323,7 @@ fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) .filter(|p| p.file_type().map(|ft| ft.is_dir()).unwrap_or(false)) { let _ = writeln!(out, "\n{}:", e.p_buf.display()); - enter_directory(&e, config, out); + enter_directory(e, config, out); } } } @@ -1339,8 +1339,8 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) { if let Some(md) = entry.md() { ( - display_symlink_count(&md).len(), - display_size_or_rdev(&md, config).len(), + display_symlink_count(md).len(), + display_size_or_rdev(md, config).len(), ) } else { (0, 0) @@ -1371,7 +1371,7 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter { @@ -1482,40 +1482,40 @@ fn display_item_long( #[cfg(unix)] { if config.inode { - let _ = write!(out, "{} ", get_inode(&md)); + let _ = write!(out, "{} ", get_inode(md)); } } let _ = write!( out, "{} {}", - display_permissions(&md, true), - pad_left(display_symlink_count(&md), max_links), + display_permissions(md, true), + pad_left(display_symlink_count(md), max_links), ); if config.long.owner { - let _ = write!(out, " {}", display_uname(&md, config)); + let _ = write!(out, " {}", display_uname(md, config)); } if config.long.group { - let _ = write!(out, " {}", display_group(&md, config)); + let _ = write!(out, " {}", display_group(md, config)); } // Author is only different from owner on GNU/Hurd, so we reuse // the owner, since GNU/Hurd is not currently supported by Rust. if config.long.author { - let _ = write!(out, " {}", display_uname(&md, config)); + let _ = write!(out, " {}", display_uname(md, config)); } let _ = writeln!( out, " {} {} {}", pad_left(display_size_or_rdev(md, config), max_size), - display_date(&md, config), + display_date(md, config), // unwrap is fine because it fails when metadata is not available // but we already know that it is because it's checked at the // start of the function. - display_file_name(&item, config).unwrap().contents, + display_file_name(item, config).unwrap().contents, ); } @@ -1741,7 +1741,7 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { let mut width = name.width(); if let Some(ls_colors) = &config.color { - name = color_name(&ls_colors, &path.p_buf, name, path.md()?); + name = color_name(ls_colors, &path.p_buf, name, path.md()?); } if config.indicator_style != IndicatorStyle::None { @@ -1786,7 +1786,7 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { } fn color_name(ls_colors: &LsColors, path: &Path, name: String, md: &Metadata) -> String { - match ls_colors.style_for_path_with_metadata(path, Some(&md)) { + match ls_colors.style_for_path_with_metadata(path, Some(md)) { Some(style) => style.to_ansi_term_style().paint(name).to_string(), None => name, } diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index d1461c0c9..e8a8ef2db 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -77,7 +77,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let mode_match = matches.value_of(OPT_MODE); let mode: u16 = match mode_match { Some(m) => { - let res: Option = u16::from_str_radix(&m, 8).ok(); + let res: Option = u16::from_str_radix(m, 8).ok(); match res { Some(r) => r, _ => crash!(1, "no mode given"), diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs index cf2fefa50..b8a6bbe38 100644 --- a/src/uu/mkfifo/src/mkfifo.rs +++ b/src/uu/mkfifo/src/mkfifo.rs @@ -59,7 +59,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } let mode = match matches.value_of(options::MODE) { - Some(m) => match usize::from_str_radix(&m, 8) { + Some(m) => match usize::from_str_radix(m, 8) { Ok(m) => m, Err(e) => { show_error!("invalid mode: {}", e); diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 67a88273d..e04de8702 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -165,9 +165,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; if dry_run { - dry_exec(tmpdir, prefix, rand, &suffix) + dry_exec(tmpdir, prefix, rand, suffix) } else { - exec(tmpdir, prefix, rand, &suffix, make_dir, suppress_file_err) + exec(tmpdir, prefix, rand, suffix, make_dir, suppress_file_err) } } diff --git a/src/uu/more/Cargo.toml b/src/uu/more/Cargo.toml index b3b97e6dd..af6781876 100644 --- a/src/uu/more/Cargo.toml +++ b/src/uu/more/Cargo.toml @@ -19,7 +19,7 @@ clap = "2.33" uucore = { version = ">=0.0.7", package = "uucore", path = "../../uucore" } uucore_procs = { version = ">=0.0.5", package = "uucore_procs", path = "../../uucore_procs" } crossterm = ">=0.19" -atty = "0.2.14" +atty = "0.2" unicode-width = "0.1.7" unicode-segmentation = "1.7.1" diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index 4d345e96b..206cebbc2 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -11,7 +11,6 @@ extern crate uucore; use std::{ - convert::TryInto, fs::File, io::{stdin, stdout, BufReader, Read, Stdout, Write}, path::Path, @@ -32,6 +31,8 @@ use crossterm::{ use unicode_segmentation::UnicodeSegmentation; use unicode_width::UnicodeWidthStr; +const BELL: &str = "\x07"; + pub mod options { pub const SILENT: &str = "silent"; pub const LOGICAL: &str = "logical"; @@ -53,14 +54,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let matches = App::new(executable!()) .about("A file perusal filter for CRT viewing.") .version(crate_version!()) - // The commented arguments below are unimplemented: - /* .arg( Arg::with_name(options::SILENT) .short("d") .long(options::SILENT) .help("Display help instead of ringing bell"), ) + // The commented arguments below are unimplemented: + /* .arg( Arg::with_name(options::LOGICAL) .short("f") @@ -140,6 +141,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .get_matches_from(args); let mut buff = String::new(); + let silent = matches.is_present(options::SILENT); if let Some(files) = matches.values_of(options::FILES) { let mut stdout = setup_term(); let length = files.len(); @@ -162,14 +164,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } let mut reader = BufReader::new(File::open(file).unwrap()); reader.read_to_string(&mut buff).unwrap(); - more(&buff, &mut stdout, next_file.copied()); + more(&buff, &mut stdout, next_file.copied(), silent); buff.clear(); } reset_term(&mut stdout); } else if atty::isnt(atty::Stream::Stdin) { stdin().read_to_string(&mut buff).unwrap(); let mut stdout = setup_term(); - more(&buff, &mut stdout, None); + more(&buff, &mut stdout, None, silent); reset_term(&mut stdout); } else { show_usage_error!("bad usage"); @@ -204,38 +206,18 @@ fn reset_term(stdout: &mut std::io::Stdout) { #[inline(always)] fn reset_term(_: &mut usize) {} -fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>) { +fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>, silent: bool) { let (cols, rows) = terminal::size().unwrap(); let lines = break_buff(buff, usize::from(cols)); - let line_count: u16 = lines.len().try_into().unwrap(); - let mut upper_mark = 0; - let mut lines_left = line_count.saturating_sub(upper_mark + rows); - - draw( - &mut upper_mark, - rows, - &mut stdout, - lines.clone(), - line_count, - next_file, - ); - - let is_last = next_file.is_none(); - - // Specifies whether we have reached the end of the file and should - // return on the next key press. However, we immediately return when - // this is the last file. - let mut to_be_done = false; - if lines_left == 0 && is_last { - if is_last { - return; - } else { - to_be_done = true; - } + let mut pager = Pager::new(rows as usize, lines, next_file, silent); + pager.draw(stdout, false); + if pager.should_close() { + return; } loop { + let mut wrong_key = false; if event::poll(Duration::from_millis(10)).unwrap() { match event::read().unwrap() { Event::Key(KeyEvent { @@ -257,59 +239,127 @@ fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>) { code: KeyCode::Char(' '), modifiers: KeyModifiers::NONE, }) => { - upper_mark = upper_mark.saturating_add(rows.saturating_sub(1)); + pager.page_down(); } Event::Key(KeyEvent { code: KeyCode::Up, modifiers: KeyModifiers::NONE, }) => { - upper_mark = upper_mark.saturating_sub(rows.saturating_sub(1)); + pager.page_up(); + } + _ => { + wrong_key = true; } - _ => continue, } - lines_left = line_count.saturating_sub(upper_mark + rows); - draw( - &mut upper_mark, - rows, - &mut stdout, - lines.clone(), - line_count, - next_file, - ); - if lines_left == 0 { - if to_be_done || is_last { - return; - } - to_be_done = true; + pager.draw(stdout, wrong_key); + if pager.should_close() { + return; } } } } -fn draw( - upper_mark: &mut u16, - rows: u16, - mut stdout: &mut std::io::Stdout, +struct Pager<'a> { + // The current line at the top of the screen + upper_mark: usize, + // The number of rows that fit on the screen + content_rows: usize, lines: Vec, - lc: u16, - next_file: Option<&str>, -) { - execute!(stdout, terminal::Clear(terminal::ClearType::CurrentLine)).unwrap(); - let (up_mark, lower_mark) = calc_range(*upper_mark, rows, lc); - // Reduce the row by 1 for the prompt - let displayed_lines = lines - .iter() - .skip(up_mark.into()) - .take(usize::from(rows.saturating_sub(1))); + next_file: Option<&'a str>, + line_count: usize, + close_on_down: bool, + silent: bool, +} - for line in displayed_lines { - stdout - .write_all(format!("\r{}\n", line).as_bytes()) - .unwrap(); +impl<'a> Pager<'a> { + fn new(rows: usize, lines: Vec, next_file: Option<&'a str>, silent: bool) -> Self { + let line_count = lines.len(); + Self { + upper_mark: 0, + content_rows: rows - 1, + lines, + next_file, + line_count, + close_on_down: false, + silent, + } + } + + fn should_close(&mut self) -> bool { + if self.upper_mark + self.content_rows >= self.line_count { + if self.close_on_down { + return true; + } + if self.next_file.is_none() { + return true; + } else { + self.close_on_down = true; + } + } else { + self.close_on_down = false; + } + false + } + + fn page_down(&mut self) { + self.upper_mark += self.content_rows; + } + + fn page_up(&mut self) { + self.upper_mark = self.upper_mark.saturating_sub(self.content_rows); + } + + fn draw(&self, stdout: &mut std::io::Stdout, wrong_key: bool) { + let lower_mark = self.line_count.min(self.upper_mark + self.content_rows); + self.draw_lines(stdout); + self.draw_prompt(stdout, lower_mark, wrong_key); + stdout.flush().unwrap(); + } + + fn draw_lines(&self, stdout: &mut std::io::Stdout) { + execute!(stdout, terminal::Clear(terminal::ClearType::CurrentLine)).unwrap(); + let displayed_lines = self + .lines + .iter() + .skip(self.upper_mark) + .take(self.content_rows); + + for line in displayed_lines { + stdout + .write_all(format!("\r{}\n", line).as_bytes()) + .unwrap(); + } + } + + fn draw_prompt(&self, stdout: &mut Stdout, lower_mark: usize, wrong_key: bool) { + let status_inner = if lower_mark == self.line_count { + format!("Next file: {}", self.next_file.unwrap_or_default()) + } else { + format!( + "{}%", + (lower_mark as f64 / self.line_count as f64 * 100.0).round() as u16 + ) + }; + + let status = format!("--More--({})", status_inner); + + let banner = match (self.silent, wrong_key) { + (true, true) => "[Press 'h' for instructions. (unimplemented)]".to_string(), + (true, false) => format!("{}[Press space to continue, 'q' to quit.]", status), + (false, true) => format!("{}{}", status, BELL), + (false, false) => status, + }; + + write!( + stdout, + "\r{}{}{}", + Attribute::Reverse, + banner, + Attribute::Reset + ) + .unwrap(); } - make_prompt_and_flush(&mut stdout, lower_mark, lc, next_file); - *upper_mark = up_mark; } // Break the lines on the cols of the terminal @@ -350,52 +400,11 @@ fn break_line(line: &str, cols: usize) -> Vec { lines } -// Calculate upper_mark based on certain parameters -fn calc_range(mut upper_mark: u16, rows: u16, line_count: u16) -> (u16, u16) { - let mut lower_mark = upper_mark.saturating_add(rows); - - if lower_mark >= line_count { - upper_mark = line_count.saturating_sub(rows).saturating_add(1); - lower_mark = line_count; - } else { - lower_mark = lower_mark.saturating_sub(1) - } - (upper_mark, lower_mark) -} - -// Make a prompt similar to original more -fn make_prompt_and_flush(stdout: &mut Stdout, lower_mark: u16, lc: u16, next_file: Option<&str>) { - let status = if lower_mark == lc { - format!("Next file: {}", next_file.unwrap_or_default()) - } else { - format!( - "{}%", - (lower_mark as f64 / lc as f64 * 100.0).round() as u16 - ) - }; - write!( - stdout, - "\r{}--More--({}){}", - Attribute::Reverse, - status, - Attribute::Reset - ) - .unwrap(); - stdout.flush().unwrap(); -} - #[cfg(test)] mod tests { - use super::{break_line, calc_range}; + use super::break_line; use unicode_width::UnicodeWidthStr; - // It is good to test the above functions - #[test] - fn test_calc_range() { - assert_eq!((0, 24), calc_range(0, 25, 100)); - assert_eq!((50, 74), calc_range(50, 25, 100)); - assert_eq!((76, 100), calc_range(85, 25, 100)); - } #[test] fn test_break_lines_long() { let mut test_string = String::with_capacity(100); diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 6b6482702..bb402737e 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -389,7 +389,7 @@ fn rename_with_fallback(from: &Path, to: &Path) -> io::Result<()> { let file_type = metadata.file_type(); if file_type.is_symlink() { - rename_symlink_fallback(&from, &to)?; + rename_symlink_fallback(from, to)?; } else if file_type.is_dir() { // We remove the destination directory if it exists to match the // behavior of `fs::rename`. As far as I can tell, `fs_extra`'s diff --git a/src/uu/nl/src/nl.rs b/src/uu/nl/src/nl.rs index c062eedd9..a3181e11f 100644 --- a/src/uu/nl/src/nl.rs +++ b/src/uu/nl/src/nl.rs @@ -247,7 +247,7 @@ fn nl(reader: &mut BufReader, settings: &Settings) { let mut line_filter: fn(&str, ®ex::Regex) -> bool = pass_regex; for mut l in reader.lines().map(|r| r.unwrap()) { // Sanitize the string. We want to print the newline ourselves. - if !l.is_empty() && l.chars().rev().next().unwrap() == '\n' { + if l.ends_with('\n') { l.pop(); } // Next we iterate through the individual chars to see if this diff --git a/src/uu/nohup/Cargo.toml b/src/uu/nohup/Cargo.toml index 5bbbd9dff..839219a84 100644 --- a/src/uu/nohup/Cargo.toml +++ b/src/uu/nohup/Cargo.toml @@ -17,6 +17,7 @@ path = "src/nohup.rs" [dependencies] clap = "2.33" libc = "0.2.42" +atty = "0.2" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/nohup/src/nohup.rs b/src/uu/nohup/src/nohup.rs index ea379ff49..4e6fd7a7e 100644 --- a/src/uu/nohup/src/nohup.rs +++ b/src/uu/nohup/src/nohup.rs @@ -19,7 +19,6 @@ use std::fs::{File, OpenOptions}; use std::io::Error; use std::os::unix::prelude::*; use std::path::{Path, PathBuf}; -use uucore::fs::{is_stderr_interactive, is_stdin_interactive, is_stdout_interactive}; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Run COMMAND ignoring hangup signals."; @@ -84,7 +83,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } fn replace_fds() { - if is_stdin_interactive() { + if atty::is(atty::Stream::Stdin) { let new_stdin = match File::open(Path::new("/dev/null")) { Ok(t) => t, Err(e) => crash!(2, "Cannot replace STDIN: {}", e), @@ -94,7 +93,7 @@ fn replace_fds() { } } - if is_stdout_interactive() { + if atty::is(atty::Stream::Stdout) { let new_stdout = find_stdout(); let fd = new_stdout.as_raw_fd(); @@ -103,7 +102,7 @@ fn replace_fds() { } } - if is_stderr_interactive() && unsafe { dup2(1, 2) } != 2 { + if atty::is(atty::Stream::Stderr) && unsafe { dup2(1, 2) } != 2 { crash!(2, "Cannot replace STDERR: {}", Error::last_os_error()) } } diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index ebe380569..ee692d8f0 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -238,7 +238,7 @@ fn format_and_print_delimited(s: &str, options: &NumfmtOptions) -> Result<()> { } if field_selected { - print!("{}", format_string(&field.trim_start(), options, None)?); + print!("{}", format_string(field.trim_start(), options, None)?); } else { // print unselected field without conversion print!("{}", field); @@ -271,7 +271,7 @@ fn format_and_print_whitespace(s: &str, options: &NumfmtOptions) -> Result<()> { None }; - print!("{}", format_string(&field, options, implicit_padding)?); + print!("{}", format_string(field, options, implicit_padding)?); } else { // print unselected field without conversion print!("{}{}", prefix, field); diff --git a/src/uu/od/src/formatteriteminfo.rs b/src/uu/od/src/formatteriteminfo.rs index d44d97a92..13cf62246 100644 --- a/src/uu/od/src/formatteriteminfo.rs +++ b/src/uu/od/src/formatteriteminfo.rs @@ -2,6 +2,7 @@ use std::fmt; +#[allow(clippy::enum_variant_names)] #[derive(Copy)] pub enum FormatWriter { IntWriter(fn(u64) -> String), diff --git a/src/uu/od/src/inputdecoder.rs b/src/uu/od/src/inputdecoder.rs index f6ba59885..606495461 100644 --- a/src/uu/od/src/inputdecoder.rs +++ b/src/uu/od/src/inputdecoder.rs @@ -115,7 +115,7 @@ impl<'a> MemoryDecoder<'a> { /// Creates a clone of the internal buffer. The clone only contain the valid data. pub fn clone_buffer(&self, other: &mut Vec) { - other.clone_from(&self.data); + other.clone_from(self.data); other.resize(self.used_normal_length, 0); } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 33303f0fc..bf6c39011 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -43,6 +43,7 @@ use crate::partialreader::*; use crate::peekreader::*; use crate::prn_char::format_ascii_dump; use clap::{self, crate_version, AppSettings, Arg, ArgMatches}; +use uucore::parse_size::ParseSizeError; use uucore::InvalidEncodingHandling; const PEEK_BUFFER_SIZE: usize = 4; // utf-8 can be 4 bytes @@ -128,42 +129,34 @@ impl OdOptions { } }; - let mut skip_bytes = match matches.value_of(options::SKIP_BYTES) { - None => 0, - Some(s) => match parse_number_of_bytes(&s) { - Ok(i) => i, - Err(_) => { - return Err(format!("Invalid argument --skip-bytes={}", s)); - } - }, - }; + let mut skip_bytes = matches.value_of(options::SKIP_BYTES).map_or(0, |s| { + parse_number_of_bytes(s).unwrap_or_else(|e| { + crash!(1, "{}", format_error_message(e, s, options::SKIP_BYTES)) + }) + }); let mut label: Option = None; - let input_strings = match parse_inputs(&matches) { - Ok(CommandLineInputs::FileNames(v)) => v, - Ok(CommandLineInputs::FileAndOffset((f, s, l))) => { + let parsed_input = parse_inputs(&matches).map_err(|e| format!("Invalid inputs: {}", e))?; + let input_strings = match parsed_input { + CommandLineInputs::FileNames(v) => v, + CommandLineInputs::FileAndOffset((f, s, l)) => { skip_bytes = s; label = l; vec![f] } - Err(e) => { - return Err(format!("Invalid inputs: {}", e)); - } }; - let formats = match parse_format_flags(&args) { - Ok(f) => f, - Err(e) => { - return Err(e); - } - }; + let formats = parse_format_flags(&args)?; + + let mut line_bytes = matches.value_of(options::WIDTH).map_or(16, |s| { + if matches.occurrences_of(options::WIDTH) == 0 { + return 16; + }; + parse_number_of_bytes(s) + .unwrap_or_else(|e| crash!(1, "{}", format_error_message(e, s, options::WIDTH))) + }); - let mut line_bytes = match matches.value_of(options::WIDTH) { - None => 16, - Some(_) if matches.occurrences_of(options::WIDTH) == 0 => 16, - Some(s) => s.parse::().unwrap_or(0), - }; let min_bytes = formats.iter().fold(1, |max, next| { cmp::max(max, next.formatter_item_info.byte_size) }); @@ -174,15 +167,11 @@ impl OdOptions { let output_duplicates = matches.is_present(options::OUTPUT_DUPLICATES); - let read_bytes = match matches.value_of(options::READ_BYTES) { - None => None, - Some(s) => match parse_number_of_bytes(&s) { - Ok(i) => Some(i), - Err(_) => { - return Err(format!("Invalid argument --read-bytes={}", s)); - } - }, - }; + let read_bytes = matches.value_of(options::READ_BYTES).map(|s| { + parse_number_of_bytes(s).unwrap_or_else(|e| { + crash!(1, "{}", format_error_message(e, s, options::READ_BYTES)) + }) + }); let radix = match matches.value_of(options::ADDRESS_RADIX) { None => Radix::Octal, @@ -263,7 +252,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .short("S") .long(options::STRINGS) .help( - "output strings of at least BYTES graphic chars. 3 is assumed when \ + "NotImplemented: output strings of at least BYTES graphic chars. 3 is assumed when \ BYTES is not specified.", ) .default_value("3") @@ -453,8 +442,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let od_options = match OdOptions::new(clap_matches, args) { Err(s) => { - show_usage_error!("{}", s); - return 1; + crash!(1, "{}", s); } Ok(o) => o, }; @@ -537,7 +525,7 @@ where print_bytes( &input_offset.format_byte_offset(), &memory_decoder, - &output_info, + output_info, ); } @@ -636,3 +624,13 @@ fn open_input_peek_reader( let pr = PartialReader::new(mf, skip_bytes, read_bytes); PeekReader::new(pr) } + +fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String { + // NOTE: + // GNU's od echos affected flag, -N or --read-bytes (-j or --skip-bytes, etc.), depending user's selection + // GNU's od does distinguish between "invalid (suffix in) argument" + match error { + ParseSizeError::ParseFailure(_) => format!("invalid --{} argument '{}'", option, s), + ParseSizeError::SizeTooBig(_) => format!("--{} argument '{}' too large", option, s), + } +} diff --git a/src/uu/od/src/output_info.rs b/src/uu/od/src/output_info.rs index a204fa36e..49c2a09a2 100644 --- a/src/uu/od/src/output_info.rs +++ b/src/uu/od/src/output_info.rs @@ -68,7 +68,7 @@ impl OutputInfo { let print_width_line = print_width_block * (line_bytes / byte_size_block); let spaced_formatters = - OutputInfo::create_spaced_formatter_info(&formats, byte_size_block, print_width_block); + OutputInfo::create_spaced_formatter_info(formats, byte_size_block, print_width_block); OutputInfo { byte_size_line: line_bytes, diff --git a/src/uu/od/src/parse_formats.rs b/src/uu/od/src/parse_formats.rs index abf05ea18..f5b150d61 100644 --- a/src/uu/od/src/parse_formats.rs +++ b/src/uu/od/src/parse_formats.rs @@ -108,10 +108,8 @@ pub fn parse_format_flags(args: &[String]) -> Result formats.extend(v.into_iter()), - Err(e) => return Err(e), - } + let v = parse_type_string(arg)?; + formats.extend(v.into_iter()); expect_type_string = false; } else if arg.starts_with("--") { if arg.len() == 2 { @@ -119,10 +117,8 @@ pub fn parse_format_flags(args: &[String]) -> Result Result formats.extend(v.into_iter()), - Err(e) => return Err(e), - } + let v = parse_type_string(&format_spec)?; + formats.extend(v.into_iter()); expect_type_string = false; } } @@ -275,17 +269,13 @@ fn parse_type_string(params: &str) -> Result, Strin let mut chars = params.chars(); let mut ch = chars.next(); - while ch.is_some() { - let type_char = ch.unwrap(); - let type_char = match format_type(type_char) { - Some(t) => t, - None => { - return Err(format!( - "unexpected char '{}' in format specification '{}'", - type_char, params - )); - } - }; + while let Some(type_char) = ch { + let type_char = format_type(type_char).ok_or_else(|| { + format!( + "unexpected char '{}' in format specification '{}'", + type_char, params + ) + })?; let type_cat = format_type_category(type_char); @@ -301,30 +291,25 @@ fn parse_type_string(params: &str) -> Result, Strin ch = chars.next(); } if !decimal_size.is_empty() { - byte_size = match decimal_size.parse() { - Err(_) => { - return Err(format!( - "invalid number '{}' in format specification '{}'", - decimal_size, params - )) - } - Ok(n) => n, - } + byte_size = decimal_size.parse().map_err(|_| { + format!( + "invalid number '{}' in format specification '{}'", + decimal_size, params + ) + })?; } } if is_format_dump_char(ch, &mut show_ascii_dump) { ch = chars.next(); } - match od_format_type(type_char, byte_size) { - Some(ft) => formats.push(ParsedFormatterItemInfo::new(ft, show_ascii_dump)), - None => { - return Err(format!( - "invalid size '{}' in format specification '{}'", - byte_size, params - )) - } - } + let ft = od_format_type(type_char, byte_size).ok_or_else(|| { + format!( + "invalid size '{}' in format specification '{}'", + byte_size, params + ) + })?; + formats.push(ParsedFormatterItemInfo::new(ft, show_ascii_dump)); } Ok(formats) @@ -335,16 +320,13 @@ pub fn parse_format_flags_str( args_str: &Vec<&'static str>, ) -> Result, String> { let args: Vec = args_str.iter().map(|s| s.to_string()).collect(); - match parse_format_flags(&args) { - Err(e) => Err(e), - Ok(v) => { - // tests using this function assume add_ascii_dump is not set - Ok(v.into_iter() - .inspect(|f| assert!(!f.add_ascii_dump)) - .map(|f| f.formatter_item_info) - .collect()) - } - } + parse_format_flags(&args).map(|v| { + // tests using this function assume add_ascii_dump is not set + v.into_iter() + .inspect(|f| assert!(!f.add_ascii_dump)) + .map(|f| f.formatter_item_info) + .collect() + }) } #[test] diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index 288c0870f..419b7173d 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -55,7 +55,7 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result) -> Result Ok(CommandLineInputs::FileNames(vec!["-".to_string()])), 1 => { - let offset0 = parse_offset_operand(&input_strings[0]); + let offset0 = parse_offset_operand(input_strings[0]); Ok(match offset0 { Ok(n) => CommandLineInputs::FileAndOffset(("-".to_string(), n, None)), _ => CommandLineInputs::FileNames( @@ -97,8 +97,8 @@ pub fn parse_inputs_traditional(input_strings: Vec<&str>) -> Result { - let offset0 = parse_offset_operand(&input_strings[0]); - let offset1 = parse_offset_operand(&input_strings[1]); + let offset0 = parse_offset_operand(input_strings[0]); + let offset1 = parse_offset_operand(input_strings[1]); match (offset0, offset1) { (Ok(n), Ok(m)) => Ok(CommandLineInputs::FileAndOffset(( "-".to_string(), @@ -114,8 +114,8 @@ pub fn parse_inputs_traditional(input_strings: Vec<&str>) -> Result { - let offset = parse_offset_operand(&input_strings[1]); - let label = parse_offset_operand(&input_strings[2]); + let offset = parse_offset_operand(input_strings[1]); + let label = parse_offset_operand(input_strings[2]); match (offset, label) { (Ok(n), Ok(m)) => Ok(CommandLineInputs::FileAndOffset(( input_strings[0].to_string(), diff --git a/src/uu/od/src/parse_nrofbytes.rs b/src/uu/od/src/parse_nrofbytes.rs index d2ba1527b..d6329c60a 100644 --- a/src/uu/od/src/parse_nrofbytes.rs +++ b/src/uu/od/src/parse_nrofbytes.rs @@ -1,14 +1,17 @@ -pub fn parse_number_of_bytes(s: &str) -> Result { +use uucore::parse_size::{parse_size, ParseSizeError}; + +pub fn parse_number_of_bytes(s: &str) -> Result { let mut start = 0; let mut len = s.len(); - let mut radix = 10; + let mut radix = 16; let mut multiply = 1; if s.starts_with("0x") || s.starts_with("0X") { start = 2; - radix = 16; } else if s.starts_with('0') { radix = 8; + } else { + return parse_size(&s[start..]); } let mut ends_with = s.chars().rev(); @@ -56,78 +59,33 @@ pub fn parse_number_of_bytes(s: &str) -> Result { Some('P') => 1000 * 1000 * 1000 * 1000 * 1000, #[cfg(target_pointer_width = "64")] Some('E') => 1000 * 1000 * 1000 * 1000 * 1000 * 1000, - _ => return Err("parse failed"), + _ => return Err(ParseSizeError::ParseFailure(s.to_string())), } } _ => {} } - match usize::from_str_radix(&s[start..len], radix) { - Ok(i) => Ok(i * multiply), - Err(_) => Err("parse failed"), - } -} - -#[allow(dead_code)] -fn parse_number_of_bytes_str(s: &str) -> Result { - parse_number_of_bytes(&String::from(s)) + let factor = match usize::from_str_radix(&s[start..len], radix) { + Ok(f) => f, + Err(e) => return Err(ParseSizeError::ParseFailure(e.to_string())), + }; + factor + .checked_mul(multiply) + .ok_or_else(|| ParseSizeError::SizeTooBig(s.to_string())) } #[test] fn test_parse_number_of_bytes() { - // normal decimal numbers - assert_eq!(0, parse_number_of_bytes_str("0").unwrap()); - assert_eq!(5, parse_number_of_bytes_str("5").unwrap()); - assert_eq!(999, parse_number_of_bytes_str("999").unwrap()); - assert_eq!(2 * 512, parse_number_of_bytes_str("2b").unwrap()); - assert_eq!(2 * 1024, parse_number_of_bytes_str("2k").unwrap()); - assert_eq!(4 * 1024, parse_number_of_bytes_str("4K").unwrap()); - assert_eq!(2 * 1048576, parse_number_of_bytes_str("2m").unwrap()); - assert_eq!(4 * 1048576, parse_number_of_bytes_str("4M").unwrap()); - assert_eq!(1073741824, parse_number_of_bytes_str("1G").unwrap()); - assert_eq!(2000, parse_number_of_bytes_str("2kB").unwrap()); - assert_eq!(4000, parse_number_of_bytes_str("4KB").unwrap()); - assert_eq!(2000000, parse_number_of_bytes_str("2mB").unwrap()); - assert_eq!(4000000, parse_number_of_bytes_str("4MB").unwrap()); - assert_eq!(2000000000, parse_number_of_bytes_str("2GB").unwrap()); - // octal input - assert_eq!(8, parse_number_of_bytes_str("010").unwrap()); - assert_eq!(8 * 512, parse_number_of_bytes_str("010b").unwrap()); - assert_eq!(8 * 1024, parse_number_of_bytes_str("010k").unwrap()); - assert_eq!(8 * 1048576, parse_number_of_bytes_str("010m").unwrap()); + assert_eq!(8, parse_number_of_bytes("010").unwrap()); + assert_eq!(8 * 512, parse_number_of_bytes("010b").unwrap()); + assert_eq!(8 * 1024, parse_number_of_bytes("010k").unwrap()); + assert_eq!(8 * 1_048_576, parse_number_of_bytes("010m").unwrap()); // hex input - assert_eq!(15, parse_number_of_bytes_str("0xf").unwrap()); - assert_eq!(15, parse_number_of_bytes_str("0XF").unwrap()); - assert_eq!(27, parse_number_of_bytes_str("0x1b").unwrap()); - assert_eq!(16 * 1024, parse_number_of_bytes_str("0x10k").unwrap()); - assert_eq!(16 * 1048576, parse_number_of_bytes_str("0x10m").unwrap()); - - // invalid input - parse_number_of_bytes_str("").unwrap_err(); - parse_number_of_bytes_str("-1").unwrap_err(); - parse_number_of_bytes_str("1e2").unwrap_err(); - parse_number_of_bytes_str("xyz").unwrap_err(); - parse_number_of_bytes_str("b").unwrap_err(); - parse_number_of_bytes_str("1Y").unwrap_err(); - parse_number_of_bytes_str("∞").unwrap_err(); -} - -#[test] -#[cfg(target_pointer_width = "64")] -fn test_parse_number_of_bytes_64bits() { - assert_eq!(1099511627776, parse_number_of_bytes_str("1T").unwrap()); - assert_eq!(1125899906842624, parse_number_of_bytes_str("1P").unwrap()); - assert_eq!( - 1152921504606846976, - parse_number_of_bytes_str("1E").unwrap() - ); - - assert_eq!(2000000000000, parse_number_of_bytes_str("2TB").unwrap()); - assert_eq!(2000000000000000, parse_number_of_bytes_str("2PB").unwrap()); - assert_eq!( - 2000000000000000000, - parse_number_of_bytes_str("2EB").unwrap() - ); + assert_eq!(15, parse_number_of_bytes("0xf").unwrap()); + assert_eq!(15, parse_number_of_bytes("0XF").unwrap()); + assert_eq!(27, parse_number_of_bytes("0x1b").unwrap()); + assert_eq!(16 * 1024, parse_number_of_bytes("0x10k").unwrap()); + assert_eq!(16 * 1_048_576, parse_number_of_bytes("0x10m").unwrap()); } diff --git a/src/uu/od/src/partialreader.rs b/src/uu/od/src/partialreader.rs index ee3588830..f155a7bd2 100644 --- a/src/uu/od/src/partialreader.rs +++ b/src/uu/od/src/partialreader.rs @@ -36,16 +36,15 @@ impl Read for PartialReader { while self.skip > 0 { let skip_count = cmp::min(self.skip, MAX_SKIP_BUFFER); - match self.inner.read(&mut bytes[..skip_count]) { - Ok(0) => { + match self.inner.read(&mut bytes[..skip_count])? { + 0 => { // this is an error as we still have more to skip return Err(io::Error::new( io::ErrorKind::UnexpectedEof, "tried to skip past end of input", )); } - Ok(n) => self.skip -= n, - Err(e) => return Err(e), + n => self.skip -= n, } } } diff --git a/src/uu/pathchk/src/pathchk.rs b/src/uu/pathchk/src/pathchk.rs index 9667e0ba1..358881509 100644 --- a/src/uu/pathchk/src/pathchk.rs +++ b/src/uu/pathchk/src/pathchk.rs @@ -118,10 +118,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // check a path, given as a slice of it's components and an operating mode fn check_path(mode: &Mode, path: &[String]) -> bool { match *mode { - Mode::Basic => check_basic(&path), - Mode::Extra => check_default(&path) && check_extra(&path), - Mode::Both => check_basic(&path) && check_extra(&path), - _ => check_default(&path), + Mode::Basic => check_basic(path), + Mode::Extra => check_default(path) && check_extra(path), + Mode::Both => check_basic(path) && check_extra(path), + _ => check_default(path), } } @@ -156,7 +156,7 @@ fn check_basic(path: &[String]) -> bool { ); return false; } - if !check_portable_chars(&p) { + if !check_portable_chars(p) { return false; } } @@ -168,7 +168,7 @@ fn check_basic(path: &[String]) -> bool { fn check_extra(path: &[String]) -> bool { // components: leading hyphens for p in path { - if !no_leading_hyphen(&p) { + if !no_leading_hyphen(p) { writeln!( &mut std::io::stderr(), "leading hyphen in file name component '{}'", @@ -241,13 +241,14 @@ fn no_leading_hyphen(path_segment: &str) -> bool { // check whether a path segment contains only valid (read: portable) characters fn check_portable_chars(path_segment: &str) -> bool { - let valid_str = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789._-".to_string(); - for ch in path_segment.chars() { - if !valid_str.contains(ch) { + const VALID_CHARS: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789._-"; + for (i, ch) in path_segment.as_bytes().iter().enumerate() { + if !VALID_CHARS.contains(ch) { + let invalid = path_segment[i..].chars().next().unwrap(); writeln!( &mut std::io::stderr(), "nonportable character '{}' in file name component '{}'", - ch, + invalid, path_segment ); return false; diff --git a/src/uu/pinky/src/pinky.rs b/src/uu/pinky/src/pinky.rs index 27dcc2421..d15730b32 100644 --- a/src/uu/pinky/src/pinky.rs +++ b/src/uu/pinky/src/pinky.rs @@ -283,7 +283,7 @@ impl Pinky { } } - print!(" {}", time_string(&ut)); + print!(" {}", time_string(ut)); let mut s = ut.host(); if self.include_where && !s.is_empty() { diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 486cedc00..239a0970f 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -401,18 +401,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for file_group in file_groups { let result_options = build_options(&matches, &file_group, args.join(" ")); + let options = match result_options { + Ok(options) => options, + Err(err) => { + print_error(&matches, err); + return 1; + } + }; - if result_options.is_err() { - print_error(&matches, result_options.err().unwrap()); - return 1; - } - - let options = &result_options.unwrap(); - - let cmd_result = if file_group.len() == 1 { - pr(&file_group.get(0).unwrap(), options) + let cmd_result = if let Ok(group) = file_group.iter().exactly_one() { + pr(group, &options) } else { - mpr(&file_group, options) + mpr(&file_group, &options) }; let status = match cmd_result { @@ -442,11 +442,12 @@ fn recreate_arguments(args: &[String]) -> Vec { let mut arguments = args.to_owned(); let num_option = args.iter().find_position(|x| n_regex.is_match(x.trim())); if let Some((pos, _value)) = num_option { - let num_val_opt = args.get(pos + 1); - if num_val_opt.is_some() && !num_regex.is_match(num_val_opt.unwrap()) { - let could_be_file = arguments.remove(pos + 1); - arguments.insert(pos + 1, format!("{}", NumberingMode::default().width)); - arguments.insert(pos + 2, could_be_file); + if let Some(num_val_opt) = args.get(pos + 1) { + if !num_regex.is_match(num_val_opt) { + let could_be_file = arguments.remove(pos + 1); + arguments.insert(pos + 1, format!("{}", NumberingMode::default().width)); + arguments.insert(pos + 2, could_be_file); + } } } @@ -666,12 +667,13 @@ fn build_options( None => end_page_in_plus_option, }; - if end_page.is_some() && start_page > end_page.unwrap() { - return Err(PrError::EncounteredErrors(format!( - "invalid --pages argument '{}:{}'", - start_page, - end_page.unwrap() - ))); + if let Some(end_page) = end_page { + if start_page > end_page { + return Err(PrError::EncounteredErrors(format!( + "invalid --pages argument '{}:{}'", + start_page, end_page + ))); + } } let default_lines_per_page = if form_feed_used { @@ -947,7 +949,7 @@ fn read_stream_and_create_pages( let current_page = x + 1; current_page >= start_page - && (last_page.is_none() || current_page <= last_page.unwrap()) + && last_page.map_or(true, |last_page| current_page <= last_page) }), ) } @@ -996,8 +998,8 @@ fn mpr(paths: &[String], options: &OutputOptions) -> Result { for (_key, file_line_group) in file_line_groups.into_iter() { for file_line in file_line_group { - if file_line.line_content.is_err() { - return Err(file_line.line_content.unwrap_err().into()); + if let Err(e) = file_line.line_content { + return Err(e.into()); } let new_page_number = file_line.page_number; if page_counter != new_page_number { @@ -1030,8 +1032,7 @@ fn print_page(lines: &[FileLine], options: &OutputOptions, page: usize) -> Resul let lines_written = write_columns(lines, options, out)?; - for index in 0..trailer_content.len() { - let x = trailer_content.get(index).unwrap(); + for (index, x) in trailer_content.iter().enumerate() { out.write_all(x.as_bytes())?; if index + 1 != trailer_content.len() { out.write_all(line_separator)?; @@ -1074,8 +1075,7 @@ fn write_columns( let mut offset = 0; for col in 0..columns { let mut inserted = 0; - for i in offset..lines.len() { - let line = lines.get(i).unwrap(); + for line in &lines[offset..] { if line.file_id != col { break; } @@ -1114,7 +1114,7 @@ fn write_columns( for (i, cell) in row.iter().enumerate() { if cell.is_none() && options.merge_files_print.is_some() { out.write_all( - get_line_for_printing(&options, &blank_line, columns, i, &line_width, indexes) + get_line_for_printing(options, &blank_line, columns, i, &line_width, indexes) .as_bytes(), )?; } else if cell.is_none() { @@ -1124,7 +1124,7 @@ fn write_columns( let file_line = cell.unwrap(); out.write_all( - get_line_for_printing(&options, file_line, columns, i, &line_width, indexes) + get_line_for_printing(options, file_line, columns, i, &line_width, indexes) .as_bytes(), )?; lines_printed += 1; @@ -1149,7 +1149,7 @@ fn get_line_for_printing( indexes: usize, ) -> String { let blank_line = String::new(); - let formatted_line_number = get_formatted_line_number(&options, file_line.line_number, index); + let formatted_line_number = get_formatted_line_number(options, file_line.line_number, index); let mut complete_line = format!( "{}{}", diff --git a/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs b/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs index f96a991b5..0ca993680 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs @@ -26,7 +26,7 @@ impl Formatter for CninetyNineHexFloatf { ) -> Option { let second_field = field.second_field.unwrap_or(6) + 1; let analysis = FloatAnalysis::analyze( - &str_in, + str_in, initial_prefix, Some(second_field as usize), None, diff --git a/src/uu/printf/src/tokenize/num_format/formatters/decf.rs b/src/uu/printf/src/tokenize/num_format/formatters/decf.rs index 5798eadcb..3376345e0 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/decf.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/decf.rs @@ -55,18 +55,9 @@ impl Formatter for Decf { ); // strip trailing zeroes if let Some(ref post_dec) = f_sci.post_decimal { - let mut i = post_dec.len(); - { - let mut it = post_dec.chars(); - while let Some(c) = it.next_back() { - if c != '0' { - break; - } - i -= 1; - } - } - if i != post_dec.len() { - f_sci.post_decimal = Some(String::from(&post_dec[0..i])); + let trimmed = post_dec.trim_end_matches('0'); + if trimmed.len() != post_dec.len() { + f_sci.post_decimal = Some(trimmed.to_owned()); } } let f_fl = get_primitive_dec( diff --git a/src/uu/printf/src/tokenize/num_format/formatters/float_common.rs b/src/uu/printf/src/tokenize/num_format/formatters/float_common.rs index dd8259233..97009b586 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/float_common.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/float_common.rs @@ -247,8 +247,12 @@ pub fn get_primitive_dec( first_segment.len() as isize - 1, ) } else { - match first_segment.chars().next() { - Some('0') => { + match first_segment + .chars() + .next() + .expect("float_common: no chars in first segment.") + { + '0' => { let it = second_segment.chars().enumerate(); let mut m: isize = 0; let mut pre = String::from("0"); @@ -266,10 +270,7 @@ pub fn get_primitive_dec( } (pre, post, m) } - Some(_) => (first_segment, second_segment, 0), - None => { - panic!("float_common: no chars in first segment."); - } + _ => (first_segment, second_segment, 0), } } } else { @@ -298,11 +299,11 @@ pub fn get_primitive_dec( pub fn primitive_to_str_common(prim: &FormatPrimitive, field: &FormatField) -> String { let mut final_str = String::new(); if let Some(ref prefix) = prim.prefix { - final_str.push_str(&prefix); + final_str.push_str(prefix); } match prim.pre_decimal { Some(ref pre_decimal) => { - final_str.push_str(&pre_decimal); + final_str.push_str(pre_decimal); } None => { panic!( diff --git a/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs b/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs index aed50f18e..afb2bcf08 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs @@ -21,7 +21,7 @@ impl Formatter for Floatf { ) -> Option { let second_field = field.second_field.unwrap_or(6) + 1; let analysis = FloatAnalysis::analyze( - &str_in, + str_in, initial_prefix, None, Some(second_field as usize), diff --git a/src/uu/printf/src/tokenize/num_format/formatters/intf.rs b/src/uu/printf/src/tokenize/num_format/formatters/intf.rs index 02c59211b..b6c18d436 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/intf.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/intf.rs @@ -252,7 +252,7 @@ impl Formatter for Intf { fn primitive_to_str(&self, prim: &FormatPrimitive, field: FormatField) -> String { let mut final_str: String = String::new(); if let Some(ref prefix) = prim.prefix { - final_str.push_str(&prefix); + final_str.push_str(prefix); } // integral second fields is zero-padded minimum-width // which gets handled before general minimum-width @@ -266,7 +266,7 @@ impl Formatter for Intf { i -= 1; } } - final_str.push_str(&pre_decimal); + final_str.push_str(pre_decimal); } None => { panic!( diff --git a/src/uu/printf/src/tokenize/num_format/num_format.rs b/src/uu/printf/src/tokenize/num_format/num_format.rs index a8a60cc57..b32731f2d 100644 --- a/src/uu/printf/src/tokenize/num_format/num_format.rs +++ b/src/uu/printf/src/tokenize/num_format/num_format.rs @@ -235,7 +235,7 @@ pub fn num_format(field: &FormatField, in_str_opt: Option<&String>) -> Option) -> Option { text_so_far.push('%'); - err_conv(&text_so_far); + err_conv(text_so_far); false } } diff --git a/src/uu/ptx/src/ptx.rs b/src/uu/ptx/src/ptx.rs index 69960ac49..31da8f05d 100644 --- a/src/uu/ptx/src/ptx.rs +++ b/src/uu/ptx/src/ptx.rs @@ -213,7 +213,7 @@ fn read_input(input_files: &[String], config: &Config) -> FileMap { files.push("-"); } else if config.gnu_ext { for file in input_files { - files.push(&file); + files.push(file); } } else { files.push(&input_files[0]); @@ -503,7 +503,7 @@ fn format_tex_line( let keyword = &line[word_ref.position..word_ref.position_end]; let after_chars_trim_idx = (word_ref.position_end, chars_line.len()); let all_after = &chars_line[after_chars_trim_idx.0..after_chars_trim_idx.1]; - let (tail, before, after, head) = get_output_chunks(&all_before, &keyword, &all_after, &config); + let (tail, before, after, head) = get_output_chunks(all_before, keyword, all_after, config); output.push_str(&format!( "{5}{0}{6}{5}{1}{6}{5}{2}{6}{5}{3}{6}{5}{4}{6}", format_tex_field(&tail), @@ -515,7 +515,7 @@ fn format_tex_line( "}" )); if config.auto_ref || config.input_ref { - output.push_str(&format!("{}{}{}", "{", format_tex_field(&reference), "}")); + output.push_str(&format!("{}{}{}", "{", format_tex_field(reference), "}")); } output } @@ -546,7 +546,7 @@ fn format_roff_line( let keyword = &line[word_ref.position..word_ref.position_end]; let after_chars_trim_idx = (word_ref.position_end, chars_line.len()); let all_after = &chars_line[after_chars_trim_idx.0..after_chars_trim_idx.1]; - let (tail, before, after, head) = get_output_chunks(&all_before, &keyword, &all_after, &config); + let (tail, before, after, head) = get_output_chunks(all_before, keyword, all_after, config); output.push_str(&format!( " \"{}\" \"{}\" \"{}{}\" \"{}\"", format_roff_field(&tail), @@ -556,7 +556,7 @@ fn format_roff_line( format_roff_field(&head) )); if config.auto_ref || config.input_ref { - output.push_str(&format!(" \"{}\"", format_roff_field(&reference))); + output.push_str(&format!(" \"{}\"", format_roff_field(reference))); } output } diff --git a/src/uu/rm/Cargo.toml b/src/uu/rm/Cargo.toml index 9974111aa..d84756fd3 100644 --- a/src/uu/rm/Cargo.toml +++ b/src/uu/rm/Cargo.toml @@ -18,10 +18,12 @@ path = "src/rm.rs" clap = "2.33" walkdir = "2.2" remove_dir_all = "0.5.1" +winapi = { version="0.3", features=[] } uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } + [[bin]] name = "rm" path = "src/main.rs" diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index ba764034a..40a24cea7 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -5,7 +5,7 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore (ToDO) bitor ulong +// spell-checker:ignore (path) eacces #[macro_use] extern crate uucore; @@ -430,9 +430,7 @@ use std::os::windows::prelude::MetadataExt; #[cfg(windows)] fn is_symlink_dir(metadata: &fs::Metadata) -> bool { - use std::os::raw::c_ulong; - pub type DWORD = c_ulong; - pub const FILE_ATTRIBUTE_DIRECTORY: DWORD = 0x10; + use winapi::um::winnt::FILE_ATTRIBUTE_DIRECTORY; metadata.file_type().is_symlink() && ((metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY) != 0) diff --git a/src/uu/rmdir/src/rmdir.rs b/src/uu/rmdir/src/rmdir.rs index d13a21f60..fc22cca09 100644 --- a/src/uu/rmdir/src/rmdir.rs +++ b/src/uu/rmdir/src/rmdir.rs @@ -88,7 +88,7 @@ fn remove(dirs: Vec, ignore: bool, parents: bool, verbose: bool) -> Resu for dir in &dirs { let path = Path::new(&dir[..]); - r = remove_dir(&path, ignore, verbose).and(r); + r = remove_dir(path, ignore, verbose).and(r); if parents { let mut p = path; while let Some(new_p) = p.parent() { @@ -109,17 +109,14 @@ fn remove(dirs: Vec, ignore: bool, parents: bool, verbose: bool) -> Resu } fn remove_dir(path: &Path, ignore: bool, verbose: bool) -> Result<(), i32> { - let mut read_dir = match fs::read_dir(path) { - Ok(m) => m, - Err(e) if e.raw_os_error() == Some(ENOTDIR) => { + let mut read_dir = fs::read_dir(path).map_err(|e| { + if e.raw_os_error() == Some(ENOTDIR) { show_error!("failed to remove '{}': Not a directory", path.display()); - return Err(1); - } - Err(e) => { + } else { show_error!("reading directory '{}': {}", path.display(), e); - return Err(1); } - }; + 1 + })?; let mut r = Ok(()); diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 6a43ed478..177143811 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -24,7 +24,7 @@ extern crate uucore; static NAME: &str = "shred"; const BLOCK_SIZE: usize = 512; -const NAME_CHARSET: &str = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_."; +const NAME_CHARSET: &[u8] = b"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_."; // Patterns as shown in the GNU coreutils shred implementation const PATTERNS: [&[u8]; 22] = [ @@ -89,7 +89,7 @@ impl Iterator for FilenameGenerator { // Make the return value, then increment let mut ret = String::new(); for i in name_charset_indices.iter() { - let c: char = NAME_CHARSET.chars().nth(*i).unwrap(); + let c = char::from(NAME_CHARSET[*i]); ret.push(c); } @@ -163,16 +163,14 @@ impl<'a> BytesGenerator<'a> { return None; } - let this_block_size = { - if !self.exact { + let this_block_size = if !self.exact { + self.block_size + } else { + let bytes_left = self.total_bytes - self.bytes_generated.get(); + if bytes_left >= self.block_size as u64 { self.block_size } else { - let bytes_left = self.total_bytes - self.bytes_generated.get(); - if bytes_left >= self.block_size as u64 { - self.block_size - } else { - (bytes_left % self.block_size as u64) as usize - } + (bytes_left % self.block_size as u64) as usize } }; @@ -184,12 +182,10 @@ impl<'a> BytesGenerator<'a> { rng.fill(bytes); } PassType::Pattern(pattern) => { - let skip = { - if self.bytes_generated.get() == 0 { - 0 - } else { - (pattern.len() as u64 % self.bytes_generated.get()) as usize - } + let skip = if self.bytes_generated.get() == 0 { + 0 + } else { + (pattern.len() as u64 % self.bytes_generated.get()) as usize }; // Copy the pattern in chunks rather than simply one byte at a time @@ -381,7 +377,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for path_str in matches.values_of(options::FILE).unwrap() { wipe_file( - &path_str, iterations, remove, size, exact, zero, verbose, force, + path_str, iterations, remove, size, exact, zero, verbose, force, ); } @@ -659,7 +655,7 @@ fn do_remove(path: &Path, orig_filename: &str, verbose: bool) -> Result<(), io:: println!("{}: {}: removing", NAME, orig_filename); } - let renamed_path: Option = wipe_name(&path, verbose); + let renamed_path: Option = wipe_name(path, verbose); if let Some(rp) = renamed_path { fs::remove_file(rp)?; } diff --git a/src/uu/shuf/src/shuf.rs b/src/uu/shuf/src/shuf.rs index 88a47585f..2d1f558de 100644 --- a/src/uu/shuf/src/shuf.rs +++ b/src/uu/shuf/src/shuf.rs @@ -285,14 +285,12 @@ fn parse_range(input_range: &str) -> Result<(usize, usize), String> { if split.len() != 2 { Err(format!("invalid input range: '{}'", input_range)) } else { - let begin = match split[0].parse::() { - Ok(m) => m, - Err(_) => return Err(format!("invalid input range: '{}'", split[0])), - }; - let end = match split[1].parse::() { - Ok(m) => m, - Err(_) => return Err(format!("invalid input range: '{}'", split[1])), - }; + let begin = split[0] + .parse::() + .map_err(|_| format!("invalid input range: '{}'", split[0]))?; + let end = split[1] + .parse::() + .map_err(|_| format!("invalid input range: '{}'", split[1]))?; Ok((begin, end + 1)) } } diff --git a/src/uu/sleep/Cargo.toml b/src/uu/sleep/Cargo.toml index fe7ee2941..618ea7e28 100644 --- a/src/uu/sleep/Cargo.toml +++ b/src/uu/sleep/Cargo.toml @@ -16,7 +16,7 @@ path = "src/sleep.rs" [dependencies] clap = "2.33" -uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["parse_time"] } +uucore = { version=">=0.0.8", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } [[bin]] diff --git a/src/uu/sort/src/check.rs b/src/uu/sort/src/check.rs index d3b9d6669..a8e5a0d9b 100644 --- a/src/uu/sort/src/check.rs +++ b/src/uu/sort/src/check.rs @@ -49,7 +49,7 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { let prev_last = prev_chunk.borrow_lines().last().unwrap(); let new_first = chunk.borrow_lines().first().unwrap(); - if compare_by(prev_last, new_first, &settings) == Ordering::Greater { + if compare_by(prev_last, new_first, settings) == Ordering::Greater { if !settings.check_silent { println!("sort: {}:{}: disorder: {}", path, line_idx, new_first.line); } @@ -60,7 +60,7 @@ pub fn check(path: &str, settings: &GlobalSettings) -> i32 { for (a, b) in chunk.borrow_lines().iter().tuple_windows() { line_idx += 1; - if compare_by(a, b, &settings) == Ordering::Greater { + if compare_by(a, b, settings) == Ordering::Greater { if !settings.check_silent { println!("sort: {}:{}: disorder: {}", path, line_idx, b.line); } diff --git a/src/uu/sort/src/chunks.rs b/src/uu/sort/src/chunks.rs index dde6febd3..3d996e6d6 100644 --- a/src/uu/sort/src/chunks.rs +++ b/src/uu/sort/src/chunks.rs @@ -90,7 +90,7 @@ pub fn read( if buffer.len() < carry_over.len() { buffer.resize(carry_over.len() + 10 * 1024, 0); } - buffer[..carry_over.len()].copy_from_slice(&carry_over); + buffer[..carry_over.len()].copy_from_slice(carry_over); let (read, should_continue) = read_to_buffer( file, next_files, @@ -110,7 +110,7 @@ pub fn read( std::mem::transmute::>, Vec>>(lines) }; let read = crash_if_err!(1, std::str::from_utf8(&buf[..read])); - parse_lines(read, &mut lines, separator, &settings); + parse_lines(read, &mut lines, separator, settings); lines }); sender.send(payload).unwrap(); @@ -194,7 +194,7 @@ fn read_to_buffer( continue; } } - let mut sep_iter = memchr_iter(separator, &buffer).rev(); + let mut sep_iter = memchr_iter(separator, buffer).rev(); let last_line_end = sep_iter.next(); if sep_iter.next().is_some() { // We read enough lines. diff --git a/src/uu/sort/src/custom_str_cmp.rs b/src/uu/sort/src/custom_str_cmp.rs index a087a9fc2..089d33bc4 100644 --- a/src/uu/sort/src/custom_str_cmp.rs +++ b/src/uu/sort/src/custom_str_cmp.rs @@ -38,7 +38,7 @@ pub fn custom_str_cmp( ) -> Ordering { if !(ignore_case || ignore_non_dictionary || ignore_non_printing) { // There are no custom settings. Fall back to the default strcmp, which is faster. - return a.cmp(&b); + return a.cmp(b); } let mut a_chars = a .chars() diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 70e3325ad..c2b82ee68 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -43,6 +43,7 @@ use std::ops::Range; use std::path::Path; use std::path::PathBuf; use unicode_width::UnicodeWidthStr; +use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::InvalidEncodingHandling; static NAME: &str = "sort"; @@ -162,32 +163,29 @@ pub struct GlobalSettings { } impl GlobalSettings { - /// Interpret this `&str` as a number with an optional trailing si unit. - /// - /// If there is no trailing si unit, the implicit unit is K. - /// The suffix B causes the number to be interpreted as a byte count. - fn parse_byte_count(input: &str) -> usize { - const SI_UNITS: &[char] = &['B', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y']; + /// Parse a SIZE string into a number of bytes. + /// A size string comprises an integer and an optional unit. + /// The unit may be k, K, m, M, g, G, t, T, P, E, Z, Y (powers of 1024), or b which is 1. + /// Default is K. + fn parse_byte_count(input: &str) -> Result { + // GNU sort (8.32) valid: 1b, k, K, m, M, g, G, t, T, P, E, Z, Y + // GNU sort (8.32) invalid: b, B, 1B, p, e, z, y + const ALLOW_LIST: &[char] = &[ + 'b', 'k', 'K', 'm', 'M', 'g', 'G', 't', 'T', 'P', 'E', 'Z', 'Y', + ]; + let mut size_string = input.trim().to_string(); - let input = input.trim(); - - let (num_str, si_unit) = - if input.ends_with(|c: char| SI_UNITS.contains(&c.to_ascii_uppercase())) { - let mut chars = input.chars(); - let si_suffix = chars.next_back().unwrap().to_ascii_uppercase(); - let si_unit = SI_UNITS.iter().position(|&c| c == si_suffix).unwrap(); - let num_str = chars.as_str(); - (num_str, si_unit) - } else { - (input, 1) - }; - - let num_usize: usize = num_str - .trim() - .parse() - .unwrap_or_else(|e| crash!(1, "failed to parse buffer size `{}`: {}", num_str, e)); - - num_usize.saturating_mul(1000usize.saturating_pow(si_unit as u32)) + if size_string.ends_with(|c: char| ALLOW_LIST.contains(&c) || c.is_digit(10)) { + // b 1, K 1024 (default) + if size_string.ends_with(|c: char| c.is_digit(10)) { + size_string.push('K'); + } else if size_string.ends_with('b') { + size_string.pop(); + } + parse_size(&size_string) + } else { + Err(ParseSizeError::ParseFailure("invalid suffix".to_string())) + } } fn out_writer(&self) -> BufWriter> { @@ -400,9 +398,9 @@ impl<'a> Line<'a> { let line = self.line.replace('\t', ">"); writeln!(writer, "{}", line)?; - let fields = tokenize(&self.line, settings.separator); + let fields = tokenize(self.line, settings.separator); for selector in settings.selectors.iter() { - let mut selection = selector.get_range(&self.line, Some(&fields)); + let mut selection = selector.get_range(self.line, Some(&fields)); match selector.settings.mode { SortMode::Numeric | SortMode::HumanNumeric => { // find out which range is used for numeric comparisons @@ -756,7 +754,7 @@ impl FieldSelector { /// Get the selection that corresponds to this selector for the line. /// If needs_fields returned false, tokens may be None. fn get_selection<'a>(&self, line: &'a str, tokens: Option<&[Field]>) -> Selection<'a> { - let mut range = &line[self.get_range(&line, tokens)]; + let mut range = &line[self.get_range(line, tokens)]; let num_cache = if self.settings.mode == SortMode::Numeric || self.settings.mode == SortMode::HumanNumeric { @@ -846,7 +844,7 @@ impl FieldSelector { match resolve_index(line, tokens, &self.from) { Resolution::StartOfChar(from) => { - let to = self.to.as_ref().map(|to| resolve_index(line, tokens, &to)); + let to = self.to.as_ref().map(|to| resolve_index(line, tokens, to)); let mut range = match to { Some(Resolution::StartOfChar(mut to)) => { @@ -1176,8 +1174,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.buffer_size = matches .value_of(OPT_BUF_SIZE) - .map(GlobalSettings::parse_byte_count) - .unwrap_or(DEFAULT_BUF_SIZE); + .map_or(DEFAULT_BUF_SIZE, |s| { + GlobalSettings::parse_byte_count(s) + .unwrap_or_else(|e| crash!(2, "{}", format_error_message(e, s, OPT_BUF_SIZE))) + }); settings.tmp_dir = matches .value_of(OPT_TMP_DIR) @@ -1257,11 +1257,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { fn output_sorted_lines<'a>(iter: impl Iterator>, settings: &GlobalSettings) { if settings.unique { print_sorted( - iter.dedup_by(|a, b| compare_by(a, b, &settings) == Ordering::Equal), - &settings, + iter.dedup_by(|a, b| compare_by(a, b, settings) == Ordering::Equal), + settings, ); } else { - print_sorted(iter, &settings); + print_sorted(iter, settings); } } @@ -1277,16 +1277,16 @@ fn exec(files: &[String], settings: &GlobalSettings) -> i32 { } else { let mut lines = files.iter().map(open); - ext_sort(&mut lines, &settings); + ext_sort(&mut lines, settings); } 0 } fn sort_by<'a>(unsorted: &mut Vec>, settings: &GlobalSettings) { if settings.stable || settings.unique { - unsorted.par_sort_by(|a, b| compare_by(a, b, &settings)) + unsorted.par_sort_by(|a, b| compare_by(a, b, settings)) } else { - unsorted.par_sort_unstable_by(|a, b| compare_by(a, b, &settings)) + unsorted.par_sort_unstable_by(|a, b| compare_by(a, b, settings)) } } @@ -1587,6 +1587,16 @@ fn open(path: impl AsRef) -> Box { } } +fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String { + // NOTE: + // GNU's sort echos affected flag, -S or --buffer-size, depending user's selection + // GNU's sort does distinguish between "invalid (suffix in) argument" + match error { + ParseSizeError::ParseFailure(_) => format!("invalid --{} argument '{}'", option, s), + ParseSizeError::SizeTooBig(_) => format!("--{} argument '{}' too large", option, s), + } +} + #[cfg(test)] mod tests { @@ -1676,4 +1686,48 @@ mod tests { // How big is a selection? Constant cost all lines pay when we need selections. assert_eq!(std::mem::size_of::(), 24); } + + #[test] + fn test_parse_byte_count() { + let valid_input = [ + ("0", 0), + ("50K", 50 * 1024), + ("50k", 50 * 1024), + ("1M", 1024 * 1024), + ("100M", 100 * 1024 * 1024), + #[cfg(not(target_pointer_width = "32"))] + ("1000G", 1000 * 1024 * 1024 * 1024), + #[cfg(not(target_pointer_width = "32"))] + ("10T", 10 * 1024 * 1024 * 1024 * 1024), + ("1b", 1), + ("1024b", 1024), + ("1024Mb", 1024 * 1024 * 1024), // NOTE: This might not be how GNU `sort` behaves for 'Mb' + ("1", 1024), // K is default + ("50", 50 * 1024), + ("K", 1024), + ("k", 1024), + ("m", 1024 * 1024), + #[cfg(not(target_pointer_width = "32"))] + ("E", 1024 * 1024 * 1024 * 1024 * 1024 * 1024), + ]; + for (input, expected_output) in &valid_input { + assert_eq!( + GlobalSettings::parse_byte_count(input), + Ok(*expected_output) + ); + } + + // SizeTooBig + let invalid_input = ["500E", "1Y"]; + for input in &invalid_input { + #[cfg(not(target_pointer_width = "128"))] + assert!(GlobalSettings::parse_byte_count(input).is_err()); + } + + // ParseFailure + let invalid_input = ["nonsense", "1B", "B", "b", "p", "e", "z", "y"]; + for input in &invalid_input { + assert!(GlobalSettings::parse_byte_count(input).is_err()); + } + } } diff --git a/src/uu/split/src/platform/unix.rs b/src/uu/split/src/platform/unix.rs index 20d9d637b..a115d1959 100644 --- a/src/uu/split/src/platform/unix.rs +++ b/src/uu/split/src/platform/unix.rs @@ -66,7 +66,7 @@ impl FilterWriter { /// * `filepath` - Path of the output file (forwarded to command as $FILE) fn new(command: &str, filepath: &str) -> FilterWriter { // set $FILE, save previous value (if there was one) - let _with_env_var_set = WithEnvVarSet::new("FILE", &filepath); + let _with_env_var_set = WithEnvVarSet::new("FILE", filepath); let shell_process = Command::new(env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_owned())) @@ -117,7 +117,7 @@ pub fn instantiate_current_writer( ) as Box), Some(ref filter_command) => BufWriter::new(Box::new( // spawn a shell command and write to it - FilterWriter::new(&filter_command, &filename), + FilterWriter::new(filter_command, filename), ) as Box), } } diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 6550c35ac..0d5543d8b 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -13,11 +13,13 @@ extern crate uucore; mod platform; use clap::{crate_version, App, Arg}; +use std::convert::TryFrom; use std::env; use std::fs::File; use std::io::{stdin, BufRead, BufReader, BufWriter, Read, Write}; use std::path::Path; use std::{char, fs::remove_file}; +use uucore::parse_size::parse_size; static NAME: &str = "split"; @@ -231,10 +233,9 @@ struct LineSplitter { impl LineSplitter { fn new(settings: &Settings) -> LineSplitter { LineSplitter { - lines_per_split: settings - .strategy_param - .parse() - .unwrap_or_else(|e| crash!(1, "invalid number of lines: {}", e)), + lines_per_split: settings.strategy_param.parse().unwrap_or_else(|_| { + crash!(1, "invalid number of lines: ‘{}’", settings.strategy_param) + }), } } } @@ -276,40 +277,14 @@ struct ByteSplitter { impl ByteSplitter { fn new(settings: &Settings) -> ByteSplitter { - // These multipliers are the same as supported by GNU coreutils. - let modifiers: Vec<(&str, u128)> = vec![ - ("K", 1024u128), - ("M", 1024 * 1024), - ("G", 1024 * 1024 * 1024), - ("T", 1024 * 1024 * 1024 * 1024), - ("P", 1024 * 1024 * 1024 * 1024 * 1024), - ("E", 1024 * 1024 * 1024 * 1024 * 1024 * 1024), - ("Z", 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024), - ("Y", 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024), - ("KB", 1000), - ("MB", 1000 * 1000), - ("GB", 1000 * 1000 * 1000), - ("TB", 1000 * 1000 * 1000 * 1000), - ("PB", 1000 * 1000 * 1000 * 1000 * 1000), - ("EB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000), - ("ZB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000 * 1000), - ("YB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000 * 1000 * 1000), - ]; - - // This sequential find is acceptable since none of the modifiers are - // suffixes of any other modifiers, a la Huffman codes. - let (suffix, multiplier) = modifiers - .iter() - .find(|(suffix, _)| settings.strategy_param.ends_with(suffix)) - .unwrap_or(&("", 1)); - - // Try to parse the actual numeral. - let n = &settings.strategy_param[0..(settings.strategy_param.len() - suffix.len())] - .parse::() - .unwrap_or_else(|e| crash!(1, "invalid number of bytes: {}", e)); + let size_string = &settings.strategy_param; + let size_num = match parse_size(size_string) { + Ok(n) => n, + Err(e) => crash!(1, "invalid number of bytes: {}", e.to_string()), + }; ByteSplitter { - bytes_per_split: n * multiplier, + bytes_per_split: u128::try_from(size_num).unwrap(), } } } diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index fa070d9b7..4e1d9d2c9 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -477,7 +477,7 @@ impl Stater { Stater::generate_tokens(&Stater::default_format(show_fs, terse, false), use_printf) .unwrap() } else { - Stater::generate_tokens(&format_str, use_printf)? + Stater::generate_tokens(format_str, use_printf)? }; let default_dev_tokens = Stater::generate_tokens(&Stater::default_format(show_fs, terse, true), use_printf) diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index 852fe3ef9..fc0c83ec8 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -19,6 +19,7 @@ use std::path::PathBuf; use std::process::Command; use tempfile::tempdir; use tempfile::TempDir; +use uucore::parse_size::parse_size; use uucore::InvalidEncodingHandling; static ABOUT: &str = @@ -55,7 +56,7 @@ const STDBUF_INJECT: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/libstdbuf enum BufferType { Default, Line, - Size(u64), + Size(usize), } struct ProgramOptions { @@ -69,9 +70,9 @@ impl<'a> TryFrom<&ArgMatches<'a>> for ProgramOptions { fn try_from(matches: &ArgMatches) -> Result { Ok(ProgramOptions { - stdin: check_option(&matches, options::INPUT)?, - stdout: check_option(&matches, options::OUTPUT)?, - stderr: check_option(&matches, options::ERROR)?, + stdin: check_option(matches, options::INPUT)?, + stdout: check_option(matches, options::OUTPUT)?, + stderr: check_option(matches, options::ERROR)?, }) } } @@ -104,41 +105,6 @@ fn preload_strings() -> (&'static str, &'static str) { crash!(1, "Command not supported for this operating system!") } -fn parse_size(size: &str) -> Option { - let ext = size.trim_start_matches(|c: char| c.is_digit(10)); - let num = size.trim_end_matches(char::is_alphabetic); - let mut recovered = num.to_owned(); - recovered.push_str(ext); - if recovered != size { - return None; - } - let buf_size: u64 = match num.parse().ok() { - Some(m) => m, - None => return None, - }; - let (power, base): (u32, u64) = match ext { - "" => (0, 0), - "KB" => (1, 1024), - "K" => (1, 1000), - "MB" => (2, 1024), - "M" => (2, 1000), - "GB" => (3, 1024), - "G" => (3, 1000), - "TB" => (4, 1024), - "T" => (4, 1000), - "PB" => (5, 1024), - "P" => (5, 1000), - "EB" => (6, 1024), - "E" => (6, 1000), - "ZB" => (7, 1024), - "Z" => (7, 1000), - "YB" => (8, 1024), - "Y" => (8, 1000), - _ => return None, - }; - Some(buf_size * base.pow(power)) -} - fn check_option(matches: &ArgMatches, name: &str) -> Result { match matches.value_of(name) { Some(value) => match value { @@ -151,13 +117,10 @@ fn check_option(matches: &ArgMatches, name: &str) -> Result { - let size = match parse_size(x) { - Some(m) => m, - None => return Err(ProgramOptionsError(format!("invalid mode {}", x))), - }; - Ok(BufferType::Size(size)) - } + x => parse_size(x).map_or_else( + |e| crash!(125, "invalid mode {}", e), + |m| Ok(BufferType::Size(m)), + ), }, None => Ok(BufferType::Default), } diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index 15a819d35..8950886a2 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -5,7 +5,6 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// * // spell-checker:ignore (ToDO) seekable seek'd tail'ing ringbuffer ringbuf @@ -21,19 +20,18 @@ use chunks::ReverseChunks; use clap::{App, Arg}; use std::collections::VecDeque; -use std::error::Error; use std::fmt; use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, Read, Seek, SeekFrom, Write}; use std::path::Path; use std::thread::sleep; use std::time::Duration; +use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::ringbuffer::RingBuffer; pub mod options { pub mod verbosity { pub static QUIET: &str = "quiet"; - pub static SILENT: &str = "silent"; pub static VERBOSE: &str = "verbose"; } pub static BYTES: &str = "bytes"; @@ -42,13 +40,12 @@ pub mod options { pub static PID: &str = "pid"; pub static SLEEP_INT: &str = "sleep-interval"; pub static ZERO_TERM: &str = "zero-terminated"; + pub static ARG_FILES: &str = "files"; } -static ARG_FILES: &str = "files"; - enum FilterMode { - Bytes(u64), - Lines(u64, u8), // (number of lines, delimiter) + Bytes(usize), + Lines(usize, u8), // (number of lines, delimiter) } struct Settings { @@ -78,12 +75,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let app = App::new(executable!()) .version(crate_version!()) .about("output the last part of files") + // TODO: add usage .arg( Arg::with_name(options::BYTES) .short("c") .long(options::BYTES) .takes_value(true) .allow_hyphen_values(true) + .overrides_with_all(&[options::BYTES, options::LINES]) .help("Number of bytes to print"), ) .arg( @@ -98,6 +97,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::LINES) .takes_value(true) .allow_hyphen_values(true) + .overrides_with_all(&[options::BYTES, options::LINES]) .help("Number of lines to print"), ) .arg( @@ -110,13 +110,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::verbosity::QUIET) .short("q") .long(options::verbosity::QUIET) + .visible_alias("silent") + .overrides_with_all(&[options::verbosity::QUIET, options::verbosity::VERBOSE]) .help("never output headers giving file names"), ) - .arg( - Arg::with_name(options::verbosity::SILENT) - .long(options::verbosity::SILENT) - .help("synonym of --quiet"), - ) .arg( Arg::with_name(options::SLEEP_INT) .short("s") @@ -128,6 +125,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::verbosity::VERBOSE) .short("v") .long(options::verbosity::VERBOSE) + .overrides_with_all(&[options::verbosity::QUIET, options::verbosity::VERBOSE]) .help("always output headers giving file names"), ) .arg( @@ -137,7 +135,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("Line delimiter is NUL, not newline"), ) .arg( - Arg::with_name(ARG_FILES) + Arg::with_name(options::ARG_FILES) .multiple(true) .takes_value(true) .min_values(1), @@ -171,38 +169,21 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } - match matches.value_of(options::LINES) { - Some(n) => { - let mut slice: &str = n; - if slice.chars().next().unwrap_or('_') == '+' { - settings.beginning = true; - slice = &slice[1..]; - } - match parse_size(slice) { - Ok(m) => settings.mode = FilterMode::Lines(m, b'\n'), - Err(e) => { - show_error!("{}", e.to_string()); - return 1; - } - } + let mode_and_beginning = if let Some(arg) = matches.value_of(options::BYTES) { + match parse_num(arg) { + Ok((n, beginning)) => (FilterMode::Bytes(n), beginning), + Err(e) => crash!(1, "invalid number of bytes: {}", e.to_string()), } - None => { - if let Some(n) = matches.value_of(options::BYTES) { - let mut slice: &str = n; - if slice.chars().next().unwrap_or('_') == '+' { - settings.beginning = true; - slice = &slice[1..]; - } - match parse_size(slice) { - Ok(m) => settings.mode = FilterMode::Bytes(m), - Err(e) => { - show_error!("{}", e.to_string()); - return 1; - } - } - } + } else if let Some(arg) = matches.value_of(options::LINES) { + match parse_num(arg) { + Ok((n, beginning)) => (FilterMode::Lines(n, b'\n'), beginning), + Err(e) => crash!(1, "invalid number of lines: {}", e.to_string()), } + } else { + (FilterMode::Lines(10, b'\n'), false) }; + settings.mode = mode_and_beginning.0; + settings.beginning = mode_and_beginning.1; if matches.is_present(options::ZERO_TERM) { if let FilterMode::Lines(count, _) = settings.mode { @@ -211,11 +192,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } let verbose = matches.is_present(options::verbosity::VERBOSE); - let quiet = matches.is_present(options::verbosity::QUIET) - || matches.is_present(options::verbosity::SILENT); + let quiet = matches.is_present(options::verbosity::QUIET); let files: Vec = matches - .values_of(ARG_FILES) + .values_of(options::ARG_FILES) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); @@ -264,98 +244,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { 0 } -#[derive(Debug, PartialEq, Eq)] -pub enum ParseSizeErr { - ParseFailure(String), - SizeTooBig(String), -} - -impl Error for ParseSizeErr { - fn description(&self) -> &str { - match *self { - ParseSizeErr::ParseFailure(ref s) => &*s, - ParseSizeErr::SizeTooBig(ref s) => &*s, - } - } -} - -impl fmt::Display for ParseSizeErr { - fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - let s = match self { - ParseSizeErr::ParseFailure(s) => s, - ParseSizeErr::SizeTooBig(s) => s, - }; - write!(f, "{}", s) - } -} - -impl ParseSizeErr { - fn parse_failure(s: &str) -> ParseSizeErr { - ParseSizeErr::ParseFailure(format!("invalid size: '{}'", s)) - } - - fn size_too_big(s: &str) -> ParseSizeErr { - ParseSizeErr::SizeTooBig(format!( - "invalid size: '{}': Value too large to be stored in data type", - s - )) - } -} - -pub type ParseSizeResult = Result; - -pub fn parse_size(mut size_slice: &str) -> Result { - let mut base = if size_slice.chars().last().unwrap_or('_') == 'B' { - size_slice = &size_slice[..size_slice.len() - 1]; - 1000u64 - } else { - 1024u64 - }; - - let exponent = if !size_slice.is_empty() { - let mut has_suffix = true; - let exp = match size_slice.chars().last().unwrap_or('_') { - 'K' | 'k' => 1u64, - 'M' => 2u64, - 'G' => 3u64, - 'T' => 4u64, - 'P' => 5u64, - 'E' => 6u64, - 'Z' | 'Y' => { - return Err(ParseSizeErr::size_too_big(size_slice)); - } - 'b' => { - base = 512u64; - 1u64 - } - _ => { - has_suffix = false; - 0u64 - } - }; - if has_suffix { - size_slice = &size_slice[..size_slice.len() - 1]; - } - exp - } else { - 0u64 - }; - - let mut multiplier = 1u64; - for _ in 0u64..exponent { - multiplier *= base; - } - if base == 1000u64 && exponent == 0u64 { - // sole B is not a valid suffix - Err(ParseSizeErr::parse_failure(size_slice)) - } else { - let value: Option = size_slice.parse().ok(); - value - .map(|v| Ok((multiplier as i64 * v.abs()) as u64)) - .unwrap_or_else(|| Err(ParseSizeErr::parse_failure(size_slice))) - } -} - fn follow(readers: &mut [BufReader], filenames: &[String], settings: &Settings) { assert!(settings.follow); let mut last = readers.len() - 1; @@ -469,7 +357,7 @@ fn bounded_tail(file: &mut File, settings: &Settings) { /// If any element of `iter` is an [`Err`], then this function panics. fn unbounded_tail_collect( iter: impl Iterator>, - count: u64, + count: usize, beginning: bool, ) -> VecDeque where @@ -514,3 +402,22 @@ fn print_byte(stdout: &mut T, ch: u8) { crash!(1, "{}", err); } } + +fn parse_num(src: &str) -> Result<(usize, bool), ParseSizeError> { + let mut size_string = src.trim(); + let mut starting_with = false; + + if let Some(c) = size_string.chars().next() { + if c == '+' || c == '-' { + // tail: '-' is not documented (8.32 man pages) + size_string = &size_string[1..]; + if c == '+' { + starting_with = true; + } + } + } else { + return Err(ParseSizeError::ParseFailure(src.to_string())); + } + + parse_size(size_string).map(|n| (n, starting_with)) +} diff --git a/src/uu/timeout/Cargo.toml b/src/uu/timeout/Cargo.toml index 206a98c08..a46b029bd 100644 --- a/src/uu/timeout/Cargo.toml +++ b/src/uu/timeout/Cargo.toml @@ -16,9 +16,8 @@ path = "src/timeout.rs" [dependencies] clap = "2.33" -getopts = "0.2.18" libc = "0.2.42" -uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["parse_time", "process", "signals"] } +uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["process", "signals"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 4ef9b2331..afe560ee5 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -55,7 +55,7 @@ impl Config { fn from(options: clap::ArgMatches) -> Config { let signal = match options.value_of(options::SIGNAL) { Some(signal_) => { - let signal_result = signal_by_name_or_value(&signal_); + let signal_result = signal_by_name_or_value(signal_); match signal_result { None => { unreachable!("invalid signal '{}'", signal_); @@ -67,7 +67,7 @@ impl Config { }; let kill_after: Duration = match options.value_of(options::KILL_AFTER) { - Some(time) => uucore::parse_time::from_str(&time).unwrap(), + Some(time) => uucore::parse_time::from_str(time).unwrap(), None => Duration::new(0, 0), }; diff --git a/src/uu/tr/src/expand.rs b/src/uu/tr/src/expand.rs index 7d0c61c30..5d960921e 100644 --- a/src/uu/tr/src/expand.rs +++ b/src/uu/tr/src/expand.rs @@ -22,14 +22,15 @@ use std::ops::RangeInclusive; /// character; octal escape sequences consume 1 to 3 octal digits. #[inline] fn parse_sequence(s: &str) -> (char, usize) { - let c = s.chars().next().expect("invalid escape: empty string"); + let mut s = s.chars(); + let c = s.next().expect("invalid escape: empty string"); if ('0'..='7').contains(&c) { let mut v = c.to_digit(8).unwrap(); let mut consumed = 1; let bits_per_digit = 3; - for c in s.chars().skip(1).take(2) { + for c in s.take(2) { match c.to_digit(8) { Some(c) => { v = (v << bits_per_digit) | c; diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 8e785ad21..f81a95ab2 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -11,19 +11,21 @@ extern crate uucore; use clap::{crate_version, App, Arg}; +use std::convert::TryFrom; use std::fs::{metadata, OpenOptions}; use std::io::ErrorKind; use std::path::Path; +use uucore::parse_size::{parse_size, ParseSizeError}; #[derive(Debug, Eq, PartialEq)] enum TruncateMode { - Absolute(u64), - Extend(u64), - Reduce(u64), - AtMost(u64), - AtLeast(u64), - RoundDown(u64), - RoundUp(u64), + Absolute(usize), + Extend(usize), + Reduce(usize), + AtMost(usize), + AtLeast(usize), + RoundDown(usize), + RoundUp(usize), } impl TruncateMode { @@ -38,7 +40,7 @@ impl TruncateMode { /// let fsize = 10; /// assert_eq!(mode.to_size(fsize), 15); /// ``` - fn to_size(&self, fsize: u64) -> u64 { + fn to_size(&self, fsize: usize) -> usize { match self { TruncateMode::Absolute(size) => *size, TruncateMode::Extend(size) => fsize + size, @@ -58,10 +60,9 @@ pub mod options { pub static NO_CREATE: &str = "no-create"; pub static REFERENCE: &str = "reference"; pub static SIZE: &str = "size"; + pub static ARG_FILES: &str = "files"; } -static ARG_FILES: &str = "files"; - fn get_usage() -> String { format!("{0} [OPTION]... [FILE]...", executable!()) } @@ -113,21 +114,28 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::REFERENCE) .short("r") .long(options::REFERENCE) + .required_unless(options::SIZE) .help("base the size of each file on the size of RFILE") .value_name("RFILE") ) .arg( Arg::with_name(options::SIZE) .short("s") - .long("size") + .long(options::SIZE) + .required_unless(options::REFERENCE) .help("set or adjust the size of each file according to SIZE, which is in bytes unless --io-blocks is specified") .value_name("SIZE") ) - .arg(Arg::with_name(ARG_FILES).multiple(true).takes_value(true).min_values(1)) + .arg(Arg::with_name(options::ARG_FILES) + .value_name("FILE") + .multiple(true) + .takes_value(true) + .required(true) + .min_values(1)) .get_matches_from(args); let files: Vec = matches - .values_of(ARG_FILES) + .values_of(options::ARG_FILES) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); @@ -149,8 +157,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { crash!( 1, "cannot stat '{}': No such file or directory", - reference.unwrap() - ); + reference.unwrap_or_else(|| "".to_string()) + ); // TODO: fix '--no-create' see test_reference and test_truncate_bytes_size } _ => crash!(1, "{}", e.to_string()), } @@ -172,10 +180,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { /// /// If the file could not be opened, or there was a problem setting the /// size of the file. -fn file_truncate(filename: &str, create: bool, size: u64) -> std::io::Result<()> { +fn file_truncate(filename: &str, create: bool, size: usize) -> std::io::Result<()> { let path = Path::new(filename); let f = OpenOptions::new().write(true).create(create).open(path)?; - f.set_len(size) + f.set_len(u64::try_from(size).unwrap()) } /// Truncate files to a size relative to a given file. @@ -206,9 +214,9 @@ fn truncate_reference_and_size( } _ => m, }, - Err(_) => crash!(1, "Invalid number: ‘{}’", size_string), + Err(e) => crash!(1, "Invalid number: {}", e.to_string()), }; - let fsize = metadata(rfilename)?.len(); + let fsize = usize::try_from(metadata(rfilename)?.len()).unwrap(); let tsize = mode.to_size(fsize); for filename in &filenames { file_truncate(filename, create, tsize)?; @@ -232,7 +240,7 @@ fn truncate_reference_file_only( filenames: Vec, create: bool, ) -> std::io::Result<()> { - let tsize = metadata(rfilename)?.len(); + let tsize = usize::try_from(metadata(rfilename)?.len()).unwrap(); for filename in &filenames { file_truncate(filename, create, tsize)?; } @@ -261,10 +269,10 @@ fn truncate_size_only( ) -> std::io::Result<()> { let mode = match parse_mode_and_size(size_string) { Ok(m) => m, - Err(_) => crash!(1, "Invalid number: ‘{}’", size_string), + Err(e) => crash!(1, "Invalid number: {}", e.to_string()), }; for filename in &filenames { - let fsize = metadata(filename).map(|m| m.len()).unwrap_or(0); + let fsize = usize::try_from(metadata(filename)?.len()).unwrap(); let tsize = mode.to_size(fsize); file_truncate(filename, create, tsize)?; } @@ -290,7 +298,7 @@ fn truncate( } (Some(rfilename), None) => truncate_reference_file_only(&rfilename, filenames, create), (None, Some(size_string)) => truncate_size_only(&size_string, filenames, create), - (None, None) => crash!(1, "you must specify either --reference or --size"), + (None, None) => crash!(1, "you must specify either --reference or --size"), // this case cannot happen anymore because it's handled by clap } } @@ -317,117 +325,35 @@ fn is_modifier(c: char) -> bool { /// ```rust,ignore /// assert_eq!(parse_mode_and_size("+123"), (TruncateMode::Extend, 123)); /// ``` -fn parse_mode_and_size(size_string: &str) -> Result { +fn parse_mode_and_size(size_string: &str) -> Result { // Trim any whitespace. - let size_string = size_string.trim(); + let mut size_string = size_string.trim(); // Get the modifier character from the size string, if any. For // example, if the argument is "+123", then the modifier is '+'. - let c = size_string.chars().next().unwrap(); - let size_string = if is_modifier(c) { - &size_string[1..] + if let Some(c) = size_string.chars().next() { + if is_modifier(c) { + size_string = &size_string[1..]; + } + parse_size(size_string).map(match c { + '+' => TruncateMode::Extend, + '-' => TruncateMode::Reduce, + '<' => TruncateMode::AtMost, + '>' => TruncateMode::AtLeast, + '/' => TruncateMode::RoundDown, + '%' => TruncateMode::RoundUp, + _ => TruncateMode::Absolute, + }) } else { - size_string - }; - parse_size(size_string).map(match c { - '+' => TruncateMode::Extend, - '-' => TruncateMode::Reduce, - '<' => TruncateMode::AtMost, - '>' => TruncateMode::AtLeast, - '/' => TruncateMode::RoundDown, - '%' => TruncateMode::RoundUp, - _ => TruncateMode::Absolute, - }) -} - -/// Parse a size string into a number of bytes. -/// -/// A size string comprises an integer and an optional unit. The unit -/// may be K, M, G, T, P, E, Z, or Y (powers of 1024) or KB, MB, -/// etc. (powers of 1000). -/// -/// # Errors -/// -/// This function returns an error if the string does not begin with a -/// numeral, or if the unit is not one of the supported units described -/// in the preceding section. -/// -/// # Examples -/// -/// ```rust,ignore -/// assert_eq!(parse_size("123").unwrap(), 123); -/// assert_eq!(parse_size("123K").unwrap(), 123 * 1024); -/// assert_eq!(parse_size("123KB").unwrap(), 123 * 1000); -/// ``` -fn parse_size(size: &str) -> Result { - // Get the numeric part of the size argument. For example, if the - // argument is "123K", then the numeric part is "123". - let numeric_string: String = size.chars().take_while(|c| c.is_digit(10)).collect(); - let number: u64 = match numeric_string.parse() { - Ok(n) => n, - Err(_) => return Err(()), - }; - - // Get the alphabetic units part of the size argument and compute - // the factor it represents. For example, if the argument is "123K", - // then the unit part is "K" and the factor is 1024. This may be the - // empty string, in which case, the factor is 1. - let n = numeric_string.len(); - let (base, exponent): (u64, u32) = match &size[n..] { - "" => (1, 0), - "K" | "k" => (1024, 1), - "M" | "m" => (1024, 2), - "G" | "g" => (1024, 3), - "T" | "t" => (1024, 4), - "P" | "p" => (1024, 5), - "E" | "e" => (1024, 6), - "Z" | "z" => (1024, 7), - "Y" | "y" => (1024, 8), - "KB" | "kB" => (1000, 1), - "MB" | "mB" => (1000, 2), - "GB" | "gB" => (1000, 3), - "TB" | "tB" => (1000, 4), - "PB" | "pB" => (1000, 5), - "EB" | "eB" => (1000, 6), - "ZB" | "zB" => (1000, 7), - "YB" | "yB" => (1000, 8), - _ => return Err(()), - }; - let factor = base.pow(exponent); - Ok(number * factor) + Err(ParseSizeError::ParseFailure(size_string.to_string())) + } } #[cfg(test)] mod tests { use crate::parse_mode_and_size; - use crate::parse_size; use crate::TruncateMode; - #[test] - fn test_parse_size_zero() { - assert_eq!(parse_size("0").unwrap(), 0); - assert_eq!(parse_size("0K").unwrap(), 0); - assert_eq!(parse_size("0KB").unwrap(), 0); - } - - #[test] - fn test_parse_size_without_factor() { - assert_eq!(parse_size("123").unwrap(), 123); - } - - #[test] - fn test_parse_size_kilobytes() { - assert_eq!(parse_size("123K").unwrap(), 123 * 1024); - assert_eq!(parse_size("123KB").unwrap(), 123 * 1000); - } - - #[test] - fn test_parse_size_megabytes() { - assert_eq!(parse_size("123").unwrap(), 123); - assert_eq!(parse_size("123M").unwrap(), 123 * 1024 * 1024); - assert_eq!(parse_size("123MB").unwrap(), 123 * 1000 * 1000); - } - #[test] fn test_parse_mode_and_size() { assert_eq!(parse_mode_and_size("10"), Ok(TruncateMode::Absolute(10))); diff --git a/src/uu/tty/Cargo.toml b/src/uu/tty/Cargo.toml index 7be27a900..49b7669df 100644 --- a/src/uu/tty/Cargo.toml +++ b/src/uu/tty/Cargo.toml @@ -17,6 +17,7 @@ path = "src/tty.rs" [dependencies] clap = "2.33" libc = "0.2.42" +atty = "0.2" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/tty/src/tty.rs b/src/uu/tty/src/tty.rs index 074bcf182..edcdf091e 100644 --- a/src/uu/tty/src/tty.rs +++ b/src/uu/tty/src/tty.rs @@ -14,7 +14,6 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use std::ffi::CStr; -use uucore::fs::is_stdin_interactive; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Print the file name of the terminal connected to standard input."; @@ -67,7 +66,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } - if is_stdin_interactive() { + if atty::is(atty::Stream::Stdin) { libc::EXIT_SUCCESS } else { libc::EXIT_FAILURE diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index 031c25739..d1e1f75ca 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -374,8 +374,8 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { let num_inputs = inputs.len(); for input in &inputs { - let word_count = word_count_from_input(&input, settings).unwrap_or_else(|err| { - show_error(&input, err); + let word_count = word_count_from_input(input, settings).unwrap_or_else(|err| { + show_error(input, err); error_count += 1; WordCount::default() }); diff --git a/src/uu/who/src/who.rs b/src/uu/who/src/who.rs index d2f64aa94..44f565438 100644 --- a/src/uu/who/src/who.rs +++ b/src/uu/who/src/who.rs @@ -179,124 +179,58 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // Ignored for 'who am i'. let short_list = matches.is_present(options::COUNT); - // If true, display only name, line, and time fields. - let mut short_output = false; - - // If true, display the hours:minutes since each user has touched - // the keyboard, or "." if within the last minute, or "old" if - // not within the last day. - let mut include_idle = false; + let all = matches.is_present(options::ALL); // If true, display a line at the top describing each field. let include_heading = matches.is_present(options::HEADING); // If true, display a '+' for each user if mesg y, a '-' if mesg n, // or a '?' if their tty cannot be statted. - let include_mesg = matches.is_present(options::ALL) - || matches.is_present(options::MESG) - || matches.is_present("w"); - - // If true, display process termination & exit status. - let mut include_exit = false; + let include_mesg = all || matches.is_present(options::MESG) || matches.is_present("w"); // If true, display the last boot time. - let mut need_boottime = false; + let need_boottime = all || matches.is_present(options::BOOT); // If true, display dead processes. - let mut need_deadprocs = false; + let need_deadprocs = all || matches.is_present(options::DEAD); // If true, display processes waiting for user login. - let mut need_login = false; + let need_login = all || matches.is_present(options::LOGIN); // If true, display processes started by init. - let mut need_initspawn = false; + let need_initspawn = all || matches.is_present(options::PROCESS); // If true, display the last clock change. - let mut need_clockchange = false; + let need_clockchange = all || matches.is_present(options::TIME); // If true, display the current runlevel. - let mut need_runlevel = false; + let need_runlevel = all || matches.is_present(options::RUNLEVEL); + + let use_defaults = !(all + || need_boottime + || need_deadprocs + || need_login + || need_initspawn + || need_runlevel + || need_clockchange + || matches.is_present(options::USERS)); // If true, display user processes. - let mut need_users = false; + let need_users = all || matches.is_present(options::USERS) || use_defaults; + + // If true, display the hours:minutes since each user has touched + // the keyboard, or "." if within the last minute, or "old" if + // not within the last day. + let include_idle = need_deadprocs || need_login || need_runlevel || need_users; + + // If true, display process termination & exit status. + let include_exit = need_deadprocs; + + // If true, display only name, line, and time fields. + let short_output = !include_exit && use_defaults; // If true, display info only for the controlling tty. - let mut my_line_only = false; - - let mut assumptions = true; - - #[allow(clippy::useless_let_if_seq)] - { - if matches.is_present(options::ALL) { - need_boottime = true; - need_deadprocs = true; - need_login = true; - need_initspawn = true; - need_runlevel = true; - need_clockchange = true; - need_users = true; - include_idle = true; - include_exit = true; - assumptions = false; - } - - if matches.is_present(options::BOOT) { - need_boottime = true; - assumptions = false; - } - - if matches.is_present(options::DEAD) { - need_deadprocs = true; - include_idle = true; - include_exit = true; - assumptions = false; - } - - if matches.is_present(options::LOGIN) { - need_login = true; - include_idle = true; - assumptions = false; - } - - if matches.is_present(options::ONLY_HOSTNAME_USER) || files.len() == 2 { - my_line_only = true; - } - - if matches.is_present(options::PROCESS) { - need_initspawn = true; - assumptions = false; - } - - if matches.is_present(options::RUNLEVEL) { - need_runlevel = true; - include_idle = true; - assumptions = false; - } - - if matches.is_present(options::SHORT) { - short_output = true; - } - - if matches.is_present(options::TIME) { - need_clockchange = true; - assumptions = false; - } - - if matches.is_present(options::USERS) { - need_users = true; - include_idle = true; - assumptions = false; - } - - if assumptions { - need_users = true; - short_output = true; - } - - if include_exit { - short_output = false; - } - } + let my_line_only = matches.is_present(options::ONLY_HOSTNAME_USER) || files.len() == 2; let mut who = Who { do_lookup, diff --git a/src/uu/whoami/Cargo.toml b/src/uu/whoami/Cargo.toml index f8dc01440..28670c8b5 100644 --- a/src/uu/whoami/Cargo.toml +++ b/src/uu/whoami/Cargo.toml @@ -20,7 +20,6 @@ uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=[" uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } [target.'cfg(target_os = "windows")'.dependencies] -advapi32-sys = "0.2.0" winapi = { version = "0.3", features = ["lmcons"] } [[bin]] diff --git a/src/uu/whoami/src/platform/windows.rs b/src/uu/whoami/src/platform/windows.rs index 5d648877b..3fe8eb1e7 100644 --- a/src/uu/whoami/src/platform/windows.rs +++ b/src/uu/whoami/src/platform/windows.rs @@ -11,7 +11,7 @@ extern crate winapi; use self::winapi::shared::lmcons; use self::winapi::shared::minwindef; -use self::winapi::um::winnt; +use self::winapi::um::{winbase, winnt}; use std::io::{Error, Result}; use std::mem; use uucore::wide::FromWide; @@ -20,7 +20,7 @@ pub unsafe fn get_username() -> Result { #[allow(deprecated)] let mut buffer: [winnt::WCHAR; lmcons::UNLEN as usize + 1] = mem::uninitialized(); let mut len = buffer.len() as minwindef::DWORD; - if advapi32::GetUserNameW(buffer.as_mut_ptr(), &mut len) == 0 { + if winbase::GetUserNameW(buffer.as_mut_ptr(), &mut len) == 0 { return Err(Error::last_os_error()); } let username = String::from_wide(&buffer[..len as usize - 1]); diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 482252680..0c11d2c15 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -44,7 +44,6 @@ entries = ["libc"] fs = ["libc"] fsext = ["libc", "time"] mode = ["libc"] -parse_time = [] perms = ["libc"] process = ["libc"] ringbuffer = [] diff --git a/src/uucore/src/lib/features.rs b/src/uucore/src/lib/features.rs index 310a41fe1..c1e1ec31e 100644 --- a/src/uucore/src/lib/features.rs +++ b/src/uucore/src/lib/features.rs @@ -6,8 +6,6 @@ pub mod encoding; pub mod fs; #[cfg(feature = "fsext")] pub mod fsext; -#[cfg(feature = "parse_time")] -pub mod parse_time; #[cfg(feature = "ringbuffer")] pub mod ringbuffer; #[cfg(feature = "zero-copy")] diff --git a/src/uucore/src/lib/features/entries.rs b/src/uucore/src/lib/features/entries.rs index 2d5b44362..d814e049b 100644 --- a/src/uucore/src/lib/features/entries.rs +++ b/src/uucore/src/lib/features/entries.rs @@ -94,16 +94,21 @@ pub fn get_groups() -> IOResult> { /// groups is the same (in the mathematical sense of ``set''). (The /// history of a process and its parents could affect the details of /// the result.) +#[cfg(all(unix, feature = "process"))] pub fn get_groups_gnu(arg_id: Option) -> IOResult> { - let mut groups = get_groups()?; + let groups = get_groups()?; let egid = arg_id.unwrap_or_else(crate::features::process::getegid); - if !groups.is_empty() && *groups.first().unwrap() == egid { - return Ok(groups); - } else if let Some(index) = groups.iter().position(|&x| x == egid) { - groups.remove(index); + Ok(sort_groups(groups, egid)) +} + +#[cfg(all(unix, feature = "process"))] +fn sort_groups(mut groups: Vec, egid: gid_t) -> Vec { + if let Some(index) = groups.iter().position(|&x| x == egid) { + groups[..=index].rotate_right(1); + } else { + groups.insert(0, egid); } - groups.insert(0, egid); - Ok(groups) + groups } #[derive(Copy, Clone)] @@ -315,6 +320,15 @@ pub fn grp2gid(name: &str) -> IOResult { mod test { use super::*; + #[test] + fn test_sort_groups() { + assert_eq!(sort_groups(vec![1, 2, 3], 4), vec![4, 1, 2, 3]); + assert_eq!(sort_groups(vec![1, 2, 3], 3), vec![3, 1, 2]); + assert_eq!(sort_groups(vec![1, 2, 3], 2), vec![2, 1, 3]); + assert_eq!(sort_groups(vec![1, 2, 3], 1), vec![1, 2, 3]); + assert_eq!(sort_groups(vec![1, 2, 3], 0), vec![0, 1, 2, 3]); + } + #[test] fn test_entries_get_groups_gnu() { if let Ok(mut groups) = get_groups() { diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 38cdbef94..36bdbfed0 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -113,22 +113,14 @@ fn resolve>(original: P) -> IOResult { )); } - match fs::symlink_metadata(&result) { - Err(e) => return Err(e), - Ok(ref m) if !m.file_type().is_symlink() => break, - Ok(..) => { - followed += 1; - match fs::read_link(&result) { - Ok(path) => { - result.pop(); - result.push(path); - } - Err(e) => { - return Err(e); - } - } - } + if !fs::symlink_metadata(&result)?.file_type().is_symlink() { + break; } + + followed += 1; + let path = fs::read_link(&result)?; + result.pop(); + result.push(path); } Ok(result) } @@ -193,10 +185,8 @@ pub fn canonicalize>(original: P, can_mode: CanonicalizeMode) -> } match resolve(&result) { - Err(e) => match can_mode { - CanonicalizeMode::Missing => continue, - _ => return Err(e), - }, + Err(_) if can_mode == CanonicalizeMode::Missing => continue, + Err(e) => return Err(e), Ok(path) => { result.pop(); result.push(path); @@ -211,65 +201,19 @@ pub fn canonicalize>(original: P, can_mode: CanonicalizeMode) -> } match resolve(&result) { - Err(e) => { - if can_mode == CanonicalizeMode::Existing { - return Err(e); - } + Err(e) if can_mode == CanonicalizeMode::Existing => { + return Err(e); } Ok(path) => { result.pop(); result.push(path); } + Err(_) => (), } } Ok(result) } -#[cfg(unix)] -pub fn is_stdin_interactive() -> bool { - unsafe { libc::isatty(libc::STDIN_FILENO) == 1 } -} - -#[cfg(windows)] -pub fn is_stdin_interactive() -> bool { - false -} - -#[cfg(target_os = "redox")] -pub fn is_stdin_interactive() -> bool { - termion::is_tty(&io::stdin()) -} - -#[cfg(unix)] -pub fn is_stdout_interactive() -> bool { - unsafe { libc::isatty(libc::STDOUT_FILENO) == 1 } -} - -#[cfg(windows)] -pub fn is_stdout_interactive() -> bool { - false -} - -#[cfg(target_os = "redox")] -pub fn is_stdout_interactive() -> bool { - termion::is_tty(&io::stdout()) -} - -#[cfg(unix)] -pub fn is_stderr_interactive() -> bool { - unsafe { libc::isatty(libc::STDERR_FILENO) == 1 } -} - -#[cfg(windows)] -pub fn is_stderr_interactive() -> bool { - false -} - -#[cfg(target_os = "redox")] -pub fn is_stderr_interactive() -> bool { - termion::is_tty(&io::stderr()) -} - #[cfg(not(unix))] #[allow(unused_variables)] pub fn display_permissions(metadata: &fs::Metadata, display_file_type: bool) -> String { diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 4fb5a6509..fe109d73d 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -89,19 +89,19 @@ fn parse_levels(mode: &str) -> (u32, usize) { } fn parse_op(mode: &str, default: Option) -> Result<(char, usize), String> { - match mode.chars().next() { - Some(ch) => match ch { - '+' | '-' | '=' => Ok((ch, 1)), - _ => match default { - Some(ch) => Ok((ch, 0)), - None => Err(format!( - "invalid operator (expected +, -, or =, but found {})", - ch - )), - }, - }, - None => Err("unexpected end of mode".to_owned()), - } + let ch = mode + .chars() + .next() + .ok_or_else(|| "unexpected end of mode".to_owned())?; + Ok(match ch { + '+' | '-' | '=' => (ch, 1), + _ => { + let ch = default.ok_or_else(|| { + format!("invalid operator (expected +, -, or =, but found {})", ch) + })?; + (ch, 0) + } + }) } fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) { diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index eb6cca102..89c30b53b 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -92,7 +92,7 @@ pub fn wrap_chgrp>( out = format!( "group of '{}' retained as {}", path.display(), - entries::gid2grp(dest_gid).unwrap() + entries::gid2grp(dest_gid).unwrap_or_default() ); } } diff --git a/src/uucore/src/lib/features/utmpx.rs b/src/uucore/src/lib/features/utmpx.rs index a794b01da..5077d9e59 100644 --- a/src/uucore/src/lib/features/utmpx.rs +++ b/src/uucore/src/lib/features/utmpx.rs @@ -207,7 +207,7 @@ impl Utmpx { flags: AI_CANONNAME, ..AddrInfoHints::default() }; - let sockets = getaddrinfo(Some(&hostname), None, Some(hints)) + let sockets = getaddrinfo(Some(hostname), None, Some(hints)) .unwrap() .collect::>>()?; for socket in sockets { diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 0b0d0fddf..f765b7b3e 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -19,10 +19,10 @@ pub extern crate winapi; //## internal modules -mod macros; // crate macros (macro_rules-type; exported to `crate::...`) - mod features; // feature-gated code modules +mod macros; // crate macros (macro_rules-type; exported to `crate::...`) mod mods; // core cross-platform modules +mod parser; // string parsing modules // * cross-platform modules pub use crate::mods::backup_control; @@ -31,6 +31,10 @@ pub use crate::mods::os; pub use crate::mods::panic; pub use crate::mods::ranges; +// * string parsing modules +pub use crate::parser::parse_size; +pub use crate::parser::parse_time; + // * feature-gated modules #[cfg(feature = "encoding")] pub use crate::features::encoding; @@ -38,8 +42,6 @@ pub use crate::features::encoding; pub use crate::features::fs; #[cfg(feature = "fsext")] pub use crate::features::fsext; -#[cfg(feature = "parse_time")] -pub use crate::features::parse_time; #[cfg(feature = "ringbuffer")] pub use crate::features::ringbuffer; #[cfg(feature = "zero-copy")] diff --git a/src/uucore/src/lib/mods/ranges.rs b/src/uucore/src/lib/mods/ranges.rs index d4a6bf601..9e1e67d5a 100644 --- a/src/uucore/src/lib/mods/ranges.rs +++ b/src/uucore/src/lib/mods/ranges.rs @@ -85,10 +85,9 @@ impl Range { let mut ranges: Vec = vec![]; for item in list.split(',') { - match FromStr::from_str(item) { - Ok(range_item) => ranges.push(range_item), - Err(e) => return Err(format!("range '{}' was invalid: {}", item, e)), - } + let range_item = FromStr::from_str(item) + .map_err(|e| format!("range '{}' was invalid: {}", item, e))?; + ranges.push(range_item); } ranges.sort(); diff --git a/src/uucore/src/lib/parser.rs b/src/uucore/src/lib/parser.rs new file mode 100644 index 000000000..d09777e10 --- /dev/null +++ b/src/uucore/src/lib/parser.rs @@ -0,0 +1,2 @@ +pub mod parse_size; +pub mod parse_time; diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs new file mode 100644 index 000000000..58213adef --- /dev/null +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -0,0 +1,324 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + +// spell-checker:ignore (ToDO) hdsf ghead gtail + +use std::convert::TryFrom; +use std::error::Error; +use std::fmt; + +/// Parse a size string into a number of bytes. +/// +/// A size string comprises an integer and an optional unit. The unit +/// may be K, M, G, T, P, E, Z or Y (powers of 1024), or KB, MB, +/// etc. (powers of 1000), or b which is 512. +/// Binary prefixes can be used, too: KiB=K, MiB=M, and so on. +/// +/// # Errors +/// +/// Will return `ParseSizeError` if it’s not possible to parse this +/// string into a number, e.g. if the string does not begin with a +/// numeral, or if the unit is not one of the supported units described +/// in the preceding section. +/// +/// # Examples +/// +/// ```rust +/// use uucore::parse_size::parse_size; +/// assert_eq!(Ok(123), parse_size("123")); +/// assert_eq!(Ok(9 * 1000), parse_size("9kB")); // kB is 1000 +/// assert_eq!(Ok(2 * 1024), parse_size("2K")); // K is 1024 +/// ``` +pub fn parse_size(size: &str) -> Result { + if size.is_empty() { + return Err(ParseSizeError::parse_failure(size)); + } + // Get the numeric part of the size argument. For example, if the + // argument is "123K", then the numeric part is "123". + let numeric_string: String = size.chars().take_while(|c| c.is_digit(10)).collect(); + let number: usize = if !numeric_string.is_empty() { + match numeric_string.parse() { + Ok(n) => n, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } else { + 1 + }; + + // Get the alphabetic units part of the size argument and compute + // the factor it represents. For example, if the argument is "123K", + // then the unit part is "K" and the factor is 1024. This may be the + // empty string, in which case, the factor is 1. + let unit = &size[numeric_string.len()..]; + let (base, exponent): (u128, u32) = match unit { + "" => (1, 0), + "b" => (512, 1), // (`od`, `head` and `tail` use "b") + "KiB" | "kiB" | "K" | "k" => (1024, 1), + "MiB" | "miB" | "M" | "m" => (1024, 2), + "GiB" | "giB" | "G" | "g" => (1024, 3), + "TiB" | "tiB" | "T" | "t" => (1024, 4), + "PiB" | "piB" | "P" | "p" => (1024, 5), + "EiB" | "eiB" | "E" | "e" => (1024, 6), + "ZiB" | "ziB" | "Z" | "z" => (1024, 7), + "YiB" | "yiB" | "Y" | "y" => (1024, 8), + "KB" | "kB" => (1000, 1), + "MB" | "mB" => (1000, 2), + "GB" | "gB" => (1000, 3), + "TB" | "tB" => (1000, 4), + "PB" | "pB" => (1000, 5), + "EB" | "eB" => (1000, 6), + "ZB" | "zB" => (1000, 7), + "YB" | "yB" => (1000, 8), + _ => return Err(ParseSizeError::parse_failure(size)), + }; + let factor = match usize::try_from(base.pow(exponent)) { + Ok(n) => n, + Err(_) => return Err(ParseSizeError::size_too_big(size)), + }; + number + .checked_mul(factor) + .ok_or_else(|| ParseSizeError::size_too_big(size)) +} + +#[derive(Debug, PartialEq, Eq)] +pub enum ParseSizeError { + ParseFailure(String), // Syntax + SizeTooBig(String), // Overflow +} + +impl Error for ParseSizeError { + fn description(&self) -> &str { + match *self { + ParseSizeError::ParseFailure(ref s) => &*s, + ParseSizeError::SizeTooBig(ref s) => &*s, + } + } +} + +impl fmt::Display for ParseSizeError { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + let s = match self { + ParseSizeError::ParseFailure(s) => s, + ParseSizeError::SizeTooBig(s) => s, + }; + write!(f, "{}", s) + } +} + +impl ParseSizeError { + fn parse_failure(s: &str) -> ParseSizeError { + // stderr on linux (GNU coreutils 8.32) + // has to be handled in the respective uutils because strings differ, e.g.: + // + // `NUM` + // head: invalid number of bytes: ‘1fb’ + // tail: invalid number of bytes: ‘1fb’ + // + // `SIZE` + // split: invalid number of bytes: ‘1fb’ + // truncate: Invalid number: ‘1fb’ + // + // `MODE` + // stdbuf: invalid mode ‘1fb’ + // + // `SIZE` + // sort: invalid suffix in --buffer-size argument '1fb' + // sort: invalid --buffer-size argument 'fb' + // + // `SIZE` + // du: invalid suffix in --buffer-size argument '1fb' + // du: invalid suffix in --threshold argument '1fb' + // du: invalid --buffer-size argument 'fb' + // du: invalid --threshold argument 'fb' + // + // `BYTES` + // od: invalid suffix in --read-bytes argument '1fb' + // od: invalid --read-bytes argument argument 'fb' + // --skip-bytes + // --width + // --strings + // etc. + ParseSizeError::ParseFailure(format!("‘{}’", s)) + } + + fn size_too_big(s: &str) -> ParseSizeError { + // stderr on linux (GNU coreutils 8.32) + // has to be handled in the respective uutils because strings differ, e.g.: + // + // head: invalid number of bytes: ‘1Y’: Value too large for defined data type + // tail: invalid number of bytes: ‘1Y’: Value too large for defined data type + // split: invalid number of bytes: ‘1Y’: Value too large for defined data type + // truncate: Invalid number: ‘1Y’: Value too large for defined data type + // stdbuf: invalid mode ‘1Y’: Value too large for defined data type + // sort: -S argument '1Y' too large + // du: -B argument '1Y' too large + // od: -N argument '1Y' too large + // etc. + // + // stderr on macos (brew - GNU coreutils 8.32) also differs for the same version, e.g.: + // ghead: invalid number of bytes: ‘1Y’: Value too large to be stored in data type + // gtail: invalid number of bytes: ‘1Y’: Value too large to be stored in data type + ParseSizeError::SizeTooBig(format!("‘{}’: Value too large for defined data type", s)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn variant_eq(a: &ParseSizeError, b: &ParseSizeError) -> bool { + std::mem::discriminant(a) == std::mem::discriminant(b) + } + + #[test] + fn all_suffixes() { + // Units are K,M,G,T,P,E,Z,Y (powers of 1024) or KB,MB,... (powers of 1000). + // Binary prefixes can be used, too: KiB=K, MiB=M, and so on. + let suffixes = [ + ('K', 1u32), + ('M', 2u32), + ('G', 3u32), + ('T', 4u32), + ('P', 5u32), + ('E', 6u32), + #[cfg(target_pointer_width = "128")] + ('Z', 7u32), // ParseSizeError::SizeTooBig on x64 + #[cfg(target_pointer_width = "128")] + ('Y', 8u32), // ParseSizeError::SizeTooBig on x64 + ]; + + for &(c, exp) in &suffixes { + let s = format!("2{}B", c); // KB + assert_eq!(Ok((2 * (1000_u128).pow(exp)) as usize), parse_size(&s)); + let s = format!("2{}", c); // K + assert_eq!(Ok((2 * (1024_u128).pow(exp)) as usize), parse_size(&s)); + let s = format!("2{}iB", c); // KiB + assert_eq!(Ok((2 * (1024_u128).pow(exp)) as usize), parse_size(&s)); + let s = format!("2{}iB", c.to_lowercase()); // kiB + assert_eq!(Ok((2 * (1024_u128).pow(exp)) as usize), parse_size(&s)); + + // suffix only + let s = format!("{}B", c); // KB + assert_eq!(Ok(((1000_u128).pow(exp)) as usize), parse_size(&s)); + let s = format!("{}", c); // K + assert_eq!(Ok(((1024_u128).pow(exp)) as usize), parse_size(&s)); + let s = format!("{}iB", c); // KiB + assert_eq!(Ok(((1024_u128).pow(exp)) as usize), parse_size(&s)); + let s = format!("{}iB", c.to_lowercase()); // kiB + assert_eq!(Ok(((1024_u128).pow(exp)) as usize), parse_size(&s)); + } + } + + #[test] + #[cfg(not(target_pointer_width = "128"))] + fn overflow_x64() { + assert!(parse_size("10000000000000000000000").is_err()); + assert!(parse_size("1000000000T").is_err()); + assert!(parse_size("100000P").is_err()); + assert!(parse_size("100E").is_err()); + assert!(parse_size("1Z").is_err()); + assert!(parse_size("1Y").is_err()); + + assert!(variant_eq( + &parse_size("1Z").unwrap_err(), + &ParseSizeError::SizeTooBig(String::new()) + )); + + assert_eq!( + ParseSizeError::SizeTooBig("‘1Y’: Value too large for defined data type".to_string()), + parse_size("1Y").unwrap_err() + ); + } + + #[test] + #[cfg(target_pointer_width = "32")] + fn overflow_x32() { + assert!(variant_eq( + &parse_size("1T").unwrap_err(), + &ParseSizeError::SizeTooBig(String::new()) + )); + assert!(variant_eq( + &parse_size("1000G").unwrap_err(), + &ParseSizeError::SizeTooBig(String::new()) + )); + } + + #[test] + fn invalid_syntax() { + let test_strings = [ + "328hdsf3290", + "5MiB nonsense", + "5mib", + "biB", + "-", + "+", + "", + "-1", + "1e2", + "∞", + ]; + for &test_string in &test_strings { + assert_eq!( + parse_size(test_string).unwrap_err(), + ParseSizeError::ParseFailure(format!("‘{}’", test_string)) + ); + } + } + + #[test] + fn b_suffix() { + assert_eq!(Ok(3 * 512), parse_size("3b")); // b is 512 + } + + #[test] + fn no_suffix() { + assert_eq!(Ok(1234), parse_size("1234")); + assert_eq!(Ok(0), parse_size("0")); + assert_eq!(Ok(5), parse_size("5")); + assert_eq!(Ok(999), parse_size("999")); + } + + #[test] + fn kilobytes_suffix() { + assert_eq!(Ok(123 * 1000), parse_size("123KB")); // KB is 1000 + assert_eq!(Ok(9 * 1000), parse_size("9kB")); // kB is 1000 + assert_eq!(Ok(2 * 1024), parse_size("2K")); // K is 1024 + assert_eq!(Ok(0), parse_size("0K")); + assert_eq!(Ok(0), parse_size("0KB")); + assert_eq!(Ok(1000), parse_size("KB")); + assert_eq!(Ok(1024), parse_size("K")); + assert_eq!(Ok(2000), parse_size("2kB")); + assert_eq!(Ok(4000), parse_size("4KB")); + } + + #[test] + fn megabytes_suffix() { + assert_eq!(Ok(123 * 1024 * 1024), parse_size("123M")); + assert_eq!(Ok(123 * 1000 * 1000), parse_size("123MB")); + assert_eq!(Ok(1024 * 1024), parse_size("M")); + assert_eq!(Ok(1000 * 1000), parse_size("MB")); + assert_eq!(Ok(2 * 1_048_576), parse_size("2m")); + assert_eq!(Ok(4 * 1_048_576), parse_size("4M")); + assert_eq!(Ok(2_000_000), parse_size("2mB")); + assert_eq!(Ok(4_000_000), parse_size("4MB")); + } + + #[test] + fn gigabytes_suffix() { + assert_eq!(Ok(1_073_741_824), parse_size("1G")); + assert_eq!(Ok(2_000_000_000), parse_size("2GB")); + } + + #[test] + #[cfg(target_pointer_width = "64")] + fn x64() { + assert_eq!(Ok(1_099_511_627_776), parse_size("1T")); + assert_eq!(Ok(1_125_899_906_842_624), parse_size("1P")); + assert_eq!(Ok(1_152_921_504_606_846_976), parse_size("1E")); + assert_eq!(Ok(2_000_000_000_000), parse_size("2TB")); + assert_eq!(Ok(2_000_000_000_000_000), parse_size("2PB")); + assert_eq!(Ok(2_000_000_000_000_000_000), parse_size("2EB")); + } +} diff --git a/src/uucore/src/lib/features/parse_time.rs b/src/uucore/src/lib/parser/parse_time.rs similarity index 78% rename from src/uucore/src/lib/features/parse_time.rs rename to src/uucore/src/lib/parser/parse_time.rs index 8e822685b..fdf43b727 100644 --- a/src/uucore/src/lib/features/parse_time.rs +++ b/src/uucore/src/lib/parser/parse_time.rs @@ -20,20 +20,18 @@ pub fn from_str(string: &str) -> Result { 'm' | 'M' => (slice, 60), 'h' | 'H' => (slice, 60 * 60), 'd' | 'D' => (slice, 60 * 60 * 24), - val => { - if !val.is_alphabetic() { - (string, 1) - } else if string == "inf" || string == "infinity" { + val if !val.is_alphabetic() => (string, 1), + _ => { + if string == "inf" || string == "infinity" { ("inf", 1) } else { return Err(format!("invalid time interval '{}'", string)); } } }; - let num = match numstr.parse::() { - Ok(m) => m, - Err(e) => return Err(format!("invalid time interval '{}': {}", string, e)), - }; + let num = numstr + .parse::() + .map_err(|e| format!("invalid time interval '{}': {}", string, e))?; const NANOS_PER_SEC: u32 = 1_000_000_000; let whole_secs = num.trunc(); diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index fadf378ab..d83b5515b 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -438,7 +438,7 @@ fn test_domain_socket() { let child = new_ucmd!().args(&[socket_path]).run_no_wait(); barrier.wait(); let stdout = &child.wait_with_output().unwrap().stdout; - let output = String::from_utf8_lossy(&stdout); + let output = String::from_utf8_lossy(stdout); assert_eq!("a\tb", output); thread.join().unwrap(); diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 45380b80b..762e922c4 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -1,4 +1,4 @@ -// spell-checker:ignore (words) nosuchgroup +// spell-checker:ignore (words) nosuchgroup groupname use crate::common::util::*; use rust_users::*; @@ -10,6 +10,33 @@ fn test_invalid_option() { static DIR: &str = "/tmp"; +// we should always get both arguments, regardless of whether --reference was used +#[test] +fn test_help() { + new_ucmd!() + .arg("--help") + .succeeds() + .stdout_contains("ARGS:\n \n ... "); +} + +#[test] +fn test_help_ref() { + new_ucmd!() + .arg("--help") + .arg("--reference=ref_file") + .succeeds() + .stdout_contains("ARGS:\n \n ... "); +} + +#[test] +fn test_ref_help() { + new_ucmd!() + .arg("--reference=ref_file") + .arg("--help") + .succeeds() + .stdout_contains("ARGS:\n \n ... "); +} + #[test] fn test_invalid_group() { new_ucmd!() @@ -121,9 +148,52 @@ fn test_reference() { fn test_reference() { new_ucmd!() .arg("-v") - .arg("--reference=/etc/passwd") + .arg("--reference=ref_file") .arg("/etc") - .succeeds(); + .fails() + // group name can differ, so just check the first part of the message + .stderr_contains("chgrp: changing group of '/etc': Operation not permitted (os error 1)\nfailed to change group of '/etc' from "); +} + +#[test] +#[cfg(any(target_os = "linux", target_vendor = "apple"))] +fn test_reference_multi_no_equal() { + new_ucmd!() + .arg("-v") + .arg("--reference") + .arg("ref_file") + .arg("file1") + .arg("file2") + .succeeds() + .stderr_contains("chgrp: group of 'file1' retained as ") + .stderr_contains("\nchgrp: group of 'file2' retained as "); +} + +#[test] +#[cfg(any(target_os = "linux", target_vendor = "apple"))] +fn test_reference_last() { + new_ucmd!() + .arg("-v") + .arg("file1") + .arg("file2") + .arg("file3") + .arg("--reference") + .arg("ref_file") + .succeeds() + .stderr_contains("chgrp: group of 'file1' retained as ") + .stderr_contains("\nchgrp: group of 'file2' retained as ") + .stderr_contains("\nchgrp: group of 'file3' retained as "); +} + +#[test] +fn test_missing_files() { + new_ucmd!() + .arg("-v") + .arg("groupname") + .fails() + .stderr_contains( + "error: The following required arguments were not provided:\n ...\n", + ); } #[test] @@ -135,7 +205,7 @@ fn test_big_p() { .arg("bin") .arg("/proc/self/cwd") .fails() - .stderr_is( + .stderr_contains( "chgrp: changing group of '/proc/self/cwd': Operation not permitted (os error 1)\n", ); } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index ff607f984..bc6c6fc79 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -618,7 +618,7 @@ fn test_cp_deref() { // Check the content of the destination file that was copied. assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n"); let path_to_check = path_to_new_symlink.to_str().unwrap(); - assert_eq!(at.read(&path_to_check), "Hello, World!\n"); + assert_eq!(at.read(path_to_check), "Hello, World!\n"); } #[test] fn test_cp_no_deref() { @@ -655,7 +655,7 @@ fn test_cp_no_deref() { // Check the content of the destination file that was copied. assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n"); let path_to_check = path_to_new_symlink.to_str().unwrap(); - assert_eq!(at.read(&path_to_check), "Hello, World!\n"); + assert_eq!(at.read(path_to_check), "Hello, World!\n"); } #[test] @@ -823,7 +823,7 @@ fn test_cp_deref_folder_to_folder() { // Check the content of the symlink let path_to_check = path_to_new_symlink.to_str().unwrap(); - assert_eq!(at.read(&path_to_check), "Hello, World!\n"); + assert_eq!(at.read(path_to_check), "Hello, World!\n"); } #[test] @@ -923,7 +923,7 @@ fn test_cp_no_deref_folder_to_folder() { // Check the content of the symlink let path_to_check = path_to_new_symlink.to_str().unwrap(); - assert_eq!(at.read(&path_to_check), "Hello, World!\n"); + assert_eq!(at.read(path_to_check), "Hello, World!\n"); } #[test] diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 3c177c6bf..8d1267423 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -1,3 +1,8 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + // spell-checker:ignore (paths) sublink subwords use crate::common::util::*; @@ -73,6 +78,23 @@ fn _du_basics_subdir(s: &str) { } } +#[test] +fn test_du_invalid_size() { + new_ucmd!() + .arg("--block-size=1fb4t") + .arg("/tmp") + .fails() + .code_is(1) + .stderr_only("du: invalid --block-size argument '1fb4t'"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .arg("--block-size=1Y") + .arg("/tmp") + .fails() + .code_is(1) + .stderr_only("du: --block-size argument '1Y' too large"); +} + #[test] fn test_du_basics_bad_name() { new_ucmd!() @@ -312,3 +334,20 @@ fn _du_no_permission(s: &str) { fn _du_no_permission(s: &str) { assert_eq!(s, "4\tsubdir/links\n"); } + +#[test] +fn test_du_one_file_system() { + let scene = TestScenario::new(util_name!()); + + let result = scene.ucmd().arg("-x").arg(SUB_DIR).succeeds(); + + #[cfg(target_os = "linux")] + { + let result_reference = scene.cmd("du").arg("-x").arg(SUB_DIR).run(); + if result_reference.succeeded() { + assert_eq!(result.stdout_str(), result_reference.stdout_str()); + return; + } + } + _du_basics_subdir(result.stdout_str()); +} diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 4db3b59bd..1d76c433d 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -187,11 +187,10 @@ fn test_change_directory() { .arg(&temporary_path) .succeeds() .stdout_move_str(); - assert_eq!( - out.lines() - .any(|line| line.ends_with(temporary_path.file_name().unwrap().to_str().unwrap())), - false - ); + + assert!(!out + .lines() + .any(|line| line.ends_with(temporary_path.file_name().unwrap().to_str().unwrap()))); } #[test] diff --git a/tests/by-util/test_groups.rs b/tests/by-util/test_groups.rs index 26ab6a75a..c1b98aea1 100644 --- a/tests/by-util/test_groups.rs +++ b/tests/by-util/test_groups.rs @@ -1,7 +1,7 @@ use crate::common::util::*; #[test] -#[cfg(any(target_vendor = "apple", target_os = "linux"))] +#[cfg(unix)] fn test_groups() { if !is_ci() { new_ucmd!().succeeds().stdout_is(expected_result(&[])); @@ -13,7 +13,7 @@ fn test_groups() { } #[test] -#[cfg(any(target_os = "linux"))] +#[cfg(unix)] #[ignore = "fixme: 'groups USERNAME' needs more debugging"] fn test_groups_username() { let scene = TestScenario::new(util_name!()); @@ -37,17 +37,20 @@ fn test_groups_username() { .stdout_is(expected_result(&[&username])); } -#[cfg(any(target_vendor = "apple", target_os = "linux"))] +#[cfg(unix)] fn expected_result(args: &[&str]) -> String { - #[cfg(target_os = "linux")] - let util_name = util_name!(); - #[cfg(target_vendor = "apple")] - let util_name = format!("g{}", util_name!()); + // We want to use GNU id. On most linux systems, this is "id", but on + // bsd-like systems (e.g. FreeBSD, MacOS), it is commonly "gid". + #[cfg(any(target_os = "linux"))] + let util_name = "id"; + #[cfg(not(target_os = "linux"))] + let util_name = "gid"; - TestScenario::new(&util_name) + TestScenario::new(util_name) .cmd_keepenv(util_name) .env("LANGUAGE", "C") .args(args) + .args(&["-Gn"]) .succeeds() .stdout_move_str() } diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 7daa80e3a..2c4b66696 100755 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -1,4 +1,9 @@ -// spell-checker:ignore (words) bogusfile emptyfile +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + +// spell-checker:ignore (words) bogusfile emptyfile abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstu use crate::common::util::*; @@ -244,3 +249,61 @@ hello ", ); } + +#[test] +fn test_head_invalid_num() { + new_ucmd!() + .args(&["-c", "1024R", "emptyfile.txt"]) + .fails() + .stderr_is("head: invalid number of bytes: ‘1024R’"); + new_ucmd!() + .args(&["-n", "1024R", "emptyfile.txt"]) + .fails() + .stderr_is("head: invalid number of lines: ‘1024R’"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-c", "1Y", "emptyfile.txt"]) + .fails() + .stderr_is("head: invalid number of bytes: ‘1Y’: Value too large for defined data type"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-n", "1Y", "emptyfile.txt"]) + .fails() + .stderr_is("head: invalid number of lines: ‘1Y’: Value too large for defined data type"); + #[cfg(target_pointer_width = "32")] + { + let sizes = ["1000G", "10T"]; + for size in &sizes { + new_ucmd!() + .args(&["-c", size]) + .fails() + .code_is(1) + .stderr_only(format!( + "head: invalid number of bytes: ‘{}’: Value too large for defined data type", + size + )); + } + } +} + +#[test] +fn test_head_num_with_undocumented_sign_bytes() { + // tail: '-' is not documented (8.32 man pages) + // head: '+' is not documented (8.32 man pages) + const ALPHABET: &str = "abcdefghijklmnopqrstuvwxyz"; + new_ucmd!() + .args(&["-c", "5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("abcde"); + new_ucmd!() + .args(&["-c", "-5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("abcdefghijklmnopqrstu"); + new_ucmd!() + .args(&["-c", "+5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("abcde"); +} diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index e475e3608..fc97ff779 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -428,20 +428,6 @@ fn test_symlink_relative() { assert_eq!(at.resolve_link(link), file_a); } -#[test] -fn test_hardlink_relative() { - let (at, mut ucmd) = at_and_ucmd!(); - let file_a = "test_hardlink_relative_a"; - let link = "test_hardlink_relative_link"; - - at.touch(file_a); - - // relative hardlink - ucmd.args(&["-r", "-v", file_a, link]) - .succeeds() - .stdout_only(format!("'{}' -> '{}'\n", link, file_a)); -} - #[test] fn test_symlink_relative_path() { let (at, mut ucmd) = at_and_ucmd!(); @@ -571,3 +557,26 @@ fn test_symlink_no_deref_file() { assert!(at.is_symlink(link)); assert_eq!(at.resolve_link(link), file1); } + +#[test] +fn test_relative_requires_symbolic() { + new_ucmd!().args(&["-r", "foo", "bar"]).fails(); +} + +#[test] +fn test_relative_dst_already_symlink() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file1"); + at.symlink_file("file1", "file2"); + ucmd.arg("-srf").arg("file1").arg("file2").succeeds(); + at.is_symlink("file2"); +} + +#[test] +fn test_relative_src_already_symlink() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file1"); + at.symlink_file("file1", "file2"); + ucmd.arg("-sr").arg("file2").arg("file3").succeeds(); + assert!(at.resolve_link("file3").ends_with("file1")); +} diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 20c6b913d..f8aa4453b 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -398,7 +398,7 @@ fn test_ls_long_formats() { .arg("--author") .arg("test-long-formats") .succeeds(); - assert!(re_three.is_match(&result.stdout_str())); + assert!(re_three.is_match(result.stdout_str())); #[cfg(unix)] { @@ -701,20 +701,20 @@ fn test_ls_styles() { .arg("-l") .arg("--time-style=full-iso") .succeeds(); - assert!(re_full.is_match(&result.stdout_str())); + assert!(re_full.is_match(result.stdout_str())); //long-iso let result = scene .ucmd() .arg("-l") .arg("--time-style=long-iso") .succeeds(); - assert!(re_long.is_match(&result.stdout_str())); + assert!(re_long.is_match(result.stdout_str())); //iso let result = scene.ucmd().arg("-l").arg("--time-style=iso").succeeds(); - assert!(re_iso.is_match(&result.stdout_str())); + assert!(re_iso.is_match(result.stdout_str())); //locale let result = scene.ucmd().arg("-l").arg("--time-style=locale").succeeds(); - assert!(re_locale.is_match(&result.stdout_str())); + assert!(re_locale.is_match(result.stdout_str())); //Overwrite options tests let result = scene @@ -723,19 +723,19 @@ fn test_ls_styles() { .arg("--time-style=long-iso") .arg("--time-style=iso") .succeeds(); - assert!(re_iso.is_match(&result.stdout_str())); + assert!(re_iso.is_match(result.stdout_str())); let result = scene .ucmd() .arg("--time-style=iso") .arg("--full-time") .succeeds(); - assert!(re_full.is_match(&result.stdout_str())); + assert!(re_full.is_match(result.stdout_str())); let result = scene .ucmd() .arg("--full-time") .arg("--time-style=iso") .succeeds(); - assert!(re_iso.is_match(&result.stdout_str())); + assert!(re_iso.is_match(result.stdout_str())); let result = scene .ucmd() @@ -743,7 +743,7 @@ fn test_ls_styles() { .arg("--time-style=iso") .arg("--full-time") .succeeds(); - assert!(re_full.is_match(&result.stdout_str())); + assert!(re_full.is_match(result.stdout_str())); let result = scene .ucmd() @@ -751,7 +751,7 @@ fn test_ls_styles() { .arg("-x") .arg("-l") .succeeds(); - assert!(re_full.is_match(&result.stdout_str())); + assert!(re_full.is_match(result.stdout_str())); at.touch("test2"); let result = scene.ucmd().arg("--full-time").arg("-x").succeeds(); @@ -1143,7 +1143,7 @@ fn test_ls_indicator_style() { for opt in options { scene .ucmd() - .arg(format!("{}", opt)) + .arg(opt.to_string()) .succeeds() .stdout_contains(&"/"); } diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index c21c683dc..33d7d4dc4 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -1,3 +1,8 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + extern crate unindent; use self::unindent::*; @@ -804,3 +809,40 @@ fn test_traditional_only_label() { ", )); } + +#[test] +fn test_od_invalid_bytes() { + const INVALID_SIZE: &str = "1fb4t"; + const BIG_SIZE: &str = "1Y"; + + // NOTE: + // GNU's od (8.32) with option '--width' does not accept 'Y' as valid suffix. + // According to the man page it should be valid in the same way it is valid for + // '--read-bytes' and '--skip-bytes'. + + let options = [ + "--read-bytes", + "--skip-bytes", + "--width", + // "--strings", // TODO: consider testing here once '--strings' is implemented + ]; + for option in &options { + new_ucmd!() + .arg(format!("{}={}", option, INVALID_SIZE)) + .arg("file") + .fails() + .code_is(1) + .stderr_only(format!( + "od: invalid {} argument '{}'", + option, INVALID_SIZE + )); + + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .arg(format!("{}={}", option, BIG_SIZE)) + .arg("file") + .fails() + .code_is(1) + .stderr_only(format!("od: {} argument '{}' too large", option, BIG_SIZE)); + } +} diff --git a/tests/by-util/test_pathchk.rs b/tests/by-util/test_pathchk.rs index 3bc12f0b6..8ba3b9033 100644 --- a/tests/by-util/test_pathchk.rs +++ b/tests/by-util/test_pathchk.rs @@ -38,7 +38,7 @@ fn test_posix_mode() { // fail on long path new_ucmd!() - .args(&["-p", &"dir".repeat(libc::PATH_MAX as usize + 1).as_str()]) + .args(&["-p", "dir".repeat(libc::PATH_MAX as usize + 1).as_str()]) .fails() .no_stdout(); @@ -46,7 +46,7 @@ fn test_posix_mode() { new_ucmd!() .args(&[ "-p", - &format!("dir/{}", "file".repeat(libc::FILENAME_MAX as usize + 1)).as_str(), + format!("dir/{}", "file".repeat(libc::FILENAME_MAX as usize + 1)).as_str(), ]) .fails() .no_stdout(); @@ -76,7 +76,7 @@ fn test_posix_special() { // fail on long path new_ucmd!() - .args(&["-P", &"dir".repeat(libc::PATH_MAX as usize + 1).as_str()]) + .args(&["-P", "dir".repeat(libc::PATH_MAX as usize + 1).as_str()]) .fails() .no_stdout(); @@ -84,7 +84,7 @@ fn test_posix_special() { new_ucmd!() .args(&[ "-P", - &format!("dir/{}", "file".repeat(libc::FILENAME_MAX as usize + 1)).as_str(), + format!("dir/{}", "file".repeat(libc::FILENAME_MAX as usize + 1)).as_str(), ]) .fails() .no_stdout(); @@ -117,7 +117,7 @@ fn test_posix_all() { .args(&[ "-p", "-P", - &"dir".repeat(libc::PATH_MAX as usize + 1).as_str(), + "dir".repeat(libc::PATH_MAX as usize + 1).as_str(), ]) .fails() .no_stdout(); @@ -127,7 +127,7 @@ fn test_posix_all() { .args(&[ "-p", "-P", - &format!("dir/{}", "file".repeat(libc::FILENAME_MAX as usize + 1)).as_str(), + format!("dir/{}", "file".repeat(libc::FILENAME_MAX as usize + 1)).as_str(), ]) .fails() .no_stdout(); diff --git a/tests/by-util/test_pinky.rs b/tests/by-util/test_pinky.rs index 0813e5e1b..8b50ec2bd 100644 --- a/tests/by-util/test_pinky.rs +++ b/tests/by-util/test_pinky.rs @@ -102,6 +102,8 @@ fn expected_result(args: &[&str]) -> String { #[cfg(target_vendor = "apple")] let util_name = format!("g{}", util_name!()); + // note: clippy::needless_borrow *false positive* + #[allow(clippy::needless_borrow)] TestScenario::new(&util_name) .cmd_keepenv(util_name) .env("LANGUAGE", "C") diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 75611abfc..2894d3007 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1,3 +1,8 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + // spell-checker:ignore (words) ints use crate::common::util::*; @@ -21,9 +26,7 @@ fn test_helper(file_name: &str, possible_args: &[&str]) { #[test] fn test_buffer_sizes() { - let buffer_sizes = [ - "0", "50K", "50k", "1M", "100M", "1000G", "10T", "500E", "1Y", - ]; + let buffer_sizes = ["0", "50K", "50k", "1M", "100M"]; for buffer_size in &buffer_sizes { new_ucmd!() .arg("-n") @@ -32,6 +35,20 @@ fn test_buffer_sizes() { .arg("ext_sort.txt") .succeeds() .stdout_is_fixture("ext_sort.expected"); + + #[cfg(not(target_pointer_width = "32"))] + { + let buffer_sizes = ["1000G", "10T"]; + for buffer_size in &buffer_sizes { + new_ucmd!() + .arg("-n") + .arg("-S") + .arg(buffer_size) + .arg("ext_sort.txt") + .succeeds() + .stdout_is_fixture("ext_sort.expected"); + } + } } } @@ -43,11 +60,39 @@ fn test_invalid_buffer_size() { .arg("-S") .arg(invalid_buffer_size) .fails() + .code_is(2) .stderr_only(format!( - "sort: failed to parse buffer size `{}`: invalid digit found in string", + "sort: invalid --buffer-size argument '{}'", invalid_buffer_size )); } + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .arg("-n") + .arg("-S") + .arg("1Y") + .arg("ext_sort.txt") + .fails() + .code_is(2) + .stderr_only("sort: --buffer-size argument '1Y' too large"); + + #[cfg(target_pointer_width = "32")] + { + let buffer_sizes = ["1000G", "10T"]; + for buffer_size in &buffer_sizes { + new_ucmd!() + .arg("-n") + .arg("-S") + .arg(buffer_size) + .arg("ext_sort.txt") + .fails() + .code_is(2) + .stderr_only(format!( + "sort: --buffer-size argument '{}' too large", + buffer_size + )); + } + } } #[test] @@ -804,7 +849,7 @@ fn sort_multiple() { #[test] fn sort_empty_chunk() { new_ucmd!() - .args(&["-S", "40B"]) + .args(&["-S", "40b"]) .pipe_in("a\na\n") .succeeds() .stdout_is("a\na\n"); @@ -844,12 +889,7 @@ fn test_compress_fail() { #[test] fn test_merge_batches() { new_ucmd!() - .args(&[ - "ext_sort.txt", - "-n", - "-S", - "150B", - ]) + .args(&["ext_sort.txt", "-n", "-S", "150b"]) .succeeds() .stdout_only_fixture("ext_sort.expected"); } diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index 85b28326b..a1350534f 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -1,3 +1,8 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + extern crate rand; extern crate regex; @@ -285,3 +290,53 @@ fn test_filter_command_fails() { ucmd.args(&["--filter=/a/path/that/totally/does/not/exist", name]) .fails(); } + +#[test] +fn test_split_lines_number() { + // Test if stdout/stderr for '--lines' option is correct + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["--lines", "2", "file"]) + .succeeds() + .no_stderr() + .no_stdout(); + scene + .ucmd() + .args(&["--lines", "2fb", "file"]) + .fails() + .code_is(1) + .stderr_only("split: invalid number of lines: ‘2fb’"); +} + +#[test] +fn test_split_invalid_bytes_size() { + new_ucmd!() + .args(&["-b", "1024R"]) + .fails() + .code_is(1) + .stderr_only("split: invalid number of bytes: ‘1024R’"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-b", "1Y"]) + .fails() + .code_is(1) + .stderr_only("split: invalid number of bytes: ‘1Y’: Value too large for defined data type"); + #[cfg(target_pointer_width = "32")] + { + let sizes = ["1000G", "10T"]; + for size in &sizes { + new_ucmd!() + .args(&["-b", size]) + .fails() + .code_is(1) + .stderr_only(format!( + "split: invalid number of bytes: ‘{}’: Value too large for defined data type", + size + )); + } + } +} diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index 89dd96752..37328d5ae 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -313,6 +313,8 @@ fn expected_result(args: &[&str]) -> String { #[cfg(target_vendor = "apple")] let util_name = format!("g{}", util_name!()); + // note: clippy::needless_borrow *false positive* + #[allow(clippy::needless_borrow)] TestScenario::new(&util_name) .cmd_keepenv(util_name) .env("LANGUAGE", "C") diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index 2e09601ce..fc1b9324a 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -57,8 +57,18 @@ fn test_stdbuf_line_buffering_stdin_fails() { #[cfg(not(target_os = "windows"))] #[test] fn test_stdbuf_invalid_mode_fails() { - new_ucmd!() - .args(&["-i", "1024R", "head"]) - .fails() - .stderr_is("stdbuf: invalid mode 1024R\nTry 'stdbuf --help' for more information."); + let options = ["--input", "--output", "--error"]; + for option in &options { + new_ucmd!() + .args(&[*option, "1024R", "head"]) + .fails() + .code_is(125) + .stderr_only("stdbuf: invalid mode ‘1024R’"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&[*option, "1Y", "head"]) + .fails() + .code_is(125) + .stderr_contains("stdbuf: invalid mode ‘1Y’: Value too large for defined data type"); + } } diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index b31344c34..8478944e2 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -1,6 +1,12 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + +// spell-checker:ignore (ToDO) abcdefghijklmnopqrstuvwxyz efghijklmnopqrstuvwxyz vwxyz emptyfile + extern crate tail; -use self::tail::parse_size; use crate::common::util::*; use std::char::from_digit; use std::io::Write; @@ -236,41 +242,6 @@ fn test_bytes_big() { } } -#[test] -fn test_parse_size() { - // No suffix. - assert_eq!(Ok(1234), parse_size("1234")); - - // kB is 1000 - assert_eq!(Ok(9 * 1000), parse_size("9kB")); - - // K is 1024 - assert_eq!(Ok(2 * 1024), parse_size("2K")); - - let suffixes = [ - ('M', 2u32), - ('G', 3u32), - ('T', 4u32), - ('P', 5u32), - ('E', 6u32), - ]; - - for &(c, exp) in &suffixes { - let s = format!("2{}B", c); - assert_eq!(Ok(2 * (1000_u64).pow(exp)), parse_size(&s)); - - let s = format!("2{}", c); - assert_eq!(Ok(2 * (1024_u64).pow(exp)), parse_size(&s)); - } - - // Sizes that are too big. - assert!(parse_size("1Z").is_err()); - assert!(parse_size("1Y").is_err()); - - // Bad number - assert!(parse_size("328hdsf3290").is_err()); // spell-checker:disable-line -} - #[test] fn test_lines_with_size_suffix() { const FILE: &str = "test_lines_with_size_suffix.txt"; @@ -320,12 +291,11 @@ fn test_multiple_input_files_with_suppressed_headers() { #[test] fn test_multiple_input_quiet_flag_overrides_verbose_flag_for_suppressing_headers() { - // TODO: actually the later one should win, i.e. -qv should lead to headers being printed, -vq to them being suppressed new_ucmd!() .arg(FOOBAR_TXT) .arg(FOOBAR_2_TXT) - .arg("-q") .arg("-v") + .arg("-q") .run() .stdout_is_fixture("foobar_multiple_quiet.expected"); } @@ -388,3 +358,61 @@ fn test_positive_zero_lines() { .succeeds() .stdout_is("a\nb\nc\nd\ne\n"); } + +#[test] +fn test_tail_invalid_num() { + new_ucmd!() + .args(&["-c", "1024R", "emptyfile.txt"]) + .fails() + .stderr_is("tail: invalid number of bytes: ‘1024R’"); + new_ucmd!() + .args(&["-n", "1024R", "emptyfile.txt"]) + .fails() + .stderr_is("tail: invalid number of lines: ‘1024R’"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-c", "1Y", "emptyfile.txt"]) + .fails() + .stderr_is("tail: invalid number of bytes: ‘1Y’: Value too large for defined data type"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-n", "1Y", "emptyfile.txt"]) + .fails() + .stderr_is("tail: invalid number of lines: ‘1Y’: Value too large for defined data type"); + #[cfg(target_pointer_width = "32")] + { + let sizes = ["1000G", "10T"]; + for size in &sizes { + new_ucmd!() + .args(&["-c", size]) + .fails() + .code_is(1) + .stderr_only(format!( + "tail: invalid number of bytes: ‘{}’: Value too large for defined data type", + size + )); + } + } +} + +#[test] +fn test_tail_num_with_undocumented_sign_bytes() { + // tail: '-' is not documented (8.32 man pages) + // head: '+' is not documented (8.32 man pages) + const ALPHABET: &str = "abcdefghijklmnopqrstuvwxyz"; + new_ucmd!() + .args(&["-c", "5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("vwxyz"); + new_ucmd!() + .args(&["-c", "-5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("vwxyz"); + new_ucmd!() + .args(&["-c", "+5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("efghijklmnopqrstuvwxyz"); +} diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index bb0f4a596..2da59035e 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -1,3 +1,10 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. + +// spell-checker:ignore (words) RFILE + use crate::common::util::*; use std::io::{Seek, SeekFrom, Write}; @@ -45,9 +52,18 @@ fn test_reference() { let at = &scene.fixtures; let mut file = at.make_file(FILE2); - scene.ucmd().arg("-s").arg("+5KB").arg(FILE1).run(); + // manpage: "A FILE argument that does not exist is created." + // TODO: 'truncate' does not create the file in this case, + // but should because '--no-create' wasn't specified. + at.touch(FILE1); // TODO: remove this when 'no-create' is fixed + scene.ucmd().arg("-s").arg("+5KB").arg(FILE1).succeeds(); - scene.ucmd().arg("--reference").arg(FILE1).arg(FILE2).run(); + scene + .ucmd() + .arg("--reference") + .arg(FILE1) + .arg(FILE2) + .succeeds(); file.seek(SeekFrom::End(0)).unwrap(); let actual = file.seek(SeekFrom::Current(0)).unwrap(); @@ -231,11 +247,18 @@ fn test_size_and_reference() { ); } +#[test] +fn test_error_filename_only() { + // truncate: you must specify either ‘--size’ or ‘--reference’ + new_ucmd!().args(&["file"]).fails().stderr_contains( + "error: The following required arguments were not provided: + --reference + --size ", + ); +} + #[test] fn test_invalid_numbers() { - // TODO For compatibility with GNU, `truncate -s 0X` should cause - // the same error as `truncate -s 0X file`, but currently it returns - // a different error. new_ucmd!() .args(&["-s", "0X", "file"]) .fails() @@ -265,3 +288,36 @@ fn test_reference_with_size_file_not_found() { .fails() .stderr_contains("cannot stat 'a': No such file or directory"); } + +#[test] +fn test_truncate_bytes_size() { + // TODO: this should succeed without error, uncomment when '--no-create' is fixed + // new_ucmd!() + // .args(&["--no-create", "--size", "K", "file"]) + // .succeeds(); + new_ucmd!() + .args(&["--size", "1024R", "file"]) + .fails() + .code_is(1) + .stderr_only("truncate: Invalid number: ‘1024R’"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["--size", "1Y", "file"]) + .fails() + .code_is(1) + .stderr_only("truncate: Invalid number: ‘1Y’: Value too large for defined data type"); + #[cfg(target_pointer_width = "32")] + { + let sizes = ["1000G", "10T"]; + for size in &sizes { + new_ucmd!() + .args(&["--size", size, "file"]) + .fails() + .code_is(1) + .stderr_only(format!( + "truncate: Invalid number: ‘{}’: Value too large for defined data type", + size + )); + } + } +} diff --git a/tests/by-util/test_users.rs b/tests/by-util/test_users.rs index 8ceb0eeb8..68bdf9a5e 100644 --- a/tests/by-util/test_users.rs +++ b/tests/by-util/test_users.rs @@ -13,6 +13,8 @@ fn test_users_check_name() { #[cfg(target_vendor = "apple")] let util_name = format!("g{}", util_name!()); + // note: clippy::needless_borrow *false positive* + #[allow(clippy::needless_borrow)] let expected = TestScenario::new(&util_name) .cmd_keepenv(util_name) .env("LANGUAGE", "C") diff --git a/tests/by-util/test_who.rs b/tests/by-util/test_who.rs index 16444b0cb..4907d2306 100644 --- a/tests/by-util/test_who.rs +++ b/tests/by-util/test_who.rs @@ -238,6 +238,8 @@ fn expected_result(args: &[&str]) -> String { #[cfg(target_vendor = "apple")] let util_name = format!("g{}", util_name!()); + // note: clippy::needless_borrow *false positive* + #[allow(clippy::needless_borrow)] TestScenario::new(&util_name) .cmd_keepenv(util_name) .env("LANGUAGE", "C") diff --git a/tests/common/util.rs b/tests/common/util.rs index 64485c0a8..922d2ba36 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -641,11 +641,20 @@ impl AtPath { // Source: // http://stackoverflow.com/questions/31439011/getfinalpathnamebyhandle-without-prepended let prefix = "\\\\?\\"; + // FixME: replace ... + #[allow(clippy::manual_strip)] if s.starts_with(prefix) { String::from(&s[prefix.len()..]) } else { s } + // ... with ... + // if let Some(stripped) = s.strip_prefix(prefix) { + // String::from(stripped) + // } else { + // s + // } + // ... when using MSRV with stabilized `strip_prefix()` } } diff --git a/tests/fixtures/chgrp/file1 b/tests/fixtures/chgrp/file1 new file mode 100644 index 000000000..73b6f48ab --- /dev/null +++ b/tests/fixtures/chgrp/file1 @@ -0,0 +1 @@ +target file 1 diff --git a/tests/fixtures/chgrp/file2 b/tests/fixtures/chgrp/file2 new file mode 100644 index 000000000..7ecd32965 --- /dev/null +++ b/tests/fixtures/chgrp/file2 @@ -0,0 +1 @@ +target file 2 diff --git a/tests/fixtures/chgrp/file3 b/tests/fixtures/chgrp/file3 new file mode 100644 index 000000000..73d293aba --- /dev/null +++ b/tests/fixtures/chgrp/file3 @@ -0,0 +1 @@ +target file 3 diff --git a/tests/fixtures/chgrp/ref_file b/tests/fixtures/chgrp/ref_file new file mode 100644 index 000000000..aba32d56e --- /dev/null +++ b/tests/fixtures/chgrp/ref_file @@ -0,0 +1 @@ +Reference file diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 1ffde8311..798a33456 100644 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -1,16 +1,16 @@ #!/bin/bash -# spell-checker:ignore (paths) abmon deref discrim getlimits getopt ginstall gnulib inacc infloop inotify reflink ; (misc) INT_OFLOW OFLOW ; (vars/env) BUILDDIR SRCDIR +# spell-checker:ignore (paths) abmon deref discrim eacces getlimits getopt ginstall gnulib inacc infloop inotify reflink ; (misc) INT_OFLOW OFLOW ; (vars/env) BUILDDIR SRCDIR set -e if test ! -d ../gnu; then echo "Could not find ../gnu" - echo "git clone git@github.com:coreutils/coreutils.git ../gnu" + echo "git clone git@github.com:coreutils/coreutils.git gnu" exit 1 fi if test ! -d ../gnulib; then echo "Could not find ../gnulib" - echo "git clone git@github.com:coreutils/gnulib.git ../gnulib" + echo "git clone git@github.com:coreutils/gnulib.git gnulib" exit 1 fi