diff --git a/include/fmt/os.h b/include/fmt/os.h index 3c7b3ccb..4ddcd9ce 100644 --- a/include/fmt/os.h +++ b/include/fmt/os.h @@ -238,6 +238,7 @@ class buffered_file { }; #if FMT_USE_FCNTL + // A file. Closed file is represented by a file object with descriptor -1. // Methods that are not declared with noexcept may throw // fmt::system_error in case of failure. Note that some errors such as @@ -251,6 +252,8 @@ class FMT_API file { // Constructs a file object with a given descriptor. explicit file(int fd) : fd_(fd) {} + friend struct pipe; + public: // Possible values for the oflag argument to the constructor. enum { @@ -313,11 +316,6 @@ class FMT_API file { // necessary. void dup2(int fd, std::error_code& ec) noexcept; - // Creates a pipe setting up read_end and write_end file objects for reading - // and writing respectively. - // DEPRECATED! Taking files as out parameters is deprecated. - static void pipe(file& read_end, file& write_end); - // Creates a buffered_file object associated with this file and detaches // this file object from the file. auto fdopen(const char* mode) -> buffered_file; @@ -329,6 +327,15 @@ class FMT_API file { # endif }; +struct FMT_API pipe { + file read_end; + file write_end; + + // Creates a pipe setting up read_end and write_end file objects for reading + // and writing respectively. + pipe(); +}; + // Returns the memory page size. auto getpagesize() -> long; diff --git a/src/os.cc b/src/os.cc index a639e78c..4b277a1d 100644 --- a/src/os.cc +++ b/src/os.cc @@ -200,6 +200,7 @@ int buffered_file::descriptor() const { # ifdef _WIN32 using mode_t = int; # endif + constexpr mode_t default_open_mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; @@ -301,29 +302,6 @@ void file::dup2(int fd, std::error_code& ec) noexcept { if (result == -1) ec = std::error_code(errno, std::generic_category()); } -void file::pipe(file& read_end, file& write_end) { - // Close the descriptors first to make sure that assignments don't throw - // and there are no leaks. - read_end.close(); - write_end.close(); - int fds[2] = {}; -# ifdef _WIN32 - // Make the default pipe capacity same as on Linux 2.6.11+. - enum { DEFAULT_CAPACITY = 65536 }; - int result = FMT_POSIX_CALL(pipe(fds, DEFAULT_CAPACITY, _O_BINARY)); -# else - // Don't retry as the pipe function doesn't return EINTR. - // http://pubs.opengroup.org/onlinepubs/009696799/functions/pipe.html - int result = FMT_POSIX_CALL(pipe(fds)); -# endif - if (result != 0) - FMT_THROW(system_error(errno, FMT_STRING("cannot create pipe"))); - // The following assignments don't throw because read_fd and write_fd - // are closed. - read_end = file(fds[0]); - write_end = file(fds[1]); -} - buffered_file file::fdopen(const char* mode) { // Don't retry as fdopen doesn't return EINTR. # if defined(__MINGW32__) && defined(_POSIX_) @@ -352,6 +330,24 @@ file file::open_windows_file(wcstring_view path, int oflag) { } # endif +pipe::pipe() { + int fds[2] = {}; +# ifdef _WIN32 + // Make the default pipe capacity same as on Linux 2.6.11+. + enum { DEFAULT_CAPACITY = 65536 }; + int result = FMT_POSIX_CALL(pipe(fds, DEFAULT_CAPACITY, _O_BINARY)); +# else + // Don't retry as the pipe function doesn't return EINTR. + // http://pubs.opengroup.org/onlinepubs/009696799/functions/pipe.html + int result = FMT_POSIX_CALL(pipe(fds)); +# endif + if (result != 0) + FMT_THROW(system_error(errno, FMT_STRING("cannot create pipe"))); + // The following assignments don't throw. + read_end = file(fds[0]); + write_end = file(fds[1]); +} + # if !defined(__MSDOS__) long getpagesize() { # ifdef _WIN32 diff --git a/test/gtest-extra-test.cc b/test/gtest-extra-test.cc index 34054b68..5c860400 100644 --- a/test/gtest-extra-test.cc +++ b/test/gtest-extra-test.cc @@ -316,10 +316,9 @@ using fmt::buffered_file; using fmt::file; TEST(output_redirect_test, scoped_redirect) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); { - buffered_file file(write_end.fdopen("w")); + buffered_file file(pipe.write_end.fdopen("w")); std::fprintf(file.get(), "[[["); { output_redirect redir(file.get()); @@ -327,16 +326,15 @@ TEST(output_redirect_test, scoped_redirect) { } std::fprintf(file.get(), "]]]"); } - EXPECT_READ(read_end, "[[[]]]"); + EXPECT_READ(pipe.read_end, "[[[]]]"); } // Test that output_redirect handles errors in flush correctly. TEST(output_redirect_test, flush_error_in_ctor) { - file read_end, write_end; - file::pipe(read_end, write_end); - int write_fd = write_end.descriptor(); - file write_copy = write_end.dup(write_fd); - buffered_file f = write_end.fdopen("w"); + auto pipe = fmt::pipe(); + int write_fd = pipe.write_end.descriptor(); + file write_copy = pipe.write_end.dup(write_fd); + buffered_file f = pipe.write_end.fdopen("w"); // Put a character in a file buffer. EXPECT_EQ('x', fputc('x', f.get())); FMT_POSIX(close(write_fd)); @@ -360,9 +358,8 @@ TEST(output_redirect_test, dup_error_in_ctor) { } TEST(output_redirect_test, restore_and_read) { - file read_end, write_end; - file::pipe(read_end, write_end); - buffered_file file(write_end.fdopen("w")); + auto pipe = fmt::pipe(); + buffered_file file(pipe.write_end.fdopen("w")); std::fprintf(file.get(), "[[["); output_redirect redir(file.get()); std::fprintf(file.get(), "censored"); @@ -370,16 +367,15 @@ TEST(output_redirect_test, restore_and_read) { EXPECT_EQ("", redir.restore_and_read()); std::fprintf(file.get(), "]]]"); file = buffered_file(); - EXPECT_READ(read_end, "[[[]]]"); + EXPECT_READ(pipe.read_end, "[[[]]]"); } // Test that OutputRedirect handles errors in flush correctly. TEST(output_redirect_test, flush_error_in_restore_and_read) { - file read_end, write_end; - file::pipe(read_end, write_end); - int write_fd = write_end.descriptor(); - file write_copy = write_end.dup(write_fd); - buffered_file f = write_end.fdopen("w"); + auto pipe = fmt::pipe(); + int write_fd = pipe.write_end.descriptor(); + file write_copy = pipe.write_end.dup(write_fd); + buffered_file f = pipe.write_end.fdopen("w"); output_redirect redir(f.get()); // Put a character in a file buffer. EXPECT_EQ('x', fputc('x', f.get())); @@ -390,11 +386,10 @@ TEST(output_redirect_test, flush_error_in_restore_and_read) { } TEST(output_redirect_test, error_in_dtor) { - file read_end, write_end; - file::pipe(read_end, write_end); - int write_fd = write_end.descriptor(); - file write_copy = write_end.dup(write_fd); - buffered_file f = write_end.fdopen("w"); + auto pipe = fmt::pipe(); + int write_fd = pipe.write_end.descriptor(); + file write_copy = pipe.write_end.dup(write_fd); + buffered_file f = pipe.write_end.fdopen("w"); std::unique_ptr redir(new output_redirect(f.get())); // Put a character in a file buffer. EXPECT_EQ('x', fputc('x', f.get())); diff --git a/test/gtest-extra.cc b/test/gtest-extra.cc index 3d27cf96..0f9fff5e 100644 --- a/test/gtest-extra.cc +++ b/test/gtest-extra.cc @@ -17,10 +17,10 @@ output_redirect::output_redirect(FILE* f, bool flush) : file_(f) { // Create a file object referring to the original file. original_ = file::dup(fd); // Create a pipe. - file write_end; - file::pipe(read_end_, write_end); + auto pipe = fmt::pipe(); + read_end_ = std::move(pipe.read_end); // Connect the passed FILE object to the write end of the pipe. - write_end.dup2(fd); + pipe.write_end.dup2(fd); } output_redirect::~output_redirect() noexcept { diff --git a/test/os-test.cc b/test/os-test.cc index 1e6c7c4b..abc5d3ad 100644 --- a/test/os-test.cc +++ b/test/os-test.cc @@ -104,7 +104,7 @@ TEST(file_test, open_windows_file) { using fmt::file; -bool isclosed(int fd) { +auto isclosed(int fd) -> bool { char buffer; auto result = std::streamsize(); SUPPRESS_ASSERT(result = FMT_POSIX(read(fd, &buffer, 1))); @@ -112,12 +112,11 @@ bool isclosed(int fd) { } // Opens a file for reading. -file open_file() { - file read_end, write_end; - file::pipe(read_end, write_end); - write_end.write(file_content, std::strlen(file_content)); - write_end.close(); - return read_end; +auto open_file() -> file { + auto pipe = fmt::pipe(); + pipe.write_end.write(file_content, std::strlen(file_content)); + pipe.write_end.close(); + return std::move(pipe.read_end); } // Attempts to write a string to a file. @@ -427,11 +426,10 @@ TEST(file_test, read_error) { } TEST(file_test, write) { - file read_end, write_end; - file::pipe(read_end, write_end); - write(write_end, "test"); - write_end.close(); - EXPECT_READ(read_end, "test"); + auto pipe = fmt::pipe(); + write(pipe.write_end, "test"); + pipe.write_end.close(); + EXPECT_READ(pipe.read_end, "test"); } TEST(file_test, write_error) { @@ -489,18 +487,16 @@ TEST(file_test, dup2_noexcept_error) { } TEST(file_test, pipe) { - file read_end, write_end; - file::pipe(read_end, write_end); - EXPECT_NE(-1, read_end.descriptor()); - EXPECT_NE(-1, write_end.descriptor()); - write(write_end, "test"); - EXPECT_READ(read_end, "test"); + auto pipe = fmt::pipe(); + EXPECT_NE(-1, pipe.read_end.descriptor()); + EXPECT_NE(-1, pipe.write_end.descriptor()); + write(pipe.write_end, "test"); + EXPECT_READ(pipe.read_end, "test"); } TEST(file_test, fdopen) { - file read_end, write_end; - file::pipe(read_end, write_end); - int read_fd = read_end.descriptor(); - EXPECT_EQ(read_fd, FMT_POSIX(fileno(read_end.fdopen("r").get()))); + auto pipe = fmt::pipe(); + int read_fd = pipe.read_end.descriptor(); + EXPECT_EQ(read_fd, FMT_POSIX(fileno(pipe.read_end.fdopen("r").get()))); } #endif // FMT_USE_FCNTL diff --git a/test/posix-mock-test.cc b/test/posix-mock-test.cc index 32078bf8..07072a02 100644 --- a/test/posix-mock-test.cc +++ b/test/posix-mock-test.cc @@ -225,9 +225,8 @@ TEST(file_test, open_retry) { } TEST(file_test, close_no_retry_in_dtor) { - file read_end, write_end; - file::pipe(read_end, write_end); - std::unique_ptr f(new file(std::move(read_end))); + auto pipe = fmt::pipe(); + std::unique_ptr f(new file(std::move(pipe.read_end))); int saved_close_count = 0; EXPECT_WRITE( stderr, @@ -242,10 +241,9 @@ TEST(file_test, close_no_retry_in_dtor) { } TEST(file_test, close_no_retry) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); close_count = 1; - EXPECT_SYSTEM_ERROR(read_end.close(), EINTR, "cannot close file"); + EXPECT_SYSTEM_ERROR(pipe.read_end.close(), EINTR, "cannot close file"); EXPECT_EQ(2, close_count); close_count = 0; } @@ -283,30 +281,28 @@ TEST(file_test, max_size) { } TEST(file_test, read_retry) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); enum { SIZE = 4 }; - write_end.write("test", SIZE); - write_end.close(); + pipe.write_end.write("test", SIZE); + pipe.write_end.close(); char buffer[SIZE]; size_t count = 0; - EXPECT_RETRY(count = read_end.read(buffer, SIZE), read, + EXPECT_RETRY(count = pipe.read_end.read(buffer, SIZE), read, "cannot read from file"); EXPECT_EQ_POSIX(static_cast(SIZE), count); } TEST(file_test, write_retry) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); enum { SIZE = 4 }; size_t count = 0; - EXPECT_RETRY(count = write_end.write("test", SIZE), write, + EXPECT_RETRY(count = pipe.write_end.write("test", SIZE), write, "cannot write to file"); - write_end.close(); + pipe.write_end.close(); # ifndef _WIN32 EXPECT_EQ(static_cast(SIZE), count); char buffer[SIZE + 1]; - read_end.read(buffer, SIZE); + pipe.read_end.read(buffer, SIZE); buffer[SIZE] = '\0'; EXPECT_STREQ("test", buffer); # endif @@ -314,27 +310,25 @@ TEST(file_test, write_retry) { # ifdef _WIN32 TEST(file_test, convert_read_count) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); char c; size_t size = UINT_MAX; if (sizeof(unsigned) != sizeof(size_t)) ++size; read_count = 1; read_nbyte = 0; - EXPECT_THROW(read_end.read(&c, size), std::system_error); + EXPECT_THROW(pipe.read_end.read(&c, size), std::system_error); read_count = 0; EXPECT_EQ(UINT_MAX, read_nbyte); } TEST(file_test, convert_write_count) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); char c; size_t size = UINT_MAX; if (sizeof(unsigned) != sizeof(size_t)) ++size; write_count = 1; write_nbyte = 0; - EXPECT_THROW(write_end.write(&c, size), std::system_error); + EXPECT_THROW(pipe.write_end.write(&c, size), std::system_error); write_count = 0; EXPECT_EQ(UINT_MAX, write_nbyte); } @@ -372,18 +366,15 @@ TEST(file_test, dup2_no_except_retry) { } TEST(file_test, pipe_no_retry) { - file read_end, write_end; pipe_count = 1; - EXPECT_SYSTEM_ERROR(file::pipe(read_end, write_end), EINTR, - "cannot create pipe"); + EXPECT_SYSTEM_ERROR(fmt::pipe(), EINTR, "cannot create pipe"); pipe_count = 0; } TEST(file_test, fdopen_no_retry) { - file read_end, write_end; - file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); fdopen_count = 1; - EXPECT_SYSTEM_ERROR(read_end.fdopen("r"), EINTR, + EXPECT_SYSTEM_ERROR(pipe.read_end.fdopen("r"), EINTR, "cannot associate stream with file descriptor"); fdopen_count = 0; } @@ -401,9 +392,8 @@ TEST(buffered_file_test, open_retry) { } TEST(buffered_file_test, close_no_retry_in_dtor) { - file read_end, write_end; - file::pipe(read_end, write_end); - std::unique_ptr f(new buffered_file(read_end.fdopen("r"))); + auto pipe = fmt::pipe(); + std::unique_ptr f(new buffered_file(pipe.read_end.fdopen("r"))); int saved_fclose_count = 0; EXPECT_WRITE( stderr, @@ -418,9 +408,8 @@ TEST(buffered_file_test, close_no_retry_in_dtor) { } TEST(buffered_file_test, close_no_retry) { - file read_end, write_end; - file::pipe(read_end, write_end); - buffered_file f = read_end.fdopen("r"); + auto pipe = fmt::pipe(); + buffered_file f = pipe.read_end.fdopen("r"); fclose_count = 1; EXPECT_SYSTEM_ERROR(f.close(), EINTR, "cannot close file"); EXPECT_EQ(2, fclose_count); @@ -428,9 +417,8 @@ TEST(buffered_file_test, close_no_retry) { } TEST(buffered_file_test, fileno_no_retry) { - file read_end, write_end; - file::pipe(read_end, write_end); - buffered_file f = read_end.fdopen("r"); + auto pipe = fmt::pipe(); + buffered_file f = pipe.read_end.fdopen("r"); fileno_count = 1; EXPECT_SYSTEM_ERROR((f.descriptor)(), EINTR, "cannot get file descriptor"); EXPECT_EQ(2, fileno_count); diff --git a/test/printf-test.cc b/test/printf-test.cc index 7e09ecca..63d5b437 100644 --- a/test/printf-test.cc +++ b/test/printf-test.cc @@ -517,9 +517,8 @@ TEST(printf_test, examples) { } TEST(printf_test, printf_error) { - fmt::file read_end, write_end; - fmt::file::pipe(read_end, write_end); - int result = fmt::fprintf(read_end.fdopen("r").get(), "test"); + auto pipe = fmt::pipe(); + int result = fmt::fprintf(pipe.read_end.fdopen("r").get(), "test"); EXPECT_LT(result, 0); } #endif diff --git a/test/scan-test.cc b/test/scan-test.cc index f0ede192..6891bf20 100644 --- a/test/scan-test.cc +++ b/test/scan-test.cc @@ -139,37 +139,36 @@ TEST(scan_test, end_of_input) { #if FMT_USE_FCNTL TEST(scan_test, file) { - fmt::file read_end, write_end; - fmt::file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); fmt::string_view input = "10 20"; - write_end.write(input.data(), input.size()); - write_end.close(); + pipe.write_end.write(input.data(), input.size()); + pipe.write_end.close(); int n1 = 0, n2 = 0; - fmt::buffered_file f = read_end.fdopen("r"); + fmt::buffered_file f = pipe.read_end.fdopen("r"); fmt::scan(f.get(), "{} {}", n1, n2); EXPECT_EQ(n1, 10); EXPECT_EQ(n2, 20); } TEST(scan_test, lock) { - fmt::file read_end, write_end; - fmt::file::pipe(read_end, write_end); + auto pipe = fmt::pipe(); std::thread producer([&]() { fmt::string_view input = "42 "; - for (int i = 0; i < 1000; ++i) write_end.write(input.data(), input.size()); - write_end.close(); + for (int i = 0; i < 1000; ++i) + pipe.write_end.write(input.data(), input.size()); + pipe.write_end.close(); }); std::atomic count(0); - fmt::buffered_file f = read_end.fdopen("r"); + fmt::buffered_file f = pipe.read_end.fdopen("r"); auto fun = [&]() { int value = 0; while (fmt::scan(f.get(), "{}", value)) { if (value != 42) { - read_end.close(); + pipe.read_end.close(); EXPECT_EQ(value, 42); break; } diff --git a/test/util.cc b/test/util.cc index d3f2dc73..a1b992b8 100644 --- a/test/util.cc +++ b/test/util.cc @@ -13,11 +13,10 @@ const char* const file_content = "Don't panic!"; fmt::buffered_file open_buffered_file(FILE** fp) { #if FMT_USE_FCNTL - fmt::file read_end, write_end; - fmt::file::pipe(read_end, write_end); - write_end.write(file_content, std::strlen(file_content)); - write_end.close(); - fmt::buffered_file f = read_end.fdopen("r"); + auto pipe = fmt::pipe(); + pipe.write_end.write(file_content, std::strlen(file_content)); + pipe.write_end.close(); + fmt::buffered_file f = pipe.read_end.fdopen("r"); if (fp) *fp = f.get(); #else fmt::buffered_file f("test-file", "w");