commit 5e7561542faa3aaedb393525a646c82fb7811c7a from: Aleksey Ryndin date: Mon Sep 11 11:09:40 2023 UTC Refactoring: open file and its name together. commit - 4dc73fe178293aa9744ad4f7fd0ff1c93a306db7 commit + 5e7561542faa3aaedb393525a646c82fb7811c7a blob - 9417da2e6457b779766d062cf311ab409ea719fd blob + ee8dc1b8becc3072574095085fcf2f5d3654d66d --- tests/test_open_file.cc +++ tests/test_open_file.cc @@ -30,17 +30,19 @@ TEST_START(test_open_file) std::string file_name; { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = ""; - IS_TRUE(open_file(dir.get(), file_name , opened_file, opened_path) == FILE_NOT_FOUND and !opened_file); - + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::NOT_FOUND); + } + { + OpenedFile opened_file; file_name = "non-existent-file"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_NOT_FOUND and !opened_file); - + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::NOT_FOUND); + } + { + OpenedFile opened_file; file_name = "non-existent-dir/file"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_NOT_FOUND and !opened_file); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::NOT_FOUND); } const UniqueFd index_gmi{openat(dir.get(), "index.gmi", O_RDONLY | O_CREAT | O_EXCL, S_IRWXU)}; @@ -49,39 +51,36 @@ TEST_START(test_open_file) IS_TRUE_ERRNO(fstat(index_gmi.get(), &sb1) != -1, "Stat file " << ss.str().c_str() << "/index.gmi"); { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = ""; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_OPENED and opened_file); - IS_TRUE(*opened_path == "index.gmi"); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::OPENED); + IS_TRUE(opened_file.get_path() == "index.gmi"); struct stat sb2{}; - IS_TRUE_ERRNO(fstat(opened_file.get(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/index.gmi"); + IS_TRUE_ERRNO(fstat(opened_file.get_fd(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/index.gmi"); IS_TRUE((sb1.st_dev == sb2.st_dev) && (sb1.st_ino == sb2.st_ino)); } { - UniqueFd opened_file; - const std::string *opened_path = nullptr; + OpenedFile opened_file; file_name = "index.gmi"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_OPENED and opened_file); - IS_TRUE(*opened_path == "index.gmi"); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::OPENED); + IS_TRUE(opened_file.get_path() == "index.gmi"); struct stat sb2{}; - IS_TRUE_ERRNO(fstat(opened_file.get(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/index.gmi"); + IS_TRUE_ERRNO(fstat(opened_file.get_fd(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/index.gmi"); IS_TRUE((sb1.st_dev == sb2.st_dev) && (sb1.st_ino == sb2.st_ino)); } IS_TRUE_ERRNO(mkdirat(dir.get(), "subdir", S_IRWXU) != -1, "Create directory " << ss.str().c_str() << "/subdir"); { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = "subdir"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_NOT_FOUND and !opened_file); - + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::NOT_FOUND); + } + { + OpenedFile opened_file; file_name = "subdir/"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_NOT_FOUND and !opened_file); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::NOT_FOUND); } const UniqueFd subdir{openat(dir.get(), "subdir", O_RDONLY | O_DIRECTORY)}; IS_TRUE_ERRNO(subdir, "Open directory " << ss.str().c_str() << "/subdir"); @@ -91,38 +90,30 @@ TEST_START(test_open_file) IS_TRUE_ERRNO(fstat(sub_index_gmi.get(), &sb1) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = "subdir"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_OPENED and opened_file); - IS_TRUE(*opened_path == "index.gmi"); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::OPENED); + IS_TRUE(opened_file.get_path() == "index.gmi"); struct stat sb2{}; - IS_TRUE_ERRNO(fstat(opened_file.get(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); + IS_TRUE_ERRNO(fstat(opened_file.get_fd(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); IS_TRUE((sb1.st_dev == sb2.st_dev) && (sb1.st_ino == sb2.st_ino)); } - { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = "subdir/"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_OPENED and opened_file); - IS_TRUE(*opened_path == "index.gmi"); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::OPENED); + IS_TRUE(opened_file.get_path() == "index.gmi"); struct stat sb2{}; - IS_TRUE_ERRNO(fstat(opened_file.get(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); + IS_TRUE_ERRNO(fstat(opened_file.get_fd(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); IS_TRUE((sb1.st_dev == sb2.st_dev) && (sb1.st_ino == sb2.st_ino)); } - { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = "subdir/index.gmi"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_OPENED and opened_file); - IS_TRUE(*opened_path == "subdir/index.gmi"); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::OPENED); + IS_TRUE(opened_file.get_path() == "subdir/index.gmi"); struct stat sb2{}; - IS_TRUE_ERRNO(fstat(opened_file.get(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); + IS_TRUE_ERRNO(fstat(opened_file.get_fd(), &sb2) != -1, "Stat file " << ss.str().c_str() << "/subdir/index.gmi"); IS_TRUE((sb1.st_dev == sb2.st_dev) && (sb1.st_ino == sb2.st_ino)); } @@ -130,11 +121,9 @@ TEST_START(test_open_file) IS_TRUE_ERRNO(eperm, "Create file " << ss.str().c_str() << "/subdir/index.gmi"); { - UniqueFd opened_file; - const std::string *opened_path = nullptr; - + OpenedFile opened_file; file_name = "subdir/EPERM.gmi"; - IS_TRUE(open_file(dir.get(), file_name, opened_file, opened_path) == FILE_OPENING_ERROR and !opened_file); + IS_TRUE(opened_file.open(dir.get(), file_name) == OpenedFile::OPENING_ERROR); } IS_TRUE_ERRNO(unlinkat(subdir.get(), "EPERM.gmi", 0) != -1, "Remove file " << ss.str().c_str() << "/subdir/EPERM.gmi"); blob - 7e53ac30ac83be704bad5b3eb71ab4cf0ab4c44e blob + 3c2916fd4db1cae0ce47ab970207e3dac0fdeb62 --- vostok/open_file.cc +++ vostok/open_file.cc @@ -9,62 +9,71 @@ namespace vostok { -static const std::string index_gmi{"index.gmi"}; -OpenFileResult -open_file( +const std::string OpenedFile::s_index_gmi{"index.gmi"}; + + +OpenedFile::Result +OpenedFile::open( /* in */ int directory_fd, - /* in */ const std::string &request_path, - /* out */ UniqueFd &opened_file, - /* out */ const std::string* &opened_path + /* in/out */ std::string &request_path ) { - opened_path = request_path.empty() ? &index_gmi : &request_path; - opened_file.reset(openat(directory_fd, opened_path->c_str(), O_RDONLY)); - if (!opened_file) + if (request_path.empty()) { + m_path = &s_index_gmi; + } + else + { + m_owner.swap(request_path); + m_path = &m_owner; + } + + m_fd.reset(openat(directory_fd, m_path->c_str(), O_RDONLY)); + if (!m_fd) + { const auto error_code = errno; error::occurred( - [&opened_path] + [this] { - error::g_log << "Open file \"" << opened_path->c_str() << "\""; + error::g_log << "Open file \"" << *this->m_path << "\""; }, error::Print{error_code} ); - return (error_code == ENOENT) ? FILE_NOT_FOUND : FILE_OPENING_ERROR; + return (error_code == ENOENT) ? NOT_FOUND : OPENING_ERROR; } struct stat sb{}; - if (fstat(opened_file.get(), &sb) == -1) + if (fstat(m_fd.get(), &sb) == -1) { error::occurred( - [&opened_path] + [this] { - error::g_log << "Stat file \"" << opened_path->c_str() << "\""; + error::g_log << "Stat file \"" << *this->m_path << "\""; }, error::Print{} ); - return FILE_OPENING_ERROR; + return OPENING_ERROR; } if (S_ISDIR(sb.st_mode)) { - const UniqueFd new_parent{opened_file.release()}; - opened_path = &index_gmi; - opened_file.reset(openat(new_parent.get(), opened_path->c_str(), O_RDONLY)); - if (!opened_file) + const UniqueFd new_parent{m_fd.release()}; + m_path = &s_index_gmi; + m_fd.reset(openat(new_parent.get(), m_path->c_str(), O_RDONLY)); + if (!m_fd) { const auto error_code = errno; error::occurred( - [&opened_path] + [this] { - error::g_log << "Open file \"" << opened_path->c_str() << "\" /" << index_gmi; + error::g_log << "Re-open file " << this->m_path->c_str(); }, error::Print{error_code} ); - return (error_code == ENOENT) ? FILE_NOT_FOUND : FILE_OPENING_ERROR; + return (error_code == ENOENT) ? NOT_FOUND : OPENING_ERROR; } } - return FILE_OPENED; + return OPENED; } blob - 3b9c2eb7ae75aafc8380763b767c4323b8f75ee5 blob + 543937f216bb3e053c9f15abfb8ff9a848595c18 --- vostok/open_file.h +++ vostok/open_file.h @@ -9,21 +9,36 @@ namespace vostok { -enum OpenFileResult +class OpenedFile { - FILE_OPENED, +public: + /** Gettters: call after method open(...) has returned OPENED */ + int get_fd() const { return m_fd.get(); } + const std::string &get_path() const{ assert(m_path); return *m_path; } - FILE_NOT_FOUND, - FILE_OPENING_ERROR, -}; -OpenFileResult -open_file( - /* in */ int directory_fd, - /* in */ const std::string &request_path, - /* out */ UniqueFd &opened_file, - /* out */ const std::string* &opened_path -); + /** Open file */ + enum Result + { + OPENED, + NOT_FOUND, + OPENING_ERROR, + }; + Result + open( + /* in */ int directory_fd, + /* in/out */ std::string &request_path // move ownership + ); + +private: + UniqueFd m_fd; + const std::string *m_path{nullptr}; + + std::string m_owner; + static const std::string s_index_gmi; +}; + + } // namespace vostok blob - cc2dda64753c42de52c31b28c53ec3385379e310 blob + 22377af14c3007012e8fa2ff78e237365c6fa08e --- vostok/vostok.cc +++ vostok/vostok.cc @@ -85,25 +85,23 @@ void client_thread(const transport::AcceptedClient *ac break; } - UniqueFd opened_file{}; - const std::string *opened_path = nullptr; - const auto open_file_result = open_file(directory_fd, path, opened_file, opened_path); + OpenedFile opened_file; + const auto open_file_result = opened_file.open(directory_fd, path); switch (open_file_result) { - case FILE_NOT_FOUND: + case OpenedFile::NOT_FOUND: send_response(accepted_client->get_ctx(), gemini::STATUS_51_NOT_FOUND, meta::NOT_FOUND); return; - case FILE_OPENING_ERROR: + case OpenedFile::OPENING_ERROR: send_response(accepted_client->get_ctx(), gemini::STATUS_40_TEMPORARY_FAILURE, meta::TEMPORARY_FAILURE); return; - case FILE_OPENED: - assert(opened_file && opened_path); + case OpenedFile::OPENED: break; } // > If is an empty string, the MIME type MUST default to "text/gemini; charset=utf-8". - const auto mime_type = mime.lookup(*opened_path); + const auto mime_type = mime.lookup(opened_file.get_path()); send_response( accepted_client->get_ctx(), gemini::STATUS_20_SUCCESS, @@ -114,13 +112,13 @@ void client_thread(const transport::AcceptedClient *ac buffer.resize(64 * 1024); for (; ; ) { - const auto ret = read(opened_file.get(), buffer.data(), buffer.size()); + const auto ret = read(opened_file.get_fd(), buffer.data(), buffer.size()); if (ret == -1) { error::occurred( - [&path] + [&opened_file] { - error::g_log << "Read file \"" << path.c_str() << "\""; + error::g_log << "Read file \"" << opened_file.get_path() << "\""; }, error::Print{} ); @@ -135,7 +133,7 @@ void client_thread(const transport::AcceptedClient *ac if (readed < buffer.size()) break; } - error::g_log << "20 " << (mime_type ? *mime_type : meta::_EMPTY) << " \"" << path.c_str() << "\"" << std::endl; + error::g_log << "20 " << (mime_type ? *mime_type : meta::_EMPTY) << " \"" << opened_file.get_path() << "\"" << std::endl; }