From 4df2daad3466db26b94626434665fd23be3035a9 Mon Sep 17 00:00:00 2001 From: bugdea1er Date: Sun, 4 May 2025 21:59:21 +0300 Subject: [PATCH 1/5] Implement `file_handle` for auto-closing an open file --- src/file.cpp | 115 +++++++++++++++++++++++---------------------------- 1 file changed, 52 insertions(+), 63 deletions(-) diff --git a/src/file.cpp b/src/file.cpp index 34fcffaa..a61e940c 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -34,79 +34,69 @@ namespace tmp { namespace { -/// Implementation-defined invalid handle to the file +class file_handle { +public: + file_handle(const fs::path& path, std::error_code& ec) { #ifdef _WIN32 -const file::native_handle_type invalid_handle = nullptr; + handle = CreateFile(path.c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + if (handle == INVALID_HANDLE_VALUE) { + ec = std::error_code(GetLastError(), std::system_category()); + return; + } + + BY_HANDLE_FILE_INFORMATION handle_info; + if (GetFileInformationByHandle(handle, &handle_info) == 0) { + ec = std::error_code(GetLastError(), std::system_category()); + return; + } + + // Based on Microsoft's C++ STL implementation: If a file is not a reparse + // point and is not a directory, then it is considered a regular file + // https://github.com/microsoft/STL/blob/main/stl/inc/filesystem#L1982 + if ((handle_info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 || + (handle_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0) { + ec = std::make_error_code(std::errc::not_supported); + return; + } #else -const file::native_handle_type invalid_handle = -1; + handle = open(path.c_str(), O_RDONLY | O_NONBLOCK); + if (handle == -1) { + ec = std::error_code(errno, std::system_category()); + return; + } + + struct stat file_stat; + if (fstat(handle, &file_stat) == -1) { + ec = std::error_code(errno, std::system_category()); + return; + } + + if ((file_stat.st_mode & S_IFMT) != S_IFREG) { + ec = std::make_error_code(std::errc::not_supported); + return; + } #endif -/// Opens a file for reading and returns its handle -/// @param[in] path The path to the file to open -/// @param[out] ec Parameter for error reporting -/// @returns A handle to the open file -file::native_handle_type open_file(const fs::path& path, - std::error_code& ec) noexcept { -#ifdef _WIN32 - HANDLE handle = - CreateFile(path.c_str(), GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); - if (handle == INVALID_HANDLE_VALUE) { - ec = std::error_code(GetLastError(), std::system_category()); - return invalid_handle; + ec.clear(); } - BY_HANDLE_FILE_INFORMATION handle_info; - if (GetFileInformationByHandle(handle, &handle_info) == 0) { - ec = std::error_code(GetLastError(), std::system_category()); - CloseHandle(handle); - return invalid_handle; + operator file::native_handle_type() const noexcept { + return handle; } - // Based on Microsoft's C++ STL implementation: If a file is not a reparse - // point and is not a directory, then it is considered a regular file - // https://github.com/microsoft/STL/blob/main/stl/inc/filesystem#L1982 - if ((handle_info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 || - (handle_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0) { - ec = std::make_error_code(std::errc::not_supported); + ~file_handle() noexcept { +#ifdef _WIN32 CloseHandle(handle); - return invalid_handle; - } #else - int handle = open(path.c_str(), O_RDONLY | O_NONBLOCK); - if (handle == -1) { - ec = std::error_code(errno, std::system_category()); - return invalid_handle; - } - - struct stat file_stat; - if (fstat(handle, &file_stat) == -1) { - ec = std::error_code(errno, std::system_category()); close(handle); - return invalid_handle; - } - - if ((file_stat.st_mode & S_IFMT) != S_IFREG) { - ec = std::make_error_code(std::errc::not_supported); - close(handle); - return invalid_handle; - } #endif + } - ec.clear(); - return handle; -} - -/// Closes the given handle, ignoring any errors -/// @param[in] handle The handle to close -void close_file(file::native_handle_type handle) noexcept { -#ifdef _WIN32 - CloseHandle(handle); -#else - close(handle); -#endif -} +private: + file::native_handle_type handle; +}; #if __has_include() /// Copies file contents from one file descriptor to another @@ -215,7 +205,7 @@ file::file(std::ios::openmode mode) if (!sb.is_open()) { #ifndef _WIN32 - close_file(handle); + close(handle); #endif throw std::invalid_argument( "Cannot create a temporary file: invalid openmode"); @@ -226,10 +216,9 @@ file file::copy(const fs::path& path, std::ios::openmode mode) { file tmpfile = file(mode); std::error_code ec; - native_handle_type source = open_file(path, ec); + file_handle source = file_handle(path, ec); if (!ec) { copy_file(source, tmpfile.native_handle(), ec); - close_file(source); } if (ec) { From 5caf80c1a64d2824564c5587761fa049cf42ee5c Mon Sep 17 00:00:00 2001 From: bugdea1er Date: Sun, 4 May 2025 22:06:31 +0300 Subject: [PATCH 2/5] Add docs --- src/file.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/file.cpp b/src/file.cpp index a61e940c..8d144f3f 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -34,8 +34,13 @@ namespace tmp { namespace { +/// RAII wrapper for native open file descriptor +/// @note closes the file in destructor class file_handle { public: + /// Opens a file for reading + /// @param[in] path The path to the file to open + /// @param[out] ec Parameter for error reporting file_handle(const fs::path& path, std::error_code& ec) { #ifdef _WIN32 handle = CreateFile(path.c_str(), GENERIC_READ, @@ -82,10 +87,12 @@ class file_handle { ec.clear(); } + /// Returns the underlying file handle operator file::native_handle_type() const noexcept { return handle; } + /// Closes the file, ignoring any errors ~file_handle() noexcept { #ifdef _WIN32 CloseHandle(handle); @@ -95,6 +102,7 @@ class file_handle { } private: + /// Returns the underlying file handle file::native_handle_type handle; }; From d14cebcc3c1492f39d6f2f4f4f08ee8a44f90dbc Mon Sep 17 00:00:00 2001 From: bugdea1er Date: Mon, 5 May 2025 23:01:19 +0300 Subject: [PATCH 3/5] Fix clang-tidy warnings --- src/file.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/file.cpp b/src/file.cpp index 8d144f3f..ca5bc326 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -41,11 +41,17 @@ class file_handle { /// Opens a file for reading /// @param[in] path The path to the file to open /// @param[out] ec Parameter for error reporting - file_handle(const fs::path& path, std::error_code& ec) { + file_handle(const fs::path& path, std::error_code& ec) +#ifdef _WIN32 + : handle( + CreateFile(path.c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)) +#else + : handle(open(path.c_str(), O_RDONLY | O_NONBLOCK)) +#endif + { #ifdef _WIN32 - handle = CreateFile(path.c_str(), GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); if (handle == INVALID_HANDLE_VALUE) { ec = std::error_code(GetLastError(), std::system_category()); return; @@ -66,7 +72,6 @@ class file_handle { return; } #else - handle = open(path.c_str(), O_RDONLY | O_NONBLOCK); if (handle == -1) { ec = std::error_code(errno, std::system_category()); return; @@ -101,6 +106,11 @@ class file_handle { #endif } + file_handle(file_handle&&) noexcept = default; + file_handle& operator=(file_handle&&) = default; + file_handle(const file_handle&) = delete; + file_handle& operator=(const file_handle&) = delete; + private: /// Returns the underlying file handle file::native_handle_type handle; From 98aaba121b2d7e480d87d6d295db5a4526790140 Mon Sep 17 00:00:00 2001 From: bugdea1er Date: Mon, 5 May 2025 23:07:15 +0300 Subject: [PATCH 4/5] Delete `file_handle` move semantics --- src/file.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/file.cpp b/src/file.cpp index ca5bc326..7873efc6 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -106,8 +106,8 @@ class file_handle { #endif } - file_handle(file_handle&&) noexcept = default; - file_handle& operator=(file_handle&&) = default; + file_handle(file_handle&&) noexcept = delete; + file_handle& operator=(file_handle&&) = delete; file_handle(const file_handle&) = delete; file_handle& operator=(const file_handle&) = delete; From 5093ddb3aa555f2f8168fd1dd2a59de127981ae3 Mon Sep 17 00:00:00 2001 From: bugdea1er Date: Tue, 6 May 2025 11:23:41 +0300 Subject: [PATCH 5/5] Check for the handle validity before closing it --- src/file.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/file.cpp b/src/file.cpp index 7873efc6..3bdf801d 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -100,9 +100,13 @@ class file_handle { /// Closes the file, ignoring any errors ~file_handle() noexcept { #ifdef _WIN32 - CloseHandle(handle); + if (handle != INVALID_HANDLE_VALUE) { + CloseHandle(handle); + } #else - close(handle); + if (handle != 1) { + close(handle); + } #endif }