dd: replace cascading if/else if with enum match

Replace a cascading `if/else if` chain in `conv_block_unblock_helper()`
with a match statement on a new enum, `ConversionMode`, that enumerates
the various modes in which `dd` can operate.

src/uu/dd/src/blocks.rs

@@ -6,7 +6,7 @@
 // spell-checker:ignore datastructures rstat rposition cflags ctable
 
 use crate::conversion_tables::ConversionTable;
-use crate::datastructures::InternalError;
+use crate::datastructures::ConversionMode;
 use crate::progress::ReadStat;
 use crate::Input;
 use std::io::Read;
@@ -65,6 +65,47 @@ fn unblock(buf: &[u8], cbs: usize) -> Vec<u8> {
     })
 }
 
+/// Given the various command-line parameters, determine the conversion mode.
+///
+/// The `conv` command-line option can take many different values,
+/// each of which may combine with others. For example, `conv=ascii`,
+/// `conv=lcase`, `conv=sync`, and so on. The arguments to this
+/// function represent the settings of those various command-line
+/// parameters. This function translates those settings to a
+/// [`ConversionMode`].
+fn conversion_mode(
+    ctable: Option<&ConversionTable>,
+    block: Option<usize>,
+    unblock: Option<usize>,
+    non_ascii: bool,
+    is_sync: bool,
+) -> Option<ConversionMode> {
+    match (ctable, block, unblock) {
+        (Some(ct), None, None) => Some(ConversionMode::ConvertOnly(ct)),
+        (Some(ct), Some(cbs), None) => {
+            if non_ascii {
+                Some(ConversionMode::ConvertThenBlock(ct, cbs, is_sync))
+            } else {
+                Some(ConversionMode::BlockThenConvert(ct, cbs, is_sync))
+            }
+        }
+        (Some(ct), None, Some(cbs)) => {
+            if non_ascii {
+                Some(ConversionMode::ConvertThenUnblock(ct, cbs))
+            } else {
+                Some(ConversionMode::UnblockThenConvert(ct, cbs))
+            }
+        }
+        (None, Some(cbs), None) => Some(ConversionMode::BlockOnly(cbs, is_sync)),
+        (None, None, Some(cbs)) => Some(ConversionMode::UnblockOnly(cbs)),
+        (None, None, None) => None,
+        // The remaining variants should never happen because the
+        // argument parsing above should result in an error before
+        // getting to this line of code.
+        _ => unreachable!(),
+    }
+}
+
 /// A helper for teasing out which options must be applied and in which order.
 /// Some user options, such as the presence of conversion tables, will determine whether the input is assumed to be ascii. The parser sets the Input::non_ascii flag accordingly.
 /// Examples:
@@ -76,94 +117,58 @@ pub(crate) fn conv_block_unblock_helper<R: Read>(
     mut buf: Vec<u8>,
     i: &mut Input<R>,
     rstat: &mut ReadStat,
-) -> Result<Vec<u8>, InternalError> {
+) -> Vec<u8> {
-    // Local Predicate Fns -------------------------------------------------
+    // TODO This function has a mutable input `buf` but also returns a
-    fn should_block_then_conv<R: Read>(i: &Input<R>) -> bool {
+    // completely new `Vec`; that seems fishy. Could we either make
-        !i.non_ascii && i.cflags.block.is_some()
+    // the input immutable or make the function not return anything?
-    }
+
-    fn should_conv_then_block<R: Read>(i: &Input<R>) -> bool {
-        i.non_ascii && i.cflags.block.is_some()
-    }
-    fn should_unblock_then_conv<R: Read>(i: &Input<R>) -> bool {
-        !i.non_ascii && i.cflags.unblock.is_some()
-    }
-    fn should_conv_then_unblock<R: Read>(i: &Input<R>) -> bool {
-        i.non_ascii && i.cflags.unblock.is_some()
-    }
-    fn conv_only<R: Read>(i: &Input<R>) -> bool {
-        i.cflags.ctable.is_some() && i.cflags.block.is_none() && i.cflags.unblock.is_none()
-    }
-    // Local Helper Fns ----------------------------------------------------
     fn apply_conversion(buf: &mut [u8], ct: &ConversionTable) {
         for idx in 0..buf.len() {
             buf[idx] = ct[buf[idx] as usize];
         }
     }
-    // --------------------------------------------------------------------
-    if conv_only(i) {
-        // no block/unblock
-        let ct = i.cflags.ctable.unwrap();
-        apply_conversion(&mut buf, ct);
 
-        Ok(buf)
+    let mode = conversion_mode(
-    } else if should_block_then_conv(i) {
+        i.cflags.ctable,
-        // ascii input so perform the block first
+        i.cflags.block,
-        let cbs = i.cflags.block.unwrap();
+        i.cflags.unblock,
-
+        i.non_ascii,
-        let mut blocks = block(&buf, cbs, i.cflags.sync.is_some(), rstat);
+        i.cflags.sync.is_some(),
-
+    )
-        if let Some(ct) = i.cflags.ctable {
+    .unwrap();
+    match &mode {
+        ConversionMode::ConvertOnly(ct) => {
+            apply_conversion(&mut buf, ct);
+            buf
+        }
+        ConversionMode::BlockThenConvert(ct, cbs, sync) => {
+            let mut blocks = block(&buf, *cbs, *sync, rstat);
             for buf in &mut blocks {
                 apply_conversion(buf, ct);
             }
+            blocks.into_iter().flatten().collect()
         }
-
+        ConversionMode::ConvertThenBlock(ct, cbs, sync) => {
-        let blocks = blocks.into_iter().flatten().collect();
-
-        Ok(blocks)
-    } else if should_conv_then_block(i) {
-        // Non-ascii so perform the conversion first
-        let cbs = i.cflags.block.unwrap();
-
-        if let Some(ct) = i.cflags.ctable {
             apply_conversion(&mut buf, ct);
+            block(&buf, *cbs, *sync, rstat)
+                .into_iter()
+                .flatten()
+                .collect()
         }
-
+        ConversionMode::BlockOnly(cbs, sync) => block(&buf, *cbs, *sync, rstat)
-        let blocks = block(&buf, cbs, i.cflags.sync.is_some(), rstat)
             .into_iter()
             .flatten()
-            .collect();
+            .collect(),
-
+        ConversionMode::UnblockThenConvert(ct, cbs) => {
-        Ok(blocks)
+            let mut buf = unblock(&buf, *cbs);
-    } else if should_unblock_then_conv(i) {
-        // ascii input so perform the unblock first
-        let cbs = i.cflags.unblock.unwrap();
-
-        let mut buf = unblock(&buf, cbs);
-
-        if let Some(ct) = i.cflags.ctable {
             apply_conversion(&mut buf, ct);
+            buf
         }
-
+        ConversionMode::ConvertThenUnblock(ct, cbs) => {
-        Ok(buf)
-    } else if should_conv_then_unblock(i) {
-        // Non-ascii input so perform the conversion first
-        let cbs = i.cflags.unblock.unwrap();
-
-        if let Some(ct) = i.cflags.ctable {
             apply_conversion(&mut buf, ct);
+            unblock(&buf, *cbs)
         }
-
+        ConversionMode::UnblockOnly(cbs) => unblock(&buf, *cbs),
-        let buf = unblock(&buf, cbs);
-
-        Ok(buf)
-    } else {
-        // The following error should not happen, as it results from
-        // insufficient command line data. This case should be caught
-        // 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(InternalError::InvalidConvBlockUnblockCase)
     }
 }
 

src/uu/dd/src/datastructures.rs

@@ -14,6 +14,23 @@ use crate::conversion_tables::*;
 
 type Cbs = usize;
 
+/// How to apply conversion, blocking, and/or unblocking.
+///
+/// Certain settings of the `conv` parameter to `dd` require a
+/// combination of conversion, blocking, or unblocking, applied in a
+/// certain order. The variants of this enumeration give the different
+/// ways of combining those three operations.
+#[derive(Debug, PartialEq)]
+pub(crate) enum ConversionMode<'a> {
+    ConvertOnly(&'a ConversionTable),
+    BlockOnly(Cbs, bool),
+    UnblockOnly(Cbs),
+    BlockThenConvert(&'a ConversionTable, Cbs, bool),
+    ConvertThenBlock(&'a ConversionTable, Cbs, bool),
+    UnblockThenConvert(&'a ConversionTable, Cbs),
+    ConvertThenUnblock(&'a ConversionTable, Cbs),
+}
+
 /// Stores all Conv Flags that apply to the input
 #[derive(Debug, Default, PartialEq)]
 pub struct IConvFlags {
@@ -91,19 +108,11 @@ pub enum CountType {
 pub enum InternalError {
     WrongInputType,
     WrongOutputType,
-    InvalidConvBlockUnblockCase,
 }
 
 impl std::fmt::Display for InternalError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
+        write!(f, "Internal dd error: Wrong Input/Output data type")
-            Self::WrongInputType | Self::WrongOutputType => {
-                write!(f, "Internal dd error: Wrong Input/Output data type")
-            }
-            Self::InvalidConvBlockUnblockCase => {
-                write!(f, "Invalid Conversion, Block, or Unblock data")
-            }
-        }
     }
 }
 

src/uu/dd/src/dd.rs

@@ -640,7 +640,7 @@ fn read_helper<R: Read>(i: &mut Input<R>, bsize: usize) -> std::io::Result<(Read
         perform_swab(&mut buf);
     }
     if is_conv(i) || is_block(i) || is_unblock(i) {
-        let buf = conv_block_unblock_helper(buf, i, &mut rstat).unwrap();
+        let buf = conv_block_unblock_helper(buf, i, &mut rstat);
         Ok((rstat, buf))
     } else {
         Ok((rstat, buf))