From 00cfb67b64201112e907423780d3c63ca2984fc9 Mon Sep 17 00:00:00 2001 From: "Scott E. Graves" Date: Thu, 14 Dec 2023 08:46:44 -0600 Subject: [PATCH] fix file read --- include/utils/native_file.hpp | 22 ++-- src/file_manager/file_manager_open_file.cpp | 4 +- src/utils/encrypting_reader.cpp | 9 +- src/utils/native_file.cpp | 116 +++++++++++--------- 4 files changed, 81 insertions(+), 70 deletions(-) diff --git a/include/utils/native_file.hpp b/include/utils/native_file.hpp index 73fd1540..a79bca3c 100644 --- a/include/utils/native_file.hpp +++ b/include/utils/native_file.hpp @@ -27,6 +27,11 @@ namespace repertory { class native_file final { public: + native_file(const native_file &) = delete; + native_file(native_file &&) = delete; + auto operator=(const native_file &) -> native_file & = delete; + auto operator=(native_file &&) -> native_file & = delete; + using native_file_ptr = std::shared_ptr; public: @@ -34,22 +39,21 @@ public: return std::shared_ptr(new native_file(handle)); } - [[nodiscard]] static auto clone(const native_file_ptr &nativeFile) + [[nodiscard]] static auto clone(const native_file_ptr &ptr) -> native_file_ptr; [[nodiscard]] static auto create_or_open(const std::string &source_path, - bool should_chmod, - native_file_ptr &nf) -> api_error; + bool read_only, native_file_ptr &ptr) + -> api_error; [[nodiscard]] static auto create_or_open(const std::string &source_path, - native_file_ptr &nf) -> api_error; + native_file_ptr &ptr) -> api_error; [[nodiscard]] static auto open(const std::string &source_path, - native_file_ptr &nf) -> api_error; + native_file_ptr &ptr) -> api_error; - [[nodiscard]] static auto open(const std::string &source_path, - bool should_chmod, native_file_ptr &nf) - -> api_error; + [[nodiscard]] static auto open(const std::string &source_path, bool read_only, + native_file_ptr &ptr) -> api_error; private: explicit native_file(const native_handle &handle) : handle_(handle) {} @@ -71,7 +75,7 @@ public: void close(); - [[nodiscard]] auto copy_from(const native_file_ptr &source) -> bool; + [[nodiscard]] auto copy_from(const native_file_ptr &ptr) -> bool; [[nodiscard]] auto copy_from(const std::string &path) -> bool; diff --git a/src/file_manager/file_manager_open_file.cpp b/src/file_manager/file_manager_open_file.cpp index b6e6bb4c..22092c01 100644 --- a/src/file_manager/file_manager_open_file.cpp +++ b/src/file_manager/file_manager_open_file.cpp @@ -64,8 +64,8 @@ file_manager::open_file::open_file( } if (not fsi.directory) { - set_api_error(native_file::create_or_open( - fsi.source_path, not provider_.is_direct_only(), nf_)); + set_api_error(native_file::create_or_open(fsi.source_path, + provider_.is_direct_only(), nf_)); if (get_api_error() == api_error::success) { if (read_state.has_value()) { read_state_ = read_state.value(); diff --git a/src/utils/encrypting_reader.cpp b/src/utils/encrypting_reader.cpp index e74240df..44f01e94 100644 --- a/src/utils/encrypting_reader.cpp +++ b/src/utils/encrypting_reader.cpp @@ -170,8 +170,7 @@ encrypting_reader::encrypting_reader( : key_(utils::encryption::generate_key(token)), stop_requested_(stop_requested), error_return_(error_return) { - const auto res = native_file::create_or_open( - source_path, not relative_parent_path.has_value(), source_file_); + const auto res = native_file::create_or_open(source_path, true, source_file_); if (res != api_error::success) { throw std::runtime_error("file open failed|src|" + source_path + '|' + api_error_to_string(res)); @@ -223,8 +222,7 @@ encrypting_reader::encrypting_reader(const std::string &encrypted_file_path, : key_(utils::encryption::generate_key(token)), stop_requested_(stop_requested), error_return_(error_return) { - const auto res = - native_file::create_or_open(source_path, false, source_file_); + const auto res = native_file::create_or_open(source_path, true, source_file_); if (res != api_error::success) { throw std::runtime_error("file open failed|src|" + source_path + '|' + api_error_to_string(res)); @@ -265,8 +263,7 @@ encrypting_reader::encrypting_reader( : key_(utils::encryption::generate_key(token)), stop_requested_(stop_requested), error_return_(error_return) { - const auto res = - native_file::create_or_open(source_path, false, source_file_); + const auto res = native_file::create_or_open(source_path, true, source_file_); if (res != api_error::success) { throw std::runtime_error("file open failed|src|" + source_path + '|' + api_error_to_string(res)); diff --git a/src/utils/native_file.cpp b/src/utils/native_file.cpp index 79b5665c..6221e7fd 100644 --- a/src/utils/native_file.cpp +++ b/src/utils/native_file.cpp @@ -35,20 +35,21 @@ native_file::~native_file() { } } -auto native_file::clone(const native_file_ptr &nf) -> native_file_ptr { +auto native_file::clone(const native_file_ptr &ptr) -> native_file_ptr { std::string source_path; #ifdef _WIN32 source_path.resize(MAX_PATH + 1); - ::GetFinalPathNameByHandleA(nf->get_handle(), &source_path[0u], MAX_PATH + 1, + ::GetFinalPathNameByHandleA(nf->get_handle(), source_path.data(), + MAX_PATH + 1, FILE_NAME_NORMALIZED | VOLUME_NAME_DOS); #else source_path.resize(PATH_MAX + 1); #ifdef __APPLE__ - fcntl(nf->get_handle(), F_GETPATH, &source_path[0u]); + fcntl(nf->get_handle(), F_GETPATH, source_path.data()); #else - readlink(("/proc/self/fd/" + std::to_string(nf->get_handle())).c_str(), - &source_path[0u], source_path.size()); + readlink(("/proc/self/fd/" + std::to_string(ptr->get_handle())).c_str(), + source_path.data(), source_path.size()); #endif #endif source_path = source_path.c_str(); @@ -63,52 +64,60 @@ auto native_file::clone(const native_file_ptr &nf) -> native_file_ptr { return clone; } -auto native_file::create_or_open(const std::string &source_path, - [[maybe_unused]] bool should_chmod, - native_file_ptr &nf) -> api_error { +auto native_file::create_or_open(const std::string &source_path, bool read_only, + native_file_ptr &ptr) -> api_error { #ifdef _WIN32 - auto handle = ::CreateFileA(source_path.c_str(), GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, - OPEN_ALWAYS, FILE_FLAG_RANDOM_ACCESS, nullptr); + auto handle = + read_only + ? ::CreateFileA(source_path.c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, + OPEN_ALWAYS, FILE_FLAG_RANDOM_ACCESS, nullptr) + : ::CreateFileA(source_path.c_str(), GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, + OPEN_ALWAYS, FILE_FLAG_RANDOM_ACCESS, nullptr); #else - auto handle = should_chmod ? ::open(source_path.c_str(), - O_CREAT | O_RDWR | O_CLOEXEC, 0600u) - : ::open(source_path.c_str(), O_RDWR | O_CLOEXEC); - if (should_chmod) { - chmod(source_path.c_str(), 0600u); + auto handle = + read_only + ? ::open(source_path.c_str(), O_CREAT | O_RDONLY | O_CLOEXEC, 0600U) + : ::open(source_path.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0600U); + if (not read_only) { + chmod(source_path.c_str(), 0600U); } #endif - nf = native_file::attach(handle); + ptr = native_file::attach(handle); return ((handle == REPERTORY_INVALID_HANDLE) ? api_error::os_error : api_error::success); } auto native_file::create_or_open(const std::string &source_path, - native_file_ptr &nf) -> api_error { - return create_or_open(source_path, true, nf); + native_file_ptr &ptr) -> api_error { + return create_or_open(source_path, false, ptr); } -auto native_file::open(const std::string &source_path, native_file_ptr &nf) +auto native_file::open(const std::string &source_path, native_file_ptr &ptr) -> api_error { - return open(source_path, true, nf); + return open(source_path, false, ptr); } -auto native_file::open(const std::string &source_path, - [[maybe_unused]] bool should_chmod, native_file_ptr &nf) - -> api_error { +auto native_file::open(const std::string &source_path, bool read_only, + native_file_ptr &ptr) -> api_error { #ifdef _WIN32 - auto handle = ::CreateFileA(source_path.c_str(), GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, - OPEN_EXISTING, FILE_FLAG_RANDOM_ACCESS, nullptr); + auto handle = + read_only + ? ::CreateFileA(source_path.c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, + OPEN_EXISTING, FILE_FLAG_RANDOM_ACCESS, nullptr) + : ::CreateFileA(source_path.c_str(), GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, + OPEN_EXISTING, FILE_FLAG_RANDOM_ACCESS, nullptr); #else - auto handle = should_chmod - ? ::open(source_path.c_str(), O_RDWR | O_CLOEXEC, 0600u) - : ::open(source_path.c_str(), O_RDONLY | O_CLOEXEC); - if (should_chmod) { - chmod(source_path.c_str(), 0600u); + auto handle = read_only ? ::open(source_path.c_str(), O_RDONLY | O_CLOEXEC) + : ::open(source_path.c_str(), O_RDWR | O_CLOEXEC); + if (not read_only) { + chmod(source_path.c_str(), 0600U); } #endif - nf = native_file::attach(handle); + ptr = native_file::attach(handle); return ((handle == REPERTORY_INVALID_HANDLE) ? api_error::os_error : api_error::success); } @@ -139,19 +148,19 @@ void native_file::close() { } } -auto native_file::copy_from(const native_file_ptr &nf) -> bool { - auto ret = false; - std::uint64_t file_size = 0u; - if ((ret = nf->get_file_size(file_size))) { +auto native_file::copy_from(const native_file_ptr &ptr) -> bool { + std::uint64_t file_size{}; + auto ret = ptr->get_file_size(file_size); + if (ret) { data_buffer buffer; - buffer.resize(65536u * 2u); - std::uint64_t offset = 0u; - while (ret && (file_size > 0u)) { + buffer.resize(65536ULL * 2ULL); + std::uint64_t offset{}; + while (ret && (file_size > 0U)) { std::size_t bytes_read{}; - if ((ret = nf->read_bytes(&buffer[0u], buffer.size(), offset, - bytes_read))) { - std::size_t bytes_written = 0u; - ret = write_bytes(&buffer[0u], bytes_read, offset, bytes_written); + ret = ptr->read_bytes(buffer.data(), buffer.size(), offset, bytes_read); + if (ret) { + std::size_t bytes_written{}; + ret = write_bytes(buffer.data(), bytes_read, offset, bytes_written); file_size -= bytes_read; offset += bytes_read; } @@ -164,10 +173,10 @@ auto native_file::copy_from(const native_file_ptr &nf) -> bool { auto native_file::copy_from(const std::string &path) -> bool { auto ret = false; - native_file_ptr nf; - if (native_file::create_or_open(path, nf) == api_error ::success) { - ret = copy_from(nf); - nf->close(); + native_file_ptr ptr; + if (native_file::create_or_open(path, ptr) == api_error ::success) { + ret = copy_from(ptr); + ptr->close(); } return ret; @@ -194,11 +203,12 @@ auto native_file::get_file_size(std::uint64_t &file_size) -> bool { struct stat st {}; if (fstat(handle_, &st) >= 0) { #else - struct stat64 st {}; - if (fstat64(handle_, &st) >= 0) { + struct stat64 unix_st {}; + if (fstat64(handle_, &unix_st) >= 0) { #endif - if ((ret = (st.st_size >= 0))) { - file_size = static_cast(st.st_size); + ret = (unix_st.st_size >= 0); + if (ret) { + file_size = static_cast(unix_st.st_size); } } #endif @@ -289,7 +299,7 @@ auto native_file::write_bytes(const char *buffer, std::size_t write_size, std::uint64_t write_offset, std::size_t &bytes_written) -> bool { bytes_written = 0U; - ssize_t result = 0U; + ssize_t result{}; do { result = pwrite64(handle_, &buffer[bytes_written], write_size - bytes_written,