From 3f18b98c9dac3cf6f9aa5bf1b3ef9f1872f8f6a8 Mon Sep 17 00:00:00 2001 From: jfinkels Date: Wed, 29 Dec 2021 09:13:52 -0500 Subject: [PATCH] dd: return UResult from uumain() function (#2792) * dd: return UResult from uumain() function * fixup! dd: return UResult from uumain() function --- src/uu/dd/src/datastructures.rs | 7 +- src/uu/dd/src/dd.rs | 159 ++++++++++++++------------------ src/uu/dd/src/parseargs.rs | 7 ++ tests/by-util/test_dd.rs | 4 +- 4 files changed, 84 insertions(+), 93 deletions(-) diff --git a/src/uu/dd/src/datastructures.rs b/src/uu/dd/src/datastructures.rs index b4410d210..8fab1ffec 100644 --- a/src/uu/dd/src/datastructures.rs +++ b/src/uu/dd/src/datastructures.rs @@ -6,11 +6,13 @@ // file that was distributed with this source code. // spell-checker:ignore ctable, outfile -use crate::conversion_tables::*; - use std::error::Error; use std::time; +use uucore::error::UError; + +use crate::conversion_tables::*; + pub struct ProgUpdate { pub read_stat: ReadStat, pub write_stat: WriteStat, @@ -154,6 +156,7 @@ impl std::fmt::Display for InternalError { } impl Error for InternalError {} +impl UError for InternalError {} pub mod options { pub const INFILE: &str = "if"; diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 9f1d28714..644d7abb0 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -7,8 +7,6 @@ // spell-checker:ignore fname, tname, fpath, specfile, testfile, unspec, ifile, ofile, outfile, fullblock, urand, fileio, atoe, atoibm, behaviour, bmax, bremain, btotal, cflags, creat, ctable, ctty, datastructures, doesnt, etoa, fileout, fname, gnudd, iconvflags, nocache, noctty, noerror, nofollow, nolinks, nonblock, oconvflags, outfile, parseargs, rlen, rmax, rposition, rremain, rsofar, rstat, sigusr, sigval, wlen, wstat -use uucore::InvalidEncodingHandling; - #[cfg(test)] mod dd_unit_tests; @@ -21,14 +19,10 @@ use parseargs::Matches; mod conversion_tables; use conversion_tables::*; -use byte_unit::Byte; -use clap::{self, crate_version}; -use gcd::Gcd; -#[cfg(target_os = "linux")] -use signal_hook::consts::signal; use std::cmp; use std::convert::TryInto; use std::env; +#[cfg(target_os = "linux")] use std::error::Error; use std::fs::{File, OpenOptions}; use std::io::{self, Read, Seek, Write}; @@ -41,10 +35,17 @@ use std::sync::{atomic::AtomicUsize, atomic::Ordering, Arc}; use std::thread; use std::time; +use byte_unit::Byte; +use clap::{self, crate_version}; +use gcd::Gcd; +#[cfg(target_os = "linux")] +use signal_hook::consts::signal; +use uucore::display::Quotable; +use uucore::error::{FromIo, UResult, USimpleError}; +use uucore::InvalidEncodingHandling; + const ABOUT: &str = "copy, and optionally convert, a file system resource"; const BUF_INIT_BYTE: u8 = 0xDD; -const RTN_SUCCESS: i32 = 0; -const RTN_FAILURE: i32 = 1; const NEWLINE: u8 = b'\n'; const SPACE: u8 = b' '; @@ -59,7 +60,7 @@ struct Input { } impl Input { - fn new(matches: &Matches) -> Result> { + fn new(matches: &Matches) -> UResult { let ibs = parseargs::parse_ibs(matches)?; let non_ascii = parseargs::parse_input_non_ascii(matches)?; let print_level = parseargs::parse_status_level(matches)?; @@ -80,8 +81,8 @@ impl Input { if let Some(amt) = skip { let mut buf = vec![BUF_INIT_BYTE; amt]; - - i.force_fill(&mut buf, amt)?; + i.force_fill(&mut buf, amt) + .map_err_context(|| "failed to read input".to_string())?; } Ok(i) @@ -125,7 +126,7 @@ fn make_linux_iflags(iflags: &IFlags) -> Option { } impl Input { - fn new(matches: &Matches) -> Result> { + fn new(matches: &Matches) -> UResult { let ibs = parseargs::parse_ibs(matches)?; let non_ascii = parseargs::parse_input_non_ascii(matches)?; let print_level = parseargs::parse_status_level(matches)?; @@ -144,12 +145,16 @@ impl Input { opts.custom_flags(libc_flags); } - opts.open(fname)? + opts.open(fname) + .map_err_context(|| "failed to open input file".to_string())? }; if let Some(amt) = skip { - let amt: u64 = amt.try_into()?; - src.seek(io::SeekFrom::Start(amt))?; + let amt: u64 = amt + .try_into() + .map_err(|_| USimpleError::new(1, "failed to parse seek amount"))?; + src.seek(io::SeekFrom::Start(amt)) + .map_err_context(|| "failed to seek in input file".to_string())?; } let i = Input { @@ -196,7 +201,7 @@ impl Input { /// Fills a given buffer. /// Reads in increments of 'self.ibs'. /// The start of each ibs-sized read follows the previous one. - fn fill_consecutive(&mut self, buf: &mut Vec) -> Result> { + fn fill_consecutive(&mut self, buf: &mut Vec) -> std::io::Result { let mut reads_complete = 0; let mut reads_partial = 0; let mut bytes_total = 0; @@ -227,7 +232,7 @@ impl Input { /// Fills a given buffer. /// Reads in increments of 'self.ibs'. /// The start of each ibs-sized read is aligned to multiples of ibs; remaining space is filled with the 'pad' byte. - fn fill_blocks(&mut self, buf: &mut Vec, pad: u8) -> Result> { + fn fill_blocks(&mut self, buf: &mut Vec, pad: u8) -> std::io::Result { let mut reads_complete = 0; let mut reads_partial = 0; let mut base_idx = 0; @@ -263,7 +268,7 @@ impl Input { /// interpreted as EOF. /// Note: This will not return unless the source (eventually) produces /// enough bytes to meet target_len. - fn force_fill(&mut self, buf: &mut [u8], target_len: usize) -> Result> { + fn force_fill(&mut self, buf: &mut [u8], target_len: usize) -> std::io::Result { let mut base_idx = 0; while base_idx < target_len { base_idx += self.read(&mut buf[base_idx..target_len])?; @@ -274,7 +279,7 @@ impl Input { } trait OutputTrait: Sized + Write { - fn new(matches: &Matches) -> Result>; + fn new(matches: &Matches) -> UResult; fn fsync(&mut self) -> io::Result<()>; fn fdatasync(&mut self) -> io::Result<()>; } @@ -286,7 +291,7 @@ struct Output { } impl OutputTrait for Output { - fn new(matches: &Matches) -> Result> { + fn new(matches: &Matches) -> UResult { let obs = parseargs::parse_obs(matches)?; let cflags = parseargs::parse_conv_flag_output(matches)?; @@ -333,7 +338,7 @@ where }) } - fn dd_out(mut self, mut i: Input) -> Result<(), Box> { + fn dd_out(mut self, mut i: Input) -> UResult<()> { let mut rstat = ReadStat { reads_complete: 0, reads_partial: 0, @@ -366,24 +371,30 @@ where _, ) => break, (rstat_update, buf) => { - let wstat_update = self.write_blocks(buf)?; + let wstat_update = self + .write_blocks(buf) + .map_err_context(|| "failed to write output".to_string())?; rstat += rstat_update; wstat += wstat_update; } }; // Update Prog - prog_tx.send(ProgUpdate { - read_stat: rstat, - write_stat: wstat, - duration: start.elapsed(), - })?; + prog_tx + .send(ProgUpdate { + read_stat: rstat, + write_stat: wstat, + duration: start.elapsed(), + }) + .map_err(|_| USimpleError::new(1, "failed to write output"))?; } if self.cflags.fsync { - self.fsync()?; + self.fsync() + .map_err_context(|| "failed to write output".to_string())?; } else if self.cflags.fdatasync { - self.fdatasync()?; + self.fdatasync() + .map_err_context(|| "failed to write output".to_string())?; } match i.print_level { @@ -439,7 +450,7 @@ fn make_linux_oflags(oflags: &OFlags) -> Option { } impl OutputTrait for Output { - fn new(matches: &Matches) -> Result> { + fn new(matches: &Matches) -> UResult { fn open_dst(path: &Path, cflags: &OConvFlags, oflags: &OFlags) -> Result { let mut opts = OpenOptions::new(); opts.write(true) @@ -461,11 +472,15 @@ impl OutputTrait for Output { let seek = parseargs::parse_seek_amt(&obs, &oflags, matches)?; if let Some(fname) = matches.value_of(options::OUTFILE) { - let mut dst = open_dst(Path::new(&fname), &cflags, &oflags)?; + let mut dst = open_dst(Path::new(&fname), &cflags, &oflags) + .map_err_context(|| format!("failed to open {}", fname.quote()))?; if let Some(amt) = seek { - let amt: u64 = amt.try_into()?; - dst.seek(io::SeekFrom::Start(amt))?; + let amt: u64 = amt + .try_into() + .map_err(|_| USimpleError::new(1, "failed to parse seek amount"))?; + dst.seek(io::SeekFrom::Start(amt)) + .map_err_context(|| "failed to seek in output file".to_string())?; } Ok(Output { dst, obs, cflags }) @@ -580,7 +595,7 @@ fn conv_block_unblock_helper( mut buf: Vec, i: &mut Input, rstat: &mut ReadStat, -) -> Result, Box> { +) -> Result, InternalError> { // Local Predicate Fns ------------------------------------------------- fn should_block_then_conv(i: &Input) -> bool { !i.non_ascii && i.cflags.block.is_some() @@ -664,15 +679,12 @@ fn conv_block_unblock_helper( // by the parser before making it this far. // Producing this error is an alternative to risking an unwrap call // on 'cbs' if the required data is not provided. - Err(Box::new(InternalError::InvalidConvBlockUnblockCase)) + Err(InternalError::InvalidConvBlockUnblockCase) } } /// Read helper performs read operations common to all dd reads, and dispatches the buffer to relevant helper functions as dictated by the operations requested by the user. -fn read_helper( - i: &mut Input, - bsize: usize, -) -> Result<(ReadStat, Vec), Box> { +fn read_helper(i: &mut Input, bsize: usize) -> UResult<(ReadStat, Vec)> { // Local Predicate Fns ----------------------------------------------- fn is_conv(i: &Input) -> bool { i.cflags.ctable.is_some() @@ -693,8 +705,12 @@ fn read_helper( // Read let mut buf = vec![BUF_INIT_BYTE; bsize]; let mut rstat = match i.cflags.sync { - Some(ch) => i.fill_blocks(&mut buf, ch)?, - _ => i.fill_consecutive(&mut buf)?, + Some(ch) => i + .fill_blocks(&mut buf, ch) + .map_err_context(|| "failed to write output".to_string())?, + _ => i + .fill_consecutive(&mut buf) + .map_err_context(|| "failed to write output".to_string())?, }; // Return early if no data if rstat.reads_complete == 0 && rstat.reads_partial == 0 { @@ -877,28 +893,8 @@ fn append_dashes_if_not_present(mut acc: Vec, mut s: String) -> Vec - {{ - match ($i, $o) - { - (Ok(i), Ok(o)) => - (i,o), - (Err(e), _) => - { - eprintln!("dd Error: {}", e); - return RTN_FAILURE; - }, - (_, Err(e)) => - { - eprintln!("dd Error: {}", e); - return RTN_FAILURE; - }, - } - }}; -); - -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let dashed_args = args .collect_str(InvalidEncodingHandling::Ignore) .accept_any() @@ -909,47 +905,30 @@ pub fn uumain(args: impl uucore::Args) -> i32 { //.after_help(TODO: Add note about multiplier strings here.) .get_matches_from(dashed_args); - let result = match ( + match ( matches.is_present(options::INFILE), matches.is_present(options::OUTFILE), ) { (true, true) => { - let (i, o) = - unpack_or_rtn!(Input::::new(&matches), Output::::new(&matches)); - + let i = Input::::new(&matches)?; + let o = Output::::new(&matches)?; o.dd_out(i) } (false, true) => { - let (i, o) = unpack_or_rtn!( - Input::::new(&matches), - Output::::new(&matches) - ); - + let i = Input::::new(&matches)?; + let o = Output::::new(&matches)?; o.dd_out(i) } (true, false) => { - let (i, o) = unpack_or_rtn!( - Input::::new(&matches), - Output::::new(&matches) - ); - + let i = Input::::new(&matches)?; + let o = Output::::new(&matches)?; o.dd_out(i) } (false, false) => { - let (i, o) = unpack_or_rtn!( - Input::::new(&matches), - Output::::new(&matches) - ); - + let i = Input::::new(&matches)?; + let o = Output::::new(&matches)?; o.dd_out(i) } - }; - match result { - Ok(_) => RTN_SUCCESS, - Err(e) => { - eprintln!("dd exiting with error:\n\t{}", e); - RTN_FAILURE - } } } diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index a21e9567f..ef2d5f356 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -11,6 +11,7 @@ mod unit_tests; use super::*; use std::error::Error; +use uucore::error::UError; pub type Matches = clap::ArgMatches<'static>; @@ -79,6 +80,12 @@ impl std::fmt::Display for ParseError { impl Error for ParseError {} +impl UError for ParseError { + fn code(&self) -> i32 { + 1 + } +} + /// Some flags specified as part of a conv=CONV[,CONV]... block /// relate to the input file, others to the output file. #[derive(Debug, PartialEq)] diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 96f57a885..dd4204e2e 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -262,7 +262,9 @@ fn test_nocreat_causes_failure_when_outfile_not_present() { ucmd.args(&["conv=nocreat", of!(&fname)]) .pipe_in("") .fails() - .stderr_is("dd Error: No such file or directory (os error 2)"); + .stderr_only( + "dd: failed to open 'this-file-does-not-exist.txt': No such file or directory", + ); assert!(!fix.file_exists(fname)); }