Skip to content

Commit

Permalink
fixed a bug: src_preferred_io_size -> to_read, added a test (#225)
Browse files Browse the repository at this point in the history
fixed #223
  • Loading branch information
mikekazakov authored Apr 23, 2024
1 parent 82b4678 commit b1727ef
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
13 changes: 7 additions & 6 deletions Source/Operations/source/Copying/CopyingJob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -929,9 +929,9 @@ CopyingJob::StepResult CopyingJob::CopyNativeFileToNativeFile(vfs::NativeHost &_
}

auto read_buffer = m_Buffers[0].get(), write_buffer = m_Buffers[1].get();
const uint32_t src_preffered_io_size =
const uint32_t src_preferred_io_size =
src_fs_info.basic.io_size < m_BufferSize ? src_fs_info.basic.io_size : m_BufferSize;
const uint32_t dst_preffered_io_size =
const uint32_t dst_preferred_io_size =
dst_fs_info.basic.io_size < m_BufferSize ? dst_fs_info.basic.io_size : m_BufferSize;
constexpr int max_io_loops = 5; // looked in Apple's copyfile() - treat 5 zero-resulting reads/writes as an error
uint32_t bytes_to_write = 0;
Expand All @@ -951,7 +951,7 @@ CopyingJob::StepResult CopyingJob::CopyNativeFileToNativeFile(vfs::NativeHost &_
bytes_to_write,
destination_fd,
write_buffer,
dst_preffered_io_size,
dst_preferred_io_size,
&destination_bytes_written,
&write_return,
&_dst_path,
Expand All @@ -961,7 +961,7 @@ CopyingJob::StepResult CopyingJob::CopyNativeFileToNativeFile(vfs::NativeHost &_
int write_loops = 0;
while( left_to_write > 0 ) {
int64_t n_written =
write(destination_fd, write_buffer + has_written, std::min(left_to_write, dst_preffered_io_size));
write(destination_fd, write_buffer + has_written, std::min(left_to_write, dst_preferred_io_size));
if( n_written > 0 ) {
has_written += n_written;
left_to_write -= n_written;
Expand All @@ -984,14 +984,15 @@ CopyingJob::StepResult CopyingJob::CopyNativeFileToNativeFile(vfs::NativeHost &_

// <<<--- reading in current thread --->>>
// here we handle the case in which source io size is much smaller than dest's io size
uint32_t to_read = std::max(src_preffered_io_size, dst_preffered_io_size);
uint32_t to_read = std::max(src_preferred_io_size, dst_preferred_io_size);
if( src_stat_buffer.st_size - source_bytes_read < to_read )
to_read = uint32_t(src_stat_buffer.st_size - source_bytes_read);
uint32_t has_read = 0; // amount of bytes read into buffer this time
int read_loops = 0; // amount of zero-resulting reads
std::optional<StepResult> read_return; // optional storage for error returning
while( to_read != 0 ) {
int64_t read_result = read(source_fd, read_buffer + has_read, src_preffered_io_size);
const int64_t read_result = read(source_fd, read_buffer + has_read, to_read);
assert(read_result <= static_cast<int64_t>(to_read));
if( read_result > 0 ) {
if( _source_data_feedback )
_source_data_feedback(read_buffer + has_read, static_cast<unsigned>(read_result));
Expand Down
35 changes: 35 additions & 0 deletions Source/Operations/tests/Copying_IT.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <span>
#include <fstream>
#include <compare>
#include <thread>

using nc::ops::Copying;
using nc::ops::CopyingOptions;
Expand Down Expand Up @@ -1374,6 +1375,40 @@ Copying op(FetchItems("/System/Library/Kernels/", {"kernel"}, *TestEnv().vfs_nat
VFSEasyDelete(target_dir.c_str(), host);
}

TEST_CASE(PREFIX "Copying a native file that is being written to")
{
TempTestDir dir;
const std::filesystem::path p = dir.directory / "a";
static constexpr size_t max_size = 100'000'000;

std::atomic_bool stop = false;
std::thread t([p, &stop] {
const int f = open(p.c_str(), O_WRONLY | O_CREAT, S_IWUSR | S_IRUSR);
REQUIRE(f >= 0);
for(size_t i = 0; stop == false && i < max_size; ++i ) {
write(f, &f, 1);
}
close(f);
});

CopyingOptions opts;
opts.docopy = true;
auto host = TestEnv().vfs_native;
Copying op(FetchItems(dir.directory, {"a"}, *host), dir.directory / "b", host, opts);
op.Start();
op.Wait();

stop = true;
t.join();

REQUIRE(op.State() == OperationState::Completed);
REQUIRE(std::filesystem::status(dir.directory / "b").type() == std::filesystem::file_type::regular);
const size_t sz_a = std::filesystem::file_size(p);
const size_t sz_b = std::filesystem::file_size(dir.directory / "b");
CHECK(sz_a < max_size);
CHECK(sz_b < sz_a);
}

static std::vector<std::byte> MakeNoise(size_t _size)
{
std::vector<std::byte> bytes(_size);
Expand Down

0 comments on commit b1727ef

Please sign in to comment.