Skip to content

Commit

Permalink
dd: move argument parsing outside of Input, Output
Browse files Browse the repository at this point in the history 
Move the argument parsing code out of the `Input::new()` and
`Output::new()` functions and into the calling code. This allows the
calling code to make decisions about how to instantiate the `Input`
and `Output` objects if necessary.
  • Loading branch information
@jfinkels
jfinkels committed Aug 27, 2022
1 parent 0dbcdde commit dcf19c5
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 139 deletions.
19 changes: 0 additions & 19 deletions src/uu/dd/src/datastructures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
// file that was distributed with this source code.
// spell-checker:ignore ctable, outfile, iseek, oseek

use std::error::Error;

use uucore::error::UError;

use crate::conversion_tables::*;

type Cbs = usize;
Expand Down Expand Up @@ -102,21 +98,6 @@ pub enum CountType {
Bytes(u64),
}

#[derive(Debug)]
pub enum InternalError {
WrongInputType,
WrongOutputType,
}

impl std::fmt::Display for InternalError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "Internal dd error: Wrong Input/Output data type")
}
}

impl Error for InternalError {}
impl UError for InternalError {}

pub mod options {
pub const INFILE: &str = "if";
pub const OUTFILE: &str = "of";
Expand Down
254 changes: 134 additions & 120 deletions src/uu/dd/src/dd.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ mod datastructures;
use datastructures::*;

mod parseargs;
use parseargs::Matches;

mod conversion_tables;
use conversion_tables::*;
Expand Down Expand Up @@ -52,16 +51,14 @@ struct Input<R: Read> {
}

impl Input<io::Stdin> {
fn new(matches: &Matches) -> UResult<Self> {
let ibs = parseargs::parse_ibs(matches)?;
let print_level = parseargs::parse_status_level(matches)?;
let cflags = parseargs::parse_conv_flag_input(matches)?;
let iflags = parseargs::parse_iflags(matches)?;
let skip = parseargs::parse_seek_skip_amt(&ibs, iflags.skip_bytes, matches, options::SKIP)?;
let iseek =
parseargs::parse_seek_skip_amt(&ibs, iflags.skip_bytes, matches, options::ISEEK)?;
let count = parseargs::parse_count(&iflags, matches)?;

fn new(
ibs: usize,
print_level: Option<StatusLevel>,
count: Option<CountType>,
cflags: IConvFlags,
iflags: IFlags,
skip_amount: u64,
) -> UResult<Self> {
let mut i = Self {
src: io::stdin(),
ibs,
Expand All @@ -71,10 +68,8 @@ impl Input<io::Stdin> {
iflags,
};

// The --skip and --iseek flags are additive. On a stream, they discard bytes.
let amt = skip.unwrap_or(0) + iseek.unwrap_or(0);
if amt > 0 {
if let Err(e) = i.read_skip(amt) {
if skip_amount > 0 {
if let Err(e) = i.read_skip(skip_amount) {
if let io::ErrorKind::UnexpectedEof = e.kind() {
show_error!("'standard input': cannot skip to specified offset");
} else {
Expand Down Expand Up @@ -125,50 +120,41 @@ fn make_linux_iflags(iflags: &IFlags) -> Option<libc::c_int> {
}

impl Input<File> {
fn new(matches: &Matches) -> UResult<Self> {
let ibs = parseargs::parse_ibs(matches)?;
let print_level = parseargs::parse_status_level(matches)?;
let cflags = parseargs::parse_conv_flag_input(matches)?;
let iflags = parseargs::parse_iflags(matches)?;
let skip = parseargs::parse_seek_skip_amt(&ibs, iflags.skip_bytes, matches, options::SKIP)?;
let iseek =
parseargs::parse_seek_skip_amt(&ibs, iflags.skip_bytes, matches, options::ISEEK)?;
let count = parseargs::parse_count(&iflags, matches)?;

if let Some(fname) = matches.value_of(options::INFILE) {
let mut src = {
let mut opts = OpenOptions::new();
opts.read(true);

#[cfg(any(target_os = "linux", target_os = "android"))]
if let Some(libc_flags) = make_linux_iflags(&iflags) {
opts.custom_flags(libc_flags);
}

opts.open(fname)
.map_err_context(|| format!("failed to open {}", fname.quote()))?
};
fn new(
ibs: usize,
print_level: Option<StatusLevel>,
count: Option<CountType>,
cflags: IConvFlags,
iflags: IFlags,
fname: &str,
skip_amount: u64,
) -> UResult<Self> {
let mut src = {
let mut opts = OpenOptions::new();
opts.read(true);

// The --skip and --iseek flags are additive. On a file, they seek.
let amt = skip.unwrap_or(0) + iseek.unwrap_or(0);
if amt > 0 {
src.seek(io::SeekFrom::Start(amt))
.map_err_context(|| "failed to seek in input file".to_string())?;
#[cfg(any(target_os = "linux", target_os = "android"))]
if let Some(libc_flags) = make_linux_iflags(&iflags) {
opts.custom_flags(libc_flags);
}

let i = Self {
src,
ibs,
print_level,
count,
cflags,
iflags,
};
opts.open(fname)
.map_err_context(|| format!("failed to open {}", fname.quote()))?
};

Ok(i)
} else {
Err(Box::new(InternalError::WrongInputType))
if skip_amount > 0 {
src.seek(io::SeekFrom::Start(skip_amount))
.map_err_context(|| "failed to seek in input file".to_string())?;
}

Ok(Self {
src,
ibs,
print_level,
count,
cflags,
iflags,
})
}
}

Expand Down Expand Up @@ -278,7 +264,13 @@ impl<R: Read> Input<R> {
}

trait OutputTrait: Sized + Write {
fn new(matches: &Matches) -> UResult<Self>;
fn new(
obs: usize,
oflags: OFlags,
cflags: OConvFlags,
seek_amount: u64,
fname: &str,
) -> UResult<Self>;
fn fsync(&mut self) -> io::Result<()>;
fn fdatasync(&mut self) -> io::Result<()>;
}
Expand All @@ -290,21 +282,18 @@ struct Output<W: Write> {
}

impl OutputTrait for Output<io::Stdout> {
fn new(matches: &Matches) -> UResult<Self> {
let obs = parseargs::parse_obs(matches)?;
let cflags = parseargs::parse_conv_flag_output(matches)?;
let oflags = parseargs::parse_oflags(matches)?;
let seek = parseargs::parse_seek_skip_amt(&obs, oflags.seek_bytes, matches, options::SEEK)?;
let oseek =
parseargs::parse_seek_skip_amt(&obs, oflags.seek_bytes, matches, options::OSEEK)?;

fn new(
obs: usize,
_oflags: OFlags,
cflags: OConvFlags,
seek_amount: u64,
_fname: &str,
) -> UResult<Self> {
let mut dst = io::stdout();

// The --seek and --oseek flags are additive.
let amt = seek.unwrap_or(0) + oseek.unwrap_or(0);
// stdout is not seekable, so we just write null bytes.
if amt > 0 {
io::copy(&mut io::repeat(0u8).take(amt as u64), &mut dst)
if seek_amount > 0 {
io::copy(&mut io::repeat(0u8).take(seek_amount), &mut dst)
.map_err_context(|| String::from("write error"))?;
}

Expand Down Expand Up @@ -500,7 +489,13 @@ fn make_linux_oflags(oflags: &OFlags) -> Option<libc::c_int> {
}

impl OutputTrait for Output<File> {
fn new(matches: &Matches) -> UResult<Self> {
fn new(
obs: usize,
oflags: OFlags,
cflags: OConvFlags,
seek_amount: u64,
fname: &str,
) -> UResult<Self> {
fn open_dst(path: &Path, cflags: &OConvFlags, oflags: &OFlags) -> Result<File, io::Error> {
let mut opts = OpenOptions::new();
opts.write(true)
Expand All @@ -515,42 +510,26 @@ impl OutputTrait for Output<File> {

opts.open(path)
}
let obs = parseargs::parse_obs(matches)?;
let cflags = parseargs::parse_conv_flag_output(matches)?;
let oflags = parseargs::parse_oflags(matches)?;
let seek = parseargs::parse_seek_skip_amt(&obs, oflags.seek_bytes, matches, options::SEEK)?;
let oseek =
parseargs::parse_seek_skip_amt(&obs, oflags.seek_bytes, matches, options::OSEEK)?;

if let Some(fname) = matches.value_of(options::OUTFILE) {
let mut dst = open_dst(Path::new(&fname), &cflags, &oflags)
.map_err_context(|| format!("failed to open {}", fname.quote()))?;

// Seek to the index in the output file, truncating if requested.
//
// Calling `set_len()` may result in an error (for
// example, when calling it on `/dev/null`), but we don't
// want to terminate the process when that happens.
// Instead, we suppress the error by calling
// `Result::ok()`. This matches the behavior of GNU `dd`
// when given the command-line argument `of=/dev/null`.

// The --seek and --oseek flags are additive.
let i = seek.unwrap_or(0) + oseek.unwrap_or(0);
if !cflags.notrunc {
dst.set_len(i).ok();
}
dst.seek(io::SeekFrom::Start(i))
.map_err_context(|| "failed to seek in output file".to_string())?;

Ok(Self { dst, obs, cflags })
} else {
// The following error should only occur if someone
// mistakenly calls Output::<File>::new() without checking
// if 'of' has been provided. In this case,
// Output::<io::stdout>::new() is probably intended.
Err(Box::new(InternalError::WrongOutputType))
let mut dst = open_dst(Path::new(&fname), &cflags, &oflags)
.map_err_context(|| format!("failed to open {}", fname.quote()))?;

// Seek to the index in the output file, truncating if requested.
//
// Calling `set_len()` may result in an error (for example,
// when calling it on `/dev/null`), but we don't want to
// terminate the process when that happens. Instead, we
// suppress the error by calling `Result::ok()`. This matches
// the behavior of GNU `dd` when given the command-line
// argument `of=/dev/null`.

if !cflags.notrunc {
dst.set_len(seek_amount).ok();
}
dst.seek(io::SeekFrom::Start(seek_amount))
.map_err_context(|| "failed to seek in output file".to_string())?;

Ok(Self { dst, obs, cflags })
}

fn fsync(&mut self) -> io::Result<()> {
Expand Down Expand Up @@ -714,28 +693,61 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
//.after_help(TODO: Add note about multiplier strings here.)
.get_matches_from(dashed_args);

// Parse options for reading from input.
let ibs = parseargs::parse_ibs(&matches)?;
let print_level = parseargs::parse_status_level(&matches)?;
let icflags = parseargs::parse_conv_flag_input(&matches)?;
let iflags = parseargs::parse_iflags(&matches)?;
let skip = parseargs::parse_seek_skip_amt(&ibs, iflags.skip_bytes, &matches, options::SKIP)?;
let iseek = parseargs::parse_seek_skip_amt(&ibs, iflags.skip_bytes, &matches, options::ISEEK)?;
let count = parseargs::parse_count(&iflags, &matches)?;
// The --skip and --iseek flags are additive. On a file, they seek.
let skip_amount = skip.unwrap_or(0) + iseek.unwrap_or(0);

// Parse options for writing to the output.
let obs = parseargs::parse_obs(&matches)?;
let ocflags = parseargs::parse_conv_flag_output(&matches)?;
let oflags = parseargs::parse_oflags(&matches)?;
let seek = parseargs::parse_seek_skip_amt(&obs, oflags.seek_bytes, &matches, options::SEEK)?;
let oseek = parseargs::parse_seek_skip_amt(&obs, oflags.seek_bytes, &matches, options::OSEEK)?;
// The --seek and --oseek flags are additive.
let seek_amount = seek.unwrap_or(0) + oseek.unwrap_or(0);

match (
matches.contains_id(options::INFILE),
matches.contains_id(options::OUTFILE),
) {
(true, true) => {
let i = Input::<File>::new(&matches)?;
let o = Output::<File>::new(&matches)?;
let ifname = matches.value_of(options::INFILE).unwrap();
let ofname = matches.value_of(options::OUTFILE).unwrap();
let i = Input::<File>::new(
ibs,
print_level,
count,
icflags,
iflags,
ifname,
skip_amount,
)?;
let o = Output::<File>::new(obs, oflags, ocflags, seek_amount, ofname)?;
o.dd_out(i).map_err_context(|| "IO error".to_string())
}
(false, true) => {
let i = Input::<io::Stdin>::new(&matches)?;
let o = Output::<File>::new(&matches)?;
let ofname = matches.value_of(options::OUTFILE).unwrap();
let i = Input::<io::Stdin>::new(ibs, print_level, count, icflags, iflags, skip_amount)?;
let o = Output::<File>::new(obs, oflags, ocflags, seek_amount, ofname)?;
o.dd_out(i).map_err_context(|| "IO error".to_string())
}
(true, false) => {
let i = Input::<File>::new(&matches)?;
let o = Output::<io::Stdout>::new(&matches)?;
let fname = matches.value_of(options::INFILE).unwrap();
let i =
Input::<File>::new(ibs, print_level, count, icflags, iflags, fname, skip_amount)?;
let o = Output::<io::Stdout>::new(obs, oflags, ocflags, seek_amount, "-")?;
o.dd_out(i).map_err_context(|| "IO error".to_string())
}
(false, false) => {
let i = Input::<io::Stdin>::new(&matches)?;
let o = Output::<io::Stdout>::new(&matches)?;
let i = Input::<io::Stdin>::new(ibs, print_level, count, icflags, iflags, skip_amount)?;
let o = Output::<io::Stdout>::new(obs, oflags, ocflags, seek_amount, "-")?;
o.dd_out(i).map_err_context(|| "IO error".to_string())
}
}
Expand Down Expand Up @@ -977,8 +989,8 @@ General-Flags
#[cfg(test)]
mod tests {

use crate::datastructures::{IConvFlags, IFlags, OConvFlags};
use crate::{calc_bsize, uu_app, Input, Output, OutputTrait};
use crate::datastructures::{IConvFlags, IFlags, OConvFlags, OFlags};
use crate::{calc_bsize, Input, Output, OutputTrait};

use std::cmp;
use std::fs;
Expand Down Expand Up @@ -1070,14 +1082,16 @@ mod tests {
#[test]
#[should_panic]
fn test_nocreat_causes_failure_when_ofile_doesnt_exist() {
let args = vec![
String::from("dd"),
String::from("--conv=nocreat"),
String::from("--of=not-a-real.file"),
];

let matches = uu_app().try_get_matches_from(args).unwrap();
let _ = Output::<File>::new(&matches).unwrap();
let obs = 1;
let oflags = OFlags::default();
let ocflags = OConvFlags {
nocreat: true,
..Default::default()
};
let seek_amount = 0;
let ofname = "not-a-real.file";

let _ = Output::<File>::new(obs, oflags, ocflags, seek_amount, ofname).unwrap();
}

#[test]
Expand Down

0 comments on commit dcf19c5

Please sign in to comment.