Breaking: Remove file::path() method (#176)

This pr makes removes `file` inheritance from `entry` and removes file's
access to its path.

- On POSIX systems, the path is unlinked immediately after file creation
- On Windows, the flag "delete on close" is used

This replicates implementations of C `std::tmpfile`

Author: bugdea1er

include/tmp/file

@@ -44,7 +44,7 @@ namespace tmp {
 ///     // `tmp::file` object goes out of scope and is destroyed
 ///   }
 /// @endcode
-class TMP_EXPORT file : public entry, public std::iostream {
+class TMP_EXPORT file : public std::iostream {
 public:
   /// Creates a unique temporary file and opens it for reading and writing
   /// @note std::ios::in | std::ios::out are always added to the `mode`
@@ -87,15 +87,12 @@ public:
 private:
   /// The underlying raw file device object
   filebuf sb;
-
-  /// Creates a unique temporary file
-  /// @param handle A path to the created temporary file and a handle to it
-  explicit file(std::pair<std::filesystem::path, filebuf> handle) noexcept
-      TMP_NO_EXPORT;
 };
 }    // namespace tmp
 
 /// The template specialization of `std::hash` for `tmp::file`
-template<> struct std::hash<tmp::file> : std::hash<tmp::entry> {};
+template<> struct TMP_EXPORT std::hash<tmp::file> {
+  std::size_t operator()(const tmp::file& file) const noexcept;
+};
 
 #endif    // TMP_FILE_H

src/create.cpp

@@ -3,6 +3,7 @@
 #include <tmp/filebuf>
 
 #include <filesystem>
+#include <iostream>
 #include <stdexcept>
 #include <string_view>
 #include <system_error>
@@ -85,32 +86,32 @@ const wchar_t* make_mdstring(std::ios::openmode mode) noexcept {
   switch (mode & ~std::ios::ate) {
   case std::ios::out:
   case std::ios::out | std::ios::trunc:
-    return L"wx";
+    return L"wxD";
   case std::ios::out | std::ios::app:
   case std::ios::app:
-    return L"a";
+    return L"aD";
   case std::ios::in:
-    return L"r";
+    return L"rD";
   case std::ios::in | std::ios::out:
   case std::ios::in | std::ios::out | std::ios::trunc:
-    return L"w+x";
+    return L"w+xD";
   case std::ios::in | std::ios::out | std::ios::app:
   case std::ios::in | std::ios::app:
-    return L"a+";
+    return L"a+D";
   case std::ios::out | std::ios::binary:
   case std::ios::out | std::ios::trunc | std::ios::binary:
-    return L"wbx";
+    return L"wbxD";
   case std::ios::out | std::ios::app | std::ios::binary:
   case std::ios::app | std::ios::binary:
-    return L"ab";
+    return L"abD";
   case std::ios::in | std::ios::binary:
-    return L"rb";
+    return L"rbD";
   case std::ios::in | std::ios::out | std::ios::binary:
   case std::ios::in | std::ios::out | std::ios::trunc | std::ios::binary:
-    return L"w+bx";
+    return L"w+bxD";
   case std::ios::in | std::ios::out | std::ios::app | std::ios::binary:
   case std::ios::in | std::ios::app | std::ios::binary:
-    return L"a+b";
+    return L"a+bD";
   default:
     return nullptr;
   }
@@ -128,9 +129,9 @@ void close(filebuf::open_handle_type handle) noexcept {
 }
 }    // namespace
 
-std::pair<fs::path, filebuf> create_file(std::ios::openmode mode) {
+filebuf create_file(std::ios::openmode mode) {
   std::error_code ec;
-  std::pair<fs::path, filebuf> file = create_file(mode, ec);
+  filebuf file = create_file(mode, ec);
 
   if (ec) {
     throw fs::filesystem_error("Cannot create a temporary file", ec);
@@ -139,8 +140,7 @@ std::pair<fs::path, filebuf> create_file(std::ios::openmode mode) {
   return file;
 }
 
-std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
-                                         std::error_code& ec) {
+filebuf create_file(std::ios::openmode mode, std::error_code& ec) {
 #ifdef _WIN32
   fs::path::string_type path = make_path("");
 #else
@@ -169,6 +169,8 @@ std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
     ec = std::error_code(errno, std::system_category());
     return {};
   }
+
+  unlink(path.c_str());
 #endif
 
   filebuf filebuf;
@@ -180,7 +182,7 @@ std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
   }
 
   ec.clear();
-  return std::make_pair(path, std::move(filebuf));
+  return filebuf;
 }
 
 fs::path create_directory(std::string_view prefix) {

src/create.hpp

@@ -15,17 +15,16 @@ namespace fs = std::filesystem;
 /// Creates a temporary file in the system's temporary directory,
 /// and opens it for reading and writing
 /// @param[in] mode Specifies stream open mode
-/// @returns A path to the created temporary file and a handle to it
+/// @returns A handle to the created temporary file
 /// @throws fs::filesystem_error  if cannot create a temporary file
-std::pair<fs::path, filebuf> create_file(std::ios::openmode mode);
+filebuf create_file(std::ios::openmode mode);
 
-/// Creates a temporary file  in the system's temporary directory,
+/// Creates a temporary file in the system's temporary directory,
 /// and opens it for reading and writing
 /// @param[in]  mode Specifies stream open mode
 /// @param[out] ec   Parameter for error reporting
-/// @returns A path to the created temporary file and a handle to it
-std::pair<fs::path, filebuf> create_file(std::ios::openmode mode,
-                                         std::error_code& ec);
+/// @returns A handle to the created temporary file
+filebuf create_file(std::ios::openmode mode, std::error_code& ec);
 
 /// Creates a temporary directory with the given prefix in the system's
 /// temporary directory

src/file.cpp

@@ -54,8 +54,9 @@ file::native_handle_type open(const fs::path& path, bool readonly,
     ec = std::error_code(GetLastError(), std::system_category());
   }
 #else
+  constexpr mode_t mode = 0644;
   int oflag = readonly ? O_RDONLY | O_NONBLOCK : O_RDWR | O_TRUNC | O_CREAT;
-  int handle = ::open(path.c_str(), oflag);
+  int handle = ::open(path.c_str(), oflag, mode);
   if (handle == -1) {
     ec = std::error_code(errno, std::system_category());
   }
@@ -143,13 +144,9 @@ void copy_file(file::native_handle_type from, const fs::path& to,
 }
 }    // namespace
 
-file::file(std::pair<fs::path, filebuf> handle) noexcept
-    : entry(std::move(handle.first)),
-      std::iostream(std::addressof(sb)),
-      sb(std::move(handle.second)) {}
-
 file::file(std::ios::openmode mode)
-    : file(create_file(mode)) {}
+    : std::iostream(std::addressof(sb)),
+      sb(create_file(mode)) {}
 
 file file::copy(const fs::path& path, std::ios::openmode mode) {
   file tmpfile = file(mode);
@@ -180,31 +177,22 @@ void file::move(const fs::path& to) {
   if (ec) {
     throw fs::filesystem_error("Cannot move a temporary file", to, ec);
   }
-
-  entry::clear();
 }
 
-file::~file() noexcept {
-  sb.close();
-}
+file::~file() noexcept = default;
 
 // NOLINTBEGIN(*-use-after-move)
 file::file(file&& other) noexcept
-    : entry(std::move(other)),
-      std::iostream(std::move(other)),
+    : std::iostream(std::move(other)),
       sb(std::move(other.sb)) {
   set_rdbuf(std::addressof(sb));
 }
+// NOLINTEND(*-use-after-move)
 
-file& file::operator=(file&& other) {
-  std::iostream::operator=(std::move(other));
-
-  // The stream buffer must be assigned first to close the file;
-  // otherwise `entry` may not be able to remove the file before reassigning
-  sb = std::move(other.sb);
-  entry::operator=(std::move(other));
+file& file::operator=(file&& other) = default;
+}    // namespace tmp
 
-  return *this;
+std::size_t
+std::hash<tmp::file>::operator()(const tmp::file& file) const noexcept {
+  return std::hash<tmp::file::native_handle_type>()(file.native_handle());
 }
-// NOLINTEND(*-use-after-move)
-}    // namespace tmp

tests/directory.cpp

@@ -1,5 +1,4 @@
 #include <tmp/directory>
-#include <tmp/file>
 
 #include <gtest/gtest.h>
 
@@ -90,8 +89,8 @@ TEST(directory, copy_directory) {
 
 /// Tests creation of a temporary copy of a file
 TEST(directory, copy_file) {
-  file tmpfile = file();
-  EXPECT_THROW(directory::copy(tmpfile), fs::filesystem_error);
+  std::ofstream("existing.txt", std::ios::binary) << "Hello, world!";
+  EXPECT_THROW(directory::copy("existing.txt"), fs::filesystem_error);
 }
 
 /// Tests `operator/` of directory

tests/file.cpp

@@ -54,30 +54,18 @@ TEST(file, type_traits) {
 /// Tests file creation
 TEST(file, create) {
   file tmpfile = file();
-  fs::path parent = tmpfile.path().parent_path();
-
-  EXPECT_TRUE(fs::exists(tmpfile));
-  EXPECT_TRUE(fs::is_regular_file(tmpfile));
-  EXPECT_TRUE(fs::equivalent(parent, fs::temp_directory_path()));
   EXPECT_TRUE(is_open(tmpfile));
   EXPECT_TRUE(is_open(tmpfile.native_handle()));
 
-  fs::perms permissions = fs::status(tmpfile).permissions();
-#ifdef _WIN32
-  // GetTempFileNameW creates a file with all permissions
-  EXPECT_EQ(permissions, fs::perms::all);
-#else
-  // mkstemp creates a file that can only be read and written by the owner
-  EXPECT_EQ(permissions, fs::perms::owner_read | fs::perms::owner_write);
-#endif
+  // TODO: test that the file is on the same filesystem as temp_directory_path
 }
 
 /// Tests multiple file creation
 TEST(file, create_multiple) {
   file fst = file();
   file snd = file();
 
-  EXPECT_FALSE(fs::equivalent(fst, snd));
+  EXPECT_NE(fst.native_handle(), snd.native_handle());
 }
 
 /// Tests error handling with invalid open mode
@@ -91,22 +79,22 @@ TEST(file, ios_flags) {
   file tmpfile = file(std::ios::binary);
   tmpfile << "Hello, world!" << std::flush;
 
-  std::ifstream stream = std::ifstream(tmpfile.path());
-  std::string content = std::string(std::istreambuf_iterator(stream), {});
+  tmpfile.seekg(0, std::ios::beg);
+  std::string content = std::string(std::istreambuf_iterator(tmpfile), {});
   EXPECT_EQ(content, "Hello, world!");
 }
 
 /// Tests creation of a temporary copy of a file
 TEST(file, copy_file) {
-  file tmpfile = file();
-  tmpfile << "Hello, world!" << std::flush;
+  std::ofstream original = std::ofstream("existing.txt", std::ios::binary);
+  original << "Hello, world!" << std::flush;
 
-  file copy = file::copy(tmpfile);
-  EXPECT_TRUE(fs::exists(tmpfile));
-  EXPECT_TRUE(fs::exists(copy));
-  EXPECT_FALSE(fs::equivalent(tmpfile, copy));
+  file copy = file::copy("existing.txt");
+  EXPECT_TRUE(fs::exists("existing.txt"));
+  EXPECT_TRUE(is_open(copy));
 
-  EXPECT_TRUE(fs::is_regular_file(tmpfile));
+  // To test that we actually copy the original file
+  original << "Goodbye, world!" << std::flush;
 
   // Test get file pointer position after copying
   std::streampos gstreampos = copy.tellg();
@@ -119,8 +107,8 @@ TEST(file, copy_file) {
   EXPECT_EQ(pstreampos, copy.tellp());
 
   // Test file copy contents
-  std::ifstream stream = std::ifstream(copy.path());
-  std::string content = std::string(std::istreambuf_iterator(stream), {});
+  copy.seekg(0, std::ios::beg);
+  std::string content = std::string(std::istreambuf_iterator(copy), {});
   EXPECT_EQ(content, "Hello, world!");
 }
 
@@ -179,16 +167,13 @@ TEST(file, move_to_non_existing_directory) {
 
 /// Tests that destructor removes a file
 TEST(file, destructor) {
-  fs::path path;
   file::native_handle_type handle;
 
   {
     file tmpfile = file();
-    path = tmpfile;
     handle = tmpfile.native_handle();
   }
 
-  EXPECT_FALSE(fs::exists(path));
   EXPECT_FALSE(is_open(handle));
 }
 
@@ -199,8 +184,6 @@ TEST(file, move_constructor) {
 
   file snd = file(std::move(fst));
 
-  EXPECT_FALSE(snd.path().empty());
-  EXPECT_TRUE(fs::exists(snd));
   EXPECT_TRUE(is_open(snd));
 
   snd.seekg(0);
@@ -217,19 +200,17 @@ TEST(file, move_assignment) {
     file snd = file();
     snd << "Hello!";
 
-    fs::path path1 = fst;
-    fs::path path2 = snd;
+    file::native_handle_type fst_handle = fst.native_handle();
+    file::native_handle_type snd_handle = snd.native_handle();
 
     fst = std::move(snd);
 
-    EXPECT_FALSE(fs::exists(path1));
-    EXPECT_TRUE(fs::exists(path2));
-
-    EXPECT_TRUE(fs::exists(fst));
-    EXPECT_TRUE(fs::equivalent(fst, path2));
+    EXPECT_FALSE(is_open(fst_handle));
+    EXPECT_TRUE(is_open(snd_handle));
+    EXPECT_EQ(fst.native_handle(), snd_handle);
   }
 
-  EXPECT_FALSE(fst.path().empty());
+  EXPECT_TRUE(is_open(fst));
 
   fst.seekg(0);
   std::string content;
@@ -242,13 +223,13 @@ TEST(file, swap) {
   file fst = file();
   file snd = file();
 
-  fs::path fst_path = fst.path();
-  fs::path snd_path = snd.path();
+  file::native_handle_type fst_handle = fst.native_handle();
+  file::native_handle_type snd_handle = snd.native_handle();
 
   std::swap(fst, snd);
 
-  EXPECT_EQ(fst.path(), snd_path);
-  EXPECT_EQ(snd.path(), fst_path);
+  EXPECT_EQ(fst.native_handle(), snd_handle);
+  EXPECT_EQ(snd.native_handle(), fst_handle);
   EXPECT_TRUE(is_open(fst));
   EXPECT_TRUE(is_open(snd));
 }
@@ -258,15 +239,8 @@ TEST(file, hash) {
   file tmpfile = file();
   std::hash hash = std::hash<file>();
 
-  EXPECT_EQ(hash(tmpfile), fs::hash_value(tmpfile.path()));
-}
-
-/// Tests file relational operators
-TEST(file, relational) {
-  file tmpfile = file();
-
-  EXPECT_TRUE(tmpfile == tmpfile);
-  EXPECT_FALSE(tmpfile < tmpfile);
+  EXPECT_EQ(hash(tmpfile),
+            std::hash<file::native_handle_type>()(tmpfile.native_handle()));
 }
 }    // namespace
 }    // namespace tmp