Fix a TODO by making conv_block_unblock_helper consume the input. (#3787)

* Fix a TODO by making `conv_block_unblock_helper` consume the input.

src/uu/dd/src/blocks.rs

@@ -78,34 +78,25 @@ fn unblock(buf: &[u8], cbs: usize) -> Vec<u8> {
 /// settings of `mode`, this function will update the number of
 /// records truncated; that's why `rstat` is borrowed mutably.
 pub(crate) fn conv_block_unblock_helper(
-    mut buf: Vec<u8>,
+    buf: Vec<u8>,
     mode: &ConversionMode,
     rstat: &mut ReadStat,
 ) -> Vec<u8> {
-    // TODO This function has a mutable input `buf` but also returns a
+    fn apply_conversion(buf: Vec<u8>, ct: &ConversionTable) -> impl Iterator<Item = u8> + '_ {
-    // completely new `Vec`; that seems fishy. Could we either make
+        buf.into_iter().map(|b| ct[b as usize])
-    // the input immutable or make the function not return anything?
-
-    fn apply_conversion(buf: &mut [u8], ct: &ConversionTable) {
-        for idx in 0..buf.len() {
-            buf[idx] = ct[buf[idx] as usize];
-        }
     }
 
     match mode {
-        ConversionMode::ConvertOnly(ct) => {
+        ConversionMode::ConvertOnly(ct) => apply_conversion(buf, ct).collect(),
-            apply_conversion(&mut buf, ct);
-            buf
-        }
         ConversionMode::BlockThenConvert(ct, cbs, sync) => {
-            let mut blocks = block(&buf, *cbs, *sync, rstat);
+            let blocks = block(&buf, *cbs, *sync, rstat);
-            for buf in &mut blocks {
+            blocks
-                apply_conversion(buf, ct);
+                .into_iter()
-            }
+                .flat_map(|block| apply_conversion(block, ct))
-            blocks.into_iter().flatten().collect()
+                .collect()
         }
         ConversionMode::ConvertThenBlock(ct, cbs, sync) => {
-            apply_conversion(&mut buf, ct);
+            let buf: Vec<_> = apply_conversion(buf, ct).collect();
             block(&buf, *cbs, *sync, rstat)
                 .into_iter()
                 .flatten()
@@ -116,12 +107,11 @@ pub(crate) fn conv_block_unblock_helper(
             .flatten()
             .collect(),
         ConversionMode::UnblockThenConvert(ct, cbs) => {
-            let mut buf = unblock(&buf, *cbs);
+            let buf = unblock(&buf, *cbs);
-            apply_conversion(&mut buf, ct);
+            apply_conversion(buf, ct).collect()
-            buf
         }
         ConversionMode::ConvertThenUnblock(ct, cbs) => {
-            apply_conversion(&mut buf, ct);
+            let buf: Vec<_> = apply_conversion(buf, ct).collect();
             unblock(&buf, *cbs)
         }
         ConversionMode::UnblockOnly(cbs) => unblock(&buf, *cbs),

tests/by-util/test_dd.rs

@@ -861,6 +861,33 @@ fn test_swab_257_test() {
         .stdout_is_fixture_bytes("seq-byte-values-odd.spec");
 }
 
+#[test]
+fn test_block_lower() {
+    new_ucmd!()
+        .args(&["conv=block,lcase", "cbs=8"])
+        .pipe_in_fixture("dd-block8-lowercase.test")
+        .succeeds()
+        .stdout_is_fixture_bytes("dd-block8-lowercase.spec");
+}
+
+#[test]
+fn test_lower_block() {
+    new_ucmd!()
+        .args(&["conv=lcase,block", "cbs=8"])
+        .pipe_in_fixture("dd-block8-lowercase.test")
+        .succeeds()
+        .stdout_is_fixture_bytes("dd-block8-lowercase.spec");
+}
+
+#[test]
+fn test_unblock_lower() {
+    new_ucmd!()
+        .args(&["conv=unblock,lcase", "cbs=8"])
+        .pipe_in_fixture("dd-unblock8-lowercase.test")
+        .succeeds()
+        .stdout_is_fixture_bytes("dd-unblock8-lowercase.spec");
+}
+
 #[test]
 fn test_zeros_4k_conv_sync_obs_gt_ibs() {
     new_ucmd!()

tests/fixtures/dd/dd-block8-lowercase.spec

@@ -0,0 +1 @@
+abcd    abcdefgh

tests/fixtures/dd/dd-block8-lowercase.test

@@ -0,0 +1,2 @@
+ABCD
+ABCDEFGHI

tests/fixtures/dd/dd-unblock8-lowercase.spec

@@ -0,0 +1,2 @@
+abcd
+abcdefgh

tests/fixtures/dd/dd-unblock8-lowercase.test

@@ -0,0 +1 @@
+ABCD    ABCDEFGH