Breaking: Remove openmode parameter from file construction (#218)

According to C99 Rationale, unnamed temporary files are expected to be written first and then read as transparently as possible, so they are created in binary update mode

> ### 7.19.4.3 The tmpfile function
> The tmpfile function is intended to allow users to create binary “scratch” files. The as if principle implies that the information in such a file need never actually be stored on a file-structured device.
> The temporary file is created in binary update mode because it will presumably be first written and then read as transparently as possible. Trailing null-character padding may cause problems for some existing programs.

https://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10.pdf

I don't see any point in implementing `tmp::file` in any other way, given that we haven't found a practical use for text or append modes in production for over a year now

Author: bugdea1er

README.md

@@ -51,7 +51,7 @@ auto func() {
 
 ### Temporary files
 
-`tmp::file` is a smart handle that manages a temporary file, ensuring its deletion when the handle goes out of scope. Upon creation, a `tmp::file` object generates a unique temporary file in the current user's temporary directory, opening it for both reading and writing.
+`tmp::file` is a smart handle that manages a binary temporary file, ensuring its deletion when the handle goes out of scope. Upon creation, a `tmp::file` object generates a unique temporary file, opening it for both reading and writing in binary format.
 
 The temporary file is deleted of when either of the following happens:
 - the `tmp::file` object is destroyed
@@ -65,7 +65,7 @@ The example below demonstrates a usage of a `tmp::file` object to validate a req
 #include <tmp/file>
 
 auto func(std::string_view content) {
-  auto tmpfile = tmp::file(std::ios::binary);
+  auto tmpfile = tmp::file();
   tmpfile << contents << std::flush;
   if (validate(tmpfile)) {
     // Unarchive the file to the persistent storage

include/tmp/file

@@ -13,12 +13,11 @@
 
 namespace tmp {
 
-/// tmp::file is a smart handle that manages a temporary file, ensuring
+/// tmp::file is a smart handle that manages a binary temporary file, ensuring
 /// its deletion when the handle goes out of scope
 ///
-/// Upon creation, a tmp::file object generates a unique temporary file
-/// in the current user's temporary directory, opening it for both
-/// reading and writing
+/// Upon creation, a tmp::file object generates a unique temporary file,
+/// opening it for reading and writing in binary format
 ///
 /// The temporary file is deleted of when either of the following happens:
 /// - the tmp::file object is destroyed
@@ -35,7 +34,7 @@ namespace tmp {
 ///   #include <tmp/file>
 ///
 ///   auto func(std::string_view content) {
-///     auto tmpfile = tmp::file(std::ios::binary);
+///     auto tmpfile = tmp::file();
 ///     tmpfile << contents << std::flush;
 ///     if (validate(tmpfile)) {
 ///       // Unarchive the file to the persistent storage
@@ -57,12 +56,9 @@ public:
 #error "Target platform not supported"
 #endif
 
-  /// Creates a unique file in the current user's temporary directory
-  /// and opens it with `mode | std::ios::in | std::ios::out`
-  /// @param mode The file opening mode
+  /// Creates and opens a binary temporary file as if by POSIX `tmpfile`
   /// @throws std::filesystem::filesystem_error if cannot create a file
-  /// @throws std::invalid_argument if the given open mode is invalid
-  explicit file(std::ios::openmode mode = std::ios::in | std::ios::out);
+  explicit file();
 
   /// Returns an implementation-defined handle to this file
   /// @returns The underlying implementation-defined handle

src/file.cpp

@@ -25,6 +25,11 @@ namespace {
 
 namespace fs = std::filesystem;
 
+#ifndef _MSC_VER
+/// Open mode for binary temporary files
+constexpr auto mode = std::ios::binary | std::ios::in | std::ios::out;
+#endif
+
 /// Returns an implementation-defined handle to the file
 /// @param[in] file The file to the native handle for
 /// @returns The underlying implementation-defined handle
@@ -36,118 +41,23 @@ file::native_handle_type get_native_handle(std::FILE* file) noexcept {
 #endif
 }
 
-/// Makes a mode string for opening a temporary file
-/// @param[in] mode The file opening mode
-/// @returns A suitable mode string
-const char* make_mdstring(std::ios::openmode mode) noexcept {
-  // Special case: the C++ standard forbids `app` and `trunc` at the same time
-  if ((mode & std::ios::app) != 0 && (mode & std::ios::trunc) != 0) {
-    return nullptr;
-  }
-
-  // - `std::ios::in` and `std::ios::out` are always applied
-  // - `std::ios::trunc` has no effect on the empty file
-  // - `std::ios::noreplace` has no effect for temporary files
-  // - any other platform dependent flag is not supported
-  unsigned filtered = mode & (std::ios::app | std::ios::binary);
-
-  switch (filtered) {
-  case 0:
-#ifdef _WIN32
-    return "w+TD";
-#else
-    return "w+";
-#endif
-  case std::ios::app:
-#ifdef _WIN32
-    return "a+TD";
-#else
-    return "a+";
-#endif
-  case std::ios::binary:
-#ifdef _WIN32
-    return "w+bTD";
-#else
-    return "w+b";
-#endif
-  case std::ios::app | std::ios::binary:
-#ifdef _WIN32
-    return "a+bTD";
-#else
-    return "a+b";
-#endif
-  default:
-    return nullptr;
-  }
-}
-
-/// Reopens the given temporary file with the given open mode
-/// @param[in]  mdstring The temporary file opening mode
-/// @param[out] file     The file to reopen
-/// @param[out] ec       Parameter for error reporting
-void reopen_file(const char* mdstring, std::FILE* file,
-                 std::error_code& ec) noexcept {
-  ec.clear();
-
-#ifdef _WIN32
-  HANDLE handle = get_native_handle(file);
-
-  std::string path;
-  path.resize(MAX_PATH);
-  DWORD ret = GetFinalPathNameByHandleA(handle, path.data(), MAX_PATH, 0);
-  if (ret == 0) {
-    ec.assign(GetLastError(), std::system_category());
-    return;
-  }
-
-  path.resize(ret);
-
-  // FIXME: freopen loses shared access rules
-  file = freopen(path.c_str(), mdstring, file);
-  if (file == nullptr) {
-    ec.assign(errno, std::generic_category());
-  }
-#else
-  file = std::freopen(nullptr, mdstring, file);
-  if (file == nullptr) {
-    ec.assign(errno, std::generic_category());
-  }
-#endif
-}
-
-/// Creates and opens a temporary file in the current user's temporary directory
-/// @param[in] mode The file opening mode
-/// @returns A handle to the created temporary file
+/// Creates and opens a binary temporary file as if by POSIX `tmpfile`
+/// @returns A pointer to the file stream associated with the temporary file
 /// @throws fs::filesystem_error if cannot create a temporary file
-/// @throws std::invalid_argument if the given openmode is invalid
-std::FILE* create_file(std::ios::openmode mode) {
-  const char* mdstring = make_mdstring(mode);
-  if (mdstring == nullptr) {
-    throw std::invalid_argument(
-        "Cannot create a temporary file: invalid openmode");
-  }
-
-  std::error_code ec;
+std::FILE* create_file() {
   std::FILE* file = std::tmpfile();
   if (file == nullptr) {
-    ec.assign(errno, std::generic_category());
-  } else {
-    reopen_file(mdstring, file, ec);
-  }
-
-  if (ec) {
+    std::error_code ec = std::error_code(errno, std::generic_category());
     throw fs::filesystem_error("Cannot create a temporary file", ec);
   }
 
   return file;
 }
 }    // namespace
 
-file::file(std::ios::openmode mode)
+file::file()
     : std::iostream(std::addressof(sb)),
-      underlying(create_file(mode), &std::fclose) {
-  mode |= std::ios::in | std::ios::out;
-
+      underlying(create_file(), &std::fclose) {
 #if defined(_MSC_VER)
   sb = std::filebuf(underlying.get());
 #elif defined(_LIBCPP_VERSION)
@@ -157,8 +67,8 @@ file::file(std::ios::openmode mode)
 #endif
 
   if (!sb.is_open()) {
-    throw std::invalid_argument(
-        "Cannot create a temporary file: invalid openmode");
+    std::error_code ec = std::make_error_code(std::io_errc::stream);
+    throw fs::filesystem_error("Cannot create a temporary file", ec);
   }
 }
 

tests/file.cpp

@@ -68,14 +68,8 @@ TEST(file, create_multiple) {
   EXPECT_NE(fst.native_handle(), snd.native_handle());
 }
 
-/// Tests error handling with invalid open mode
-TEST(file, create_invalid_openmode) {
-  // C++ standard forbids opening a filebuf with `trunc | app`
-  EXPECT_THROW(file(std::ios::trunc | std::ios::app), std::invalid_argument);
-}
-
-/// Tests file default open mode
-TEST(file, openmode_default) {
+/// Tests file open mode
+TEST(file, openmode) {
   file tmpfile = file();
   tmpfile << "Hello, World!" << std::flush;
   tmpfile.seekg(0);
@@ -86,28 +80,6 @@ TEST(file, openmode_default) {
   EXPECT_EQ(content, "Goodbye!orld!");
 }
 
-/// Tests file `app` open mode
-TEST(file, openmode_append) {
-  file tmpfile = file(std::ios::app);
-  tmpfile << "Hello, World!" << std::flush;
-  tmpfile.seekg(0);
-  tmpfile << "Goodbye!" << std::flush;
-
-  tmpfile.seekg(0);
-  std::string content = std::string(std::istreambuf_iterator(tmpfile), {});
-  EXPECT_EQ(content, "Hello, World!Goodbye!");
-}
-
-/// Tests that file adds std::ios::in and std::ios::out flags
-TEST(file, ios_flags) {
-  file tmpfile = file(std::ios::binary);
-  tmpfile << "Hello, world!" << std::flush;
-
-  tmpfile.seekg(0, std::ios::beg);
-  std::string content = std::string(std::istreambuf_iterator(tmpfile), {});
-  EXPECT_EQ(content, "Hello, world!");
-}
-
 /// Tests that destructor removes a file
 TEST(file, destructor) {
   file::native_handle_type handle;