From db3467b553aaa195609103028b3724dbc5166a96 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 27 Aug 2022 14:05:21 -0500 Subject: [PATCH] refactor: remove tr_sys file_map_for_reading() (#3723) --- libtransmission/blocklist.cc | 217 +++++++++-------------- libtransmission/blocklist.h | 31 ++-- libtransmission/file-posix.cc | 32 ---- libtransmission/file-win32.cc | 48 ----- libtransmission/file.cc | 66 ------- libtransmission/file.h | 49 ----- libtransmission/natpmp_local.h | 2 +- libtransmission/session.cc | 2 +- tests/libtransmission/file-test.cc | 128 ------------- tests/libtransmission/subprocess-test.cc | 108 +++++------ 10 files changed, 139 insertions(+), 544 deletions(-) diff --git a/libtransmission/blocklist.cc b/libtransmission/blocklist.cc index 60eb6e9ef..b1275a57d 100644 --- a/libtransmission/blocklist.cc +++ b/libtransmission/blocklist.cc @@ -7,6 +7,7 @@ #include #include #include // bsearch() +#include #include #include @@ -27,84 +28,39 @@ void BlocklistFile::close() { - if (rules_ != nullptr) - { - tr_sys_file_unmap(rules_, byte_count_); - tr_sys_file_close(fd_); - rules_ = nullptr; - rule_count_ = 0; - byte_count_ = 0; - fd_ = TR_BAD_SYS_FILE; - } + rules_.clear(); } -void BlocklistFile::load() +void BlocklistFile::ensureLoaded() const { - close(); - - auto const info = tr_sys_path_get_info(getFilename()); - if (!info) + if (!std::empty(rules_)) { return; } - auto const byte_count = info->size; - if (byte_count == 0) - { - return; - } - - tr_error* error = nullptr; - auto const fd = tr_sys_file_open(getFilename(), TR_SYS_FILE_READ, 0, &error); - if (fd == TR_BAD_SYS_FILE) + auto in = std::ifstream{ filename_, std::ios_base::in | std::ios_base::binary }; + if (!in) { tr_logAddWarn(fmt::format( _("Couldn't read '{path}': {error} ({error_code})"), - fmt::arg("path", getFilename()), - fmt::arg("error", error->message), - fmt::arg("error_code", error->code))); - tr_error_free(error); + fmt::arg("path", filename_), + fmt::arg("error", tr_strerror(errno)), + fmt::arg("error_code", errno))); return; } - rules_ = static_cast(tr_sys_file_map_for_reading(fd, 0, byte_count, &error)); - if (rules_ == nullptr) + auto range = IPv4Range{}; + while (in.read(reinterpret_cast(&range), sizeof(range))) { - tr_logAddWarn(fmt::format( - _("Couldn't read '{path}': {error} ({error_code})"), - fmt::arg("path", getFilename()), - fmt::arg("error", error->message), - fmt::arg("error_code", error->code))); - tr_sys_file_close(fd); - tr_error_free(error); - return; + rules_.emplace_back(range); } - fd_ = fd; - byte_count_ = byte_count; - rule_count_ = byte_count / sizeof(IPv4Range); - tr_logAddInfo(fmt::format( - ngettext("Blocklist '{path}' has {count} entry", "Blocklist '{path}' has {count} entries", rule_count_), - fmt::arg("path", tr_sys_path_basename(getFilename())), - fmt::arg("count", rule_count_))); + ngettext("Blocklist '{path}' has {count} entry", "Blocklist '{path}' has {count} entries", std::size(rules_)), + fmt::arg("path", tr_sys_path_basename(filename_)), + fmt::arg("count", std::size(rules_)))); } -void BlocklistFile::ensureLoaded() -{ - if (rules_ == nullptr) - { - load(); - } -} - -// TODO: unused -//static void blocklistDelete(tr_blocklistFile* b) -//{ -// blocklistClose(b); -// tr_sys_path_remove(b->filename, nullptr); -//} - /*** **** PACKAGE-VISIBLE ***/ @@ -120,7 +76,7 @@ bool BlocklistFile::hasAddress(tr_address const& addr) ensureLoaded(); - if (rules_ == nullptr || rule_count_ == 0) + if (std::empty(rules_)) { return false; } @@ -130,7 +86,7 @@ bool BlocklistFile::hasAddress(tr_address const& addr) // std::binary_search works differently and requires a less-than comparison // and two arguments of the same type. std::bsearch is the right choice. auto const* range = static_cast( - std::bsearch(&needle, rules_, rule_count_, sizeof(IPv4Range), IPv4Range::compareAddressToRange)); + std::bsearch(&needle, std::data(rules_), std::size(rules_), sizeof(IPv4Range), IPv4Range::compareAddressToRange)); return range != nullptr; } @@ -267,114 +223,101 @@ bool BlocklistFile::compareAddressRangesByFirstAddress(IPv4Range const& a, IPv4R size_t BlocklistFile::setContent(char const* filename) { - int inCount = 0; - auto line = std::array{}; - tr_error* error = nullptr; - if (filename == nullptr) { - return 0; + return {}; } - auto const in = tr_sys_file_open(filename, TR_SYS_FILE_READ, 0, &error); - if (in == TR_BAD_SYS_FILE) + auto in = std::ifstream{ filename }; + if (!in.is_open()) { tr_logAddWarn(fmt::format( _("Couldn't read '{path}': {error} ({error_code})"), fmt::arg("path", filename), - fmt::arg("error", error->message), - fmt::arg("error_code", error->code))); - tr_error_free(error); - return 0; + fmt::arg("error", tr_strerror(errno)), + fmt::arg("error_code", errno))); + return {}; } - close(); - - auto const out = tr_sys_file_open( - getFilename(), - TR_SYS_FILE_WRITE | TR_SYS_FILE_CREATE | TR_SYS_FILE_TRUNCATE, - 0666, - &error); - if (out == TR_BAD_SYS_FILE) + auto line = std::string{}; + auto line_number = size_t{ 0U }; + auto ranges = std::vector{}; + while (std::getline(in, line)) { - tr_logAddWarn(fmt::format( - _("Couldn't read '{path}': {error} ({error_code})"), - fmt::arg("path", getFilename()), - fmt::arg("error", error->message), - fmt::arg("error_code", error->code))); - tr_error_free(error); - tr_sys_file_close(in); - return 0; - } - - /* load the rules into memory */ - std::vector ranges; - while (tr_sys_file_read_line(in, std::data(line), std::size(line))) - { - IPv4Range range = {}; - - ++inCount; - - if (!parseLine(std::data(line), &range)) + ++line_number; + auto range = IPv4Range{}; + if (!parseLine(line.c_str(), &range)) { /* don't try to display the actual lines - it causes issues */ - tr_logAddWarn(fmt::format(_("Couldn't parse line: '{line}'"), fmt::arg("line", inCount))); + tr_logAddWarn(fmt::format(_("Couldn't parse line: '{line}'"), fmt::arg("line", line_number))); continue; } ranges.push_back(range); } + in.close(); - if (!std::empty(ranges)) // sort and merge + if (std::empty(ranges)) { - size_t keep = 0; // index in ranges - - std::sort(std::begin(ranges), std::end(ranges), BlocklistFile::compareAddressRangesByFirstAddress); - - // merge - for (auto const& r : ranges) - { - if (ranges[keep].end_ < r.begin_) - { - ranges[++keep] = r; - } - else if (ranges[keep].end_ < r.end_) - { - ranges[keep].end_ = r.end_; - } - } - - TR_ASSERT_MSG(keep + 1 <= std::size(ranges), "Can shrink `ranges` or leave intact, but not grow"); - ranges.resize(keep + 1); - -#ifdef TR_ENABLE_ASSERTS - assertValidRules(ranges); -#endif + return {}; } - if (!tr_sys_file_write(out, ranges.data(), sizeof(IPv4Range) * std::size(ranges), nullptr, &error)) + size_t keep = 0; // index in ranges + + std::sort(std::begin(ranges), std::end(ranges), BlocklistFile::compareAddressRangesByFirstAddress); + + // merge + for (auto const& range : ranges) + { + if (ranges[keep].end_ < range.begin_) + { + ranges[++keep] = range; + } + else if (ranges[keep].end_ < range.end_) + { + ranges[keep].end_ = range.end_; + } + } + + TR_ASSERT_MSG(keep + 1 <= std::size(ranges), "Can shrink `ranges` or leave intact, but not grow"); + ranges.resize(keep + 1); + +#ifdef TR_ENABLE_ASSERTS + assertValidRules(ranges); +#endif + + auto out = std::ofstream{ filename_, std::ios_base::out | std::ios_base::trunc | std::ios_base::binary }; + if (!out.is_open()) + { + tr_logAddWarn(fmt::format( + _("Couldn't read '{path}': {error} ({error_code})"), + fmt::arg("path", filename_), + fmt::arg("error", tr_strerror(errno)), + fmt::arg("error_code", errno))); + return {}; + } + + if (!out.write(reinterpret_cast(ranges.data()), std::size(ranges) * sizeof(IPv4Range))) { tr_logAddWarn(fmt::format( _("Couldn't save '{path}': {error} ({error_code})"), - fmt::arg("path", getFilename()), - fmt::arg("error", error->message), - fmt::arg("error_code", error->code))); - tr_error_free(error); + fmt::arg("path", filename_), + fmt::arg("error", tr_strerror(errno)), + fmt::arg("error_code", errno))); } else { tr_logAddInfo(fmt::format( - ngettext("Blocklist '{path}' has {count} entry", "Blocklist '{path}' has {count} entries", rule_count_), - fmt::arg("path", tr_sys_path_basename(getFilename())), - fmt::arg("count", rule_count_))); + ngettext("Blocklist '{path}' has {count} entry", "Blocklist '{path}' has {count} entries", std::size(rules_)), + fmt::arg("path", tr_sys_path_basename(filename_)), + fmt::arg("count", std::size(rules_)))); } - tr_sys_file_close(out); - tr_sys_file_close(in); + out.close(); - load(); - - return std::size(ranges); + close(); + ensureLoaded(); + return std::size(rules_); } #ifdef TR_ENABLE_ASSERTS diff --git a/libtransmission/blocklist.h b/libtransmission/blocklist.h index 39105b5fc..582cfcec3 100644 --- a/libtransmission/blocklist.h +++ b/libtransmission/blocklist.h @@ -31,8 +31,8 @@ public: BlocklistFile& operator=(BlocklistFile&&) = delete; BlocklistFile(char const* filename, bool isEnabled) - : is_enabled_(isEnabled) - , filename_(filename) + : filename_(filename) + , is_enabled_(isEnabled) { } @@ -41,22 +41,21 @@ public: close(); } + [[nodiscard]] constexpr auto& filename() const + { + return filename_; + } + [[nodiscard]] bool exists() const { - return tr_sys_path_exists(getFilename(), nullptr); + return tr_sys_path_exists(filename_.c_str(), nullptr); } - [[nodiscard]] char const* getFilename() const - { - return filename_.c_str(); - } - - // TODO: This function should be const, but cannot be const due to it calling ensureLoaded() - size_t getRuleCount() + [[nodiscard]] size_t getRuleCount() const { ensureLoaded(); - return rule_count_; + return std::size(rules_); } [[nodiscard]] constexpr bool isEnabled() const @@ -100,7 +99,7 @@ private: } }; - void ensureLoaded(); + void ensureLoaded() const; void load(); void close(); @@ -116,12 +115,8 @@ private: static void assertValidRules(std::vector const& ranges); #endif - bool is_enabled_; - tr_sys_file_t fd_{ TR_BAD_SYS_FILE }; - size_t rule_count_ = 0; - uint64_t byte_count_ = 0; std::string const filename_; - /// @brief Not a container, memory mapped file - IPv4Range* rules_ = nullptr; + bool is_enabled_ = false; + mutable std::vector rules_; }; diff --git a/libtransmission/file-posix.cc b/libtransmission/file-posix.cc index 688ce5756..897fa5b2c 100644 --- a/libtransmission/file-posix.cc +++ b/libtransmission/file-posix.cc @@ -21,7 +21,6 @@ #include /* O_LARGEFILE, posix_fadvise(), [posix_]fallocate(), fcntl() */ #include /* basename(), dirname() */ #include /* flock() */ -#include /* mmap(), munmap() */ #include #include #include /* lseek(), write(), ftruncate(), pread(), pwrite(), pathconf(), etc */ @@ -1070,37 +1069,6 @@ bool tr_sys_file_preallocate(tr_sys_file_t handle, uint64_t size, int flags, tr_ return false; } -void* tr_sys_file_map_for_reading(tr_sys_file_t handle, uint64_t offset, uint64_t size, tr_error** error) -{ - TR_ASSERT(handle != TR_BAD_SYS_FILE); - TR_ASSERT(size > 0); - - void* ret = mmap(nullptr, size, PROT_READ, MAP_SHARED, handle, offset); - - if (ret == MAP_FAILED) // NOLINT(performance-no-int-to-ptr) - { - set_system_error(error, errno); - ret = nullptr; - } - - return ret; -} - -bool tr_sys_file_unmap(void const* address, uint64_t size, tr_error** error) -{ - TR_ASSERT(address != nullptr); - TR_ASSERT(size > 0); - - bool const ret = munmap(const_cast(address), size) != -1; - - if (!ret) - { - set_system_error(error, errno); - } - - return ret; -} - bool tr_sys_file_lock([[maybe_unused]] tr_sys_file_t handle, [[maybe_unused]] int operation, tr_error** error) { TR_ASSERT(handle != TR_BAD_SYS_FILE); diff --git a/libtransmission/file-win32.cc b/libtransmission/file-win32.cc index 6f5245f05..c6011bdb0 100644 --- a/libtransmission/file-win32.cc +++ b/libtransmission/file-win32.cc @@ -1198,54 +1198,6 @@ bool tr_sys_file_preallocate(tr_sys_file_t handle, uint64_t size, int flags, tr_ return tr_sys_file_truncate(handle, size, error); } -void* tr_sys_file_map_for_reading(tr_sys_file_t handle, uint64_t offset, uint64_t size, tr_error** error) -{ - TR_ASSERT(handle != TR_BAD_SYS_FILE); - TR_ASSERT(size > 0); - - if (size > MAXSIZE_T) - { - set_system_error(error, ERROR_INVALID_PARAMETER); - return nullptr; - } - - void* ret = nullptr; - HANDLE mappingHandle = CreateFileMappingW(handle, nullptr, PAGE_READONLY, 0, 0, nullptr); - - if (mappingHandle != nullptr) - { - ULARGE_INTEGER native_offset; - - native_offset.QuadPart = offset; - - ret = MapViewOfFile(mappingHandle, FILE_MAP_READ, native_offset.u.HighPart, native_offset.u.LowPart, (SIZE_T)size); - } - - if (ret == nullptr) - { - set_system_error(error, GetLastError()); - } - - CloseHandle(mappingHandle); - - return ret; -} - -bool tr_sys_file_unmap(void const* address, [[maybe_unused]] uint64_t size, tr_error** error) -{ - TR_ASSERT(address != nullptr); - TR_ASSERT(size > 0); - - bool ret = UnmapViewOfFile(address); - - if (!ret) - { - set_system_error(error, GetLastError()); - } - - return ret; -} - bool tr_sys_file_lock(tr_sys_file_t handle, int operation, tr_error** error) { TR_ASSERT(handle != TR_BAD_SYS_FILE); diff --git a/libtransmission/file.cc b/libtransmission/file.cc index 012745f71..0b1923350 100644 --- a/libtransmission/file.cc +++ b/libtransmission/file.cc @@ -13,72 +13,6 @@ using namespace std::literals; -bool tr_sys_file_read_line(tr_sys_file_t handle, char* buffer, size_t buffer_size, tr_error** error) -{ - TR_ASSERT(handle != TR_BAD_SYS_FILE); - TR_ASSERT(buffer != nullptr); - TR_ASSERT(buffer_size > 0); - - auto ret = bool{}; - auto offset = size_t{}; - - while (buffer_size > 0) - { - size_t const bytes_needed = std::min(buffer_size, size_t{ 1024 }); - auto bytes_read = uint64_t{}; - ret = tr_sys_file_read(handle, buffer + offset, bytes_needed, &bytes_read, error); - - if (!ret || (offset == 0 && bytes_read == 0)) - { - ret = false; - break; - } - - TR_ASSERT(bytes_read <= bytes_needed); - TR_ASSERT(bytes_read <= buffer_size); - - int64_t delta = 0; - - for (size_t i = 0; i < bytes_read; ++i) - { - if (buffer[offset] == '\n') - { - delta = i - (int64_t)bytes_read + 1; - break; - } - - ++offset; - --buffer_size; - } - - if (delta != 0 || buffer_size == 0 || bytes_read == 0) - { - if (delta != 0) - { - ret = tr_sys_file_seek(handle, delta, TR_SEEK_CUR, nullptr, error); - - if (!ret) - { - break; - } - } - - if (offset > 0 && buffer[offset - 1] == '\r') - { - buffer[offset - 1] = '\0'; - } - else - { - buffer[offset] = '\0'; - } - - break; - } - } - - return ret; -} - bool tr_sys_file_write_line(tr_sys_file_t handle, std::string_view buffer, tr_error** error) { TR_ASSERT(handle != TR_BAD_SYS_FILE); diff --git a/libtransmission/file.h b/libtransmission/file.h index 99a34563c..2b72b9868 100644 --- a/libtransmission/file.h +++ b/libtransmission/file.h @@ -541,32 +541,6 @@ bool tr_sys_file_advise( */ bool tr_sys_file_preallocate(tr_sys_file_t handle, uint64_t size, int flags, struct tr_error** error = nullptr); -/** - * @brief Portability wrapper for `mmap()` for files. - * - * @param[in] handle Valid file descriptor. - * @param[in] offset Offset in file to map from. - * @param[in] size Number of bytes to map. - * @param[out] error Pointer to error object. Optional, pass `nullptr` if you - * are not interested in error details. - * - * @return Pointer to mapped file data on success, `nullptr` otherwise (with - * `error` set accordingly). - */ -void* tr_sys_file_map_for_reading(tr_sys_file_t handle, uint64_t offset, uint64_t size, struct tr_error** error = nullptr); - -/** - * @brief Portability wrapper for `munmap()` for files. - * - * @param[in] address Pointer to mapped file data. - * @param[in] size Size of mapped data in bytes. - * @param[out] error Pointer to error object. Optional, pass `nullptr` if you - * are not interested in error details. - * - * @return `True` on success, `false` otherwise (with `error` set accordingly). - */ -bool tr_sys_file_unmap(void const* address, uint64_t size, struct tr_error** error = nullptr); - /** * @brief Portability wrapper for `flock()`. * @@ -584,29 +558,6 @@ bool tr_sys_file_lock(tr_sys_file_t handle, int operation, struct tr_error** err /* File-related wrappers (utility) */ -/** - * @brief Portability wrapper for `fgets()`, removing EOL internally. - * - * Special care should be taken when reading from one of standard input streams - * (@ref tr_std_sys_file_t) since no UTF-8 conversion is currently being made. - * - * Reading from other streams (files, pipes) also leaves data untouched, so it - * should already be in UTF-8 encoding, or whichever else you expect. - * - * @param[in] handle Valid file descriptor. - * @param[out] buffer Buffer to store read zero-terminated string to. - * @param[in] buffer_size Buffer size in bytes, taking '\0' character into - * account. - * @param[out] error Pointer to error object. Optional, pass `nullptr` if - * you are not interested in error details. - * - * @return `True` on success, `false` otherwise (with `error` set accordingly). - * Note that `false` will also be returned in case of end of file; if - * you need to distinguish the two, check if `error` is `nullptr` - * afterwards. - */ -bool tr_sys_file_read_line(tr_sys_file_t handle, char* buffer, size_t buffer_size, struct tr_error** error = nullptr); - /** * @brief Portability wrapper for `fputs()`, appending EOL internally. * diff --git a/libtransmission/natpmp_local.h b/libtransmission/natpmp_local.h index fe4513230..91c437ace 100644 --- a/libtransmission/natpmp_local.h +++ b/libtransmission/natpmp_local.h @@ -22,7 +22,7 @@ class tr_natpmp public: tr_natpmp() { - natpmp_.s = TR_BAD_SOCKET; /* socket */ + natpmp_.s = static_cast(TR_BAD_SOCKET); } ~tr_natpmp() diff --git a/libtransmission/session.cc b/libtransmission/session.cc index dc7db6228..c82675a4d 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -2396,7 +2396,7 @@ size_t tr_blocklistSetContent(tr_session* session, char const* content_filename) auto const it = std::find_if( std::begin(src), std::end(src), - [&name](auto const& blocklist) { return tr_strvEndsWith(blocklist->getFilename(), name); }); + [&name](auto const& blocklist) { return tr_strvEndsWith(blocklist->filename(), name); }); BlocklistFile* b = nullptr; if (it == std::end(src)) diff --git a/tests/libtransmission/file-test.cc b/tests/libtransmission/file-test.cc index 73746e835..d0b0d74ce 100644 --- a/tests/libtransmission/file-test.cc +++ b/tests/libtransmission/file-test.cc @@ -1295,134 +1295,6 @@ TEST_F(FileTest, filePreallocate) tr_sys_path_remove(path1); } -TEST_F(FileTest, map) -{ - auto const test_dir = createTestDir(currentTestName()); - - auto const path1 = tr_pathbuf{ test_dir, "/a"sv }; - auto const contents = std::string{ "test" }; - createFileWithContents(path1, contents.data()); - - auto fd = tr_sys_file_open(path1, TR_SYS_FILE_READ | TR_SYS_FILE_WRITE, 0600); - - tr_error* err = nullptr; - auto map_len = contents.size(); - auto* view = static_cast(tr_sys_file_map_for_reading(fd, 0, map_len, &err)); - EXPECT_NE(nullptr, view); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_EQ(contents, std::string(view, map_len)); - -#ifdef HAVE_UNIFIED_BUFFER_CACHE - - auto const contents_2 = std::string{ "more" }; - auto n_written = uint64_t{}; - tr_sys_file_write_at(fd, contents_2.data(), contents_2.size(), 0, &n_written, &err); - EXPECT_EQ(map_len, contents_2.size()); - EXPECT_EQ(map_len, n_written); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_EQ(contents_2, std::string(view, map_len)); - -#endif - - EXPECT_TRUE(tr_sys_file_unmap(view, map_len, &err)); - EXPECT_EQ(nullptr, err) << *err; - - tr_sys_file_close(fd); - - tr_sys_path_remove(path1); -} - -TEST_F(FileTest, fileUtilities) -{ - auto const test_dir = createTestDir(currentTestName()); - - auto const path1 = tr_pathbuf{ test_dir, "/a"sv }; - auto const contents = std::string{ "a\nbc\r\ndef\nghij\r\n\n\nklmno\r" }; - createFileWithContents(path1, contents.data()); - - auto fd = tr_sys_file_open(path1, TR_SYS_FILE_READ, 0); - - tr_error* err = nullptr; - auto buffer = std::array{}; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("a", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("bc", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("def", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("ghij", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), 4, &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("klmn", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("o", buffer.data()); - EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("o", buffer.data()); // on EOF, buffer stays unchanged - - tr_sys_file_close(fd); - - fd = tr_sys_file_open(path1, TR_SYS_FILE_READ | TR_SYS_FILE_WRITE | TR_SYS_FILE_TRUNCATE, 0); - - EXPECT_TRUE(tr_sys_file_write_line(fd, "p", &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_TRUE(tr_sys_file_write_line(fd, "", &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_TRUE(tr_sys_file_write_line(fd, "qr", &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_TRUE(tr_sys_file_write_line(fd, "stu", &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_TRUE(tr_sys_file_write_line(fd, "", &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_TRUE(tr_sys_file_write_line(fd, "", &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_TRUE(tr_sys_file_write_line(fd, "vwxy2", &err)); - EXPECT_EQ(nullptr, err) << *err; - - tr_sys_file_seek(fd, 0, TR_SEEK_SET, nullptr); - - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("p", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("qr", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("stu", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("", buffer.data()); - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("vwxy2", buffer.data()); - EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), &err)); - EXPECT_EQ(nullptr, err) << *err; - EXPECT_STREQ("vwxy2", buffer.data()); // on EOF, buffer stays unchanged - - tr_sys_file_close(fd); - - tr_sys_path_remove(path1); -} - TEST_F(FileTest, dirCreate) { auto const test_dir = createTestDir(currentTestName()); diff --git a/tests/libtransmission/subprocess-test.cc b/tests/libtransmission/subprocess-test.cc index 0d3e68a06..9e239fcb4 100644 --- a/tests/libtransmission/subprocess-test.cc +++ b/tests/libtransmission/subprocess-test.cc @@ -14,6 +14,7 @@ #include "test-fixtures.h" #include +#include #include #include #include @@ -122,38 +123,26 @@ TEST_P(SubprocessTest, SpawnAsyncArgs) waitForFileToExist(result_path); - auto fd = tr_sys_file_open(result_path.data(), TR_SYS_FILE_READ, 0); // NOLINT - EXPECT_NE(TR_BAD_SYS_FILE, fd); + auto in = std::ifstream{ result_path, std::ios_base::in }; + EXPECT_TRUE(in.is_open()); - auto buffer = std::array{}; + auto line = std::string{}; + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_arg1, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - buffer.back() = '\0'; - EXPECT_EQ(test_arg1, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_arg2, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - buffer.back() = '\0'; - EXPECT_EQ(test_arg2, buffer.data()); - - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - buffer.back() = '\0'; - EXPECT_EQ(test_arg3, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_arg3, line); if (allow_batch_metachars) { - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - buffer.back() = '\0'; - EXPECT_EQ(test_arg4, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_arg4, line); } - EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - buffer.back() = '\0'; - - tr_sys_file_close(fd); + EXPECT_FALSE(std::getline(in, line)); } TEST_P(SubprocessTest, SpawnAsyncEnv) @@ -203,38 +192,29 @@ TEST_P(SubprocessTest, SpawnAsyncEnv) waitForFileToExist(result_path); - auto fd = tr_sys_file_open(result_path.data(), TR_SYS_FILE_READ, 0); // NOLINT - EXPECT_NE(TR_BAD_SYS_FILE, fd); + auto in = std::ifstream{ result_path, std::ios_base::in }; + EXPECT_TRUE(in.is_open()); - auto buffer = std::array{}; + auto line = std::string{}; + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_env_value1, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_EQ(test_env_value1, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_env_value2, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_EQ(test_env_value2, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_env_value3, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_EQ(test_env_value3, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_env_value4, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_EQ(test_env_value4, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(test_env_value5, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_EQ(test_env_value5, buffer.data()); + EXPECT_TRUE(std::getline(in, line)); + EXPECT_EQ(""sv, line); - buffer[0] = '\0'; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_STREQ("", buffer.data()); - - EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - - tr_sys_file_close(fd); + EXPECT_FALSE(std::getline(in, line)); } TEST_P(SubprocessTest, SpawnAsyncCwdExplicit) @@ -251,20 +231,18 @@ TEST_P(SubprocessTest, SpawnAsyncCwdExplicit) waitForFileToExist(result_path); - auto fd = tr_sys_file_open(result_path.data(), TR_SYS_FILE_READ, 0); // NOLINT - EXPECT_NE(TR_BAD_SYS_FILE, fd); + auto in = std::ifstream{ result_path, std::ios_base::in }; + EXPECT_TRUE(in.is_open()); - auto buffer = std::array{}; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); + auto line = std::string{}; + EXPECT_TRUE(std::getline(in, line)); auto expected = std::string{ test_dir }; tr_sys_path_native_separators(std::data(expected)); - auto actual = std::string{ std::data(buffer) }; + auto actual = line; tr_sys_path_native_separators(std::data(actual)); EXPECT_EQ(expected, actual); - EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - - tr_sys_file_close(fd); + EXPECT_FALSE(std::getline(in, line)); } TEST_P(SubprocessTest, SpawnAsyncCwdInherit) @@ -280,15 +258,17 @@ TEST_P(SubprocessTest, SpawnAsyncCwdInherit) EXPECT_EQ(nullptr, error) << *error; waitForFileToExist(result_path); - auto fd = tr_sys_file_open(result_path.data(), TR_SYS_FILE_READ, 0); // NOLINT - EXPECT_NE(TR_BAD_SYS_FILE, fd); - auto buffer = std::array{}; - EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); - EXPECT_EQ(expected_cwd, tr_sys_path_native_separators(&buffer.front())); - EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size())); + auto in = std::ifstream{ result_path, std::ios_base::in }; + EXPECT_TRUE(in.is_open()); - tr_sys_file_close(fd); + auto line = std::string{}; + EXPECT_TRUE(std::getline(in, line)); + auto actual = line; + tr_sys_path_native_separators(std::data(actual)); + EXPECT_EQ(expected_cwd, actual); + + EXPECT_FALSE(std::getline(in, line)); } TEST_P(SubprocessTest, SpawnAsyncCwdMissing)