Rework FileSystem::copy to operate on 1MiB chunks

Also add a new test for the copy function.

CHANGELOG.md

@@ -12,6 +12,7 @@
 - `Parity` and `StopBits` are now a newtype-enums instead of Rust enums. Their
   members now have upper-case names.
 - `FileSystem::try_exists` now returns `FileSystemResult<bool>`.
+- `FileSystem::copy` is now more efficient for large files.
 
 ## uefi-macros - [Unreleased]
 

uefi-test-runner/src/fs/mod.rs

@@ -24,15 +24,6 @@ pub fn test(sfs: ScopedProtocol<SimpleFileSystem>) -> Result<(), fs::Error> {
     let read = String::from_utf8(read).expect("Should be valid utf8");
     assert_eq!(read.as_str(), data_to_write);
 
-    // test copy from non-existent file: does the error type work as expected?
-    let err = fs.copy(cstr16!("not_found"), cstr16!("abc"));
-    let expected_err = fs::Error::Io(IoError {
-        path: PathBuf::from(cstr16!("not_found")),
-        context: IoErrorContext::OpenError,
-        uefi_error: uefi::Error::new(Status::NOT_FOUND, ()),
-    });
-    assert_eq!(err, Err(expected_err));
-
     // test rename file + path buf replaces / with \
     fs.rename(
         PathBuf::from(cstr16!("/foo_dir/foo_cpy")),
@@ -64,5 +55,115 @@ pub fn test(sfs: ScopedProtocol<SimpleFileSystem>) -> Result<(), fs::Error> {
     // file should not be available after remove all
     assert!(!fs.try_exists(cstr16!("foo_dir\\1"))?);
 
+    test_copy_error(&mut fs)?;
+    test_copy_success(&mut fs)?;
+    test_copy_success_chunks(&mut fs)?;
+
+    Ok(())
+}
+
+fn test_copy_error(fs: &mut FileSystem) -> Result<(), fs::Error> {
+    let file1_path = cstr16!("file1");
+    let dir_path = cstr16!("dir");
+
+    // Test copy when the destination exists but the source does not. Verify
+    // that the destination is not deleted or altered.
+    fs.write(file1_path, "data1")?;
+    assert_eq!(
+        fs.copy(cstr16!("src"), file1_path),
+        Err(fs::Error::Io(IoError {
+            path: PathBuf::from(cstr16!("src")),
+            context: IoErrorContext::OpenError,
+            uefi_error: uefi::Error::new(Status::NOT_FOUND, ()),
+        }))
+    );
+    assert_eq!(fs.read(file1_path)?, b"data1");
+
+    // Test copy when the source is a directory. Verify that the destination is
+    // not deleted or altered.
+    fs.create_dir(dir_path)?;
+    assert_eq!(
+        fs.copy(dir_path, file1_path),
+        Err(fs::Error::Io(IoError {
+            path: PathBuf::from(dir_path),
+            context: IoErrorContext::NotAFile,
+            uefi_error: uefi::Error::new(Status::INVALID_PARAMETER, ()),
+        }))
+    );
+    assert_eq!(fs.read(file1_path)?, b"data1");
+
+    // Test copy when the source is valid but the destination is a
+    // directory. Verify that the directory is not deleted.
+    assert_eq!(
+        fs.copy(file1_path, dir_path),
+        Err(fs::Error::Io(IoError {
+            path: PathBuf::from(dir_path),
+            context: IoErrorContext::OpenError,
+            uefi_error: uefi::Error::new(Status::INVALID_PARAMETER, ()),
+        }))
+    );
+    assert_eq!(fs.try_exists(dir_path), Ok(true));
+
+    // Clean up temporary files.
+    fs.remove_file(file1_path)?;
+    fs.remove_dir(dir_path)?;
+
+    Ok(())
+}
+
+fn test_copy_success(fs: &mut FileSystem) -> Result<(), fs::Error> {
+    let file1_path = cstr16!("file1");
+    let file2_path = cstr16!("file2");
+
+    // Test a successful copy where the destination does not already exist.
+    fs.write(file1_path, "data1")?;
+    assert_eq!(fs.try_exists(file2_path), Ok(false));
+    fs.copy(file1_path, file2_path)?;
+    assert_eq!(fs.read(file1_path)?, b"data1");
+    assert_eq!(fs.read(file2_path)?, b"data1");
+
+    // Test that when copying a smaller file over a larger file, the file is
+    // properly truncated. Also check that the original file is unchanged.
+    fs.write(file2_path, "some long data")?;
+    fs.copy(file1_path, file2_path)?;
+    assert_eq!(fs.read(file1_path)?, b"data1");
+    assert_eq!(fs.read(file2_path)?, b"data1");
+
+    // Clean up temporary files.
+    fs.remove_file(file1_path)?;
+    fs.remove_file(file2_path)?;
+
+    Ok(())
+}
+fn test_copy_success_chunks(fs: &mut FileSystem) -> Result<(), fs::Error> {
+    let file1_path = cstr16!("file1");
+    let file2_path = cstr16!("file2");
+
+    // Test copy of a large file, where the file's size is an even multiple of
+    // the 1MiB chunk size.
+    let chunk_size = 1024 * 1024;
+    let mut big_buf = Vec::with_capacity(5 * chunk_size);
+    for i in 0..(4 * chunk_size) {
+        let byte = u8::try_from(i % 255).unwrap();
+        big_buf.push(byte);
+    }
+    fs.write(file1_path, &big_buf)?;
+    fs.copy(file1_path, file2_path)?;
+    assert_eq!(fs.read(file1_path)?, big_buf);
+    assert_eq!(fs.read(file2_path)?, big_buf);
+
+    // Test copy of a large file, where the file's size is not an even multiple
+    // of the 1MiB chunk size.
+    big_buf.extend(b"some extra data");
+    assert_ne!(big_buf.len() % chunk_size, 0);
+    fs.write(file1_path, &big_buf)?;
+    fs.copy(file1_path, file2_path)?;
+    assert_eq!(fs.read(file1_path)?, big_buf);
+    assert_eq!(fs.read(file2_path)?, big_buf);
+
+    // Clean up temporary files.
+    fs.remove_file(file1_path)?;
+    fs.remove_file(file2_path)?;
+
     Ok(())
 }

uefi/src/fs/file_system/fs.rs

@@ -52,8 +52,93 @@ impl<'a> FileSystem<'a> {
         src_path: impl AsRef<Path>,
         dest_path: impl AsRef<Path>,
     ) -> FileSystemResult<()> {
-        let read = self.read(src_path)?;
-        self.write(dest_path, read)
+        let src_path = src_path.as_ref();
+        let dest_path = dest_path.as_ref();
+
+        // Open the source file for reading.
+        let mut src = self
+            .open(src_path, UefiFileMode::Read, false)?
+            .into_regular_file()
+            .ok_or(Error::Io(IoError {
+                path: src_path.to_path_buf(),
+                context: IoErrorContext::NotAFile,
+                uefi_error: Status::INVALID_PARAMETER.into(),
+            }))?;
+
+        // Get the source file's size in bytes.
+        let src_size = {
+            let src_info = src.get_boxed_info::<UefiFileInfo>().map_err(|err| {
+                Error::Io(IoError {
+                    path: src_path.to_path_buf(),
+                    context: IoErrorContext::Metadata,
+                    uefi_error: err,
+                })
+            })?;
+            src_info.file_size()
+        };
+
+        // Try to delete the destination file in case it already exists. Allow
+        // this to fail, since it might not exist. Or it might exist, but be a
+        // directory, in which case the error will be caught when trying to
+        // create the file.
+        let _ = self.remove_file(dest_path);
+
+        // Create and open the destination file.
+        let mut dest = self
+            .open(dest_path, UefiFileMode::CreateReadWrite, false)?
+            .into_regular_file()
+            .ok_or(Error::Io(IoError {
+                path: dest_path.to_path_buf(),
+                context: IoErrorContext::OpenError,
+                uefi_error: Status::INVALID_PARAMETER.into(),
+            }))?;
+
+        // 1 MiB copy buffer.
+        let mut chunk = vec![0; 1024 * 1024];
+
+        // Read chunks from the source file and write to the destination file.
+        let mut remaining_size = src_size;
+        while remaining_size > 0 {
+            // Read one chunk.
+            let num_bytes_read = src.read(&mut chunk).map_err(|err| {
+                Error::Io(IoError {
+                    path: src_path.to_path_buf(),
+                    context: IoErrorContext::ReadFailure,
+                    uefi_error: err.to_err_without_payload(),
+                })
+            })?;
+
+            // If the read returned no bytes, but `remaining_size > 0`, return
+            // an error.
+            if num_bytes_read == 0 {
+                return Err(Error::Io(IoError {
+                    path: src_path.to_path_buf(),
+                    context: IoErrorContext::ReadFailure,
+                    uefi_error: Status::ABORTED.into(),
+                }));
+            }
+
+            // Copy the bytes read out to the destination file.
+            dest.write(&chunk[..num_bytes_read]).map_err(|err| {
+                Error::Io(IoError {
+                    path: dest_path.to_path_buf(),
+                    context: IoErrorContext::WriteFailure,
+                    uefi_error: err.to_err_without_payload(),
+                })
+            })?;
+
+            remaining_size -= u64::try_from(num_bytes_read).unwrap();
+        }
+
+        dest.flush().map_err(|err| {
+            Error::Io(IoError {
+                path: dest_path.to_path_buf(),
+                context: IoErrorContext::FlushFailure,
+                uefi_error: err,
+            })
+        })?;
+
+        Ok(())
     }
 
     /// Creates a new, empty directory at the provided path