Improve the pipe API

This commit is contained in:
Victor Zverovich 2024-01-01 13:25:07 -08:00
parent 398ddb8fec
commit c142385033
9 changed files with 111 additions and 132 deletions

View File

@ -238,6 +238,7 @@ class buffered_file {
}; };
#if FMT_USE_FCNTL #if FMT_USE_FCNTL
// A file. Closed file is represented by a file object with descriptor -1. // A file. Closed file is represented by a file object with descriptor -1.
// Methods that are not declared with noexcept may throw // Methods that are not declared with noexcept may throw
// fmt::system_error in case of failure. Note that some errors such as // 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. // Constructs a file object with a given descriptor.
explicit file(int fd) : fd_(fd) {} explicit file(int fd) : fd_(fd) {}
friend struct pipe;
public: public:
// Possible values for the oflag argument to the constructor. // Possible values for the oflag argument to the constructor.
enum { enum {
@ -313,11 +316,6 @@ class FMT_API file {
// necessary. // necessary.
void dup2(int fd, std::error_code& ec) noexcept; 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 // Creates a buffered_file object associated with this file and detaches
// this file object from the file. // this file object from the file.
auto fdopen(const char* mode) -> buffered_file; auto fdopen(const char* mode) -> buffered_file;
@ -329,6 +327,15 @@ class FMT_API file {
# endif # 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. // Returns the memory page size.
auto getpagesize() -> long; auto getpagesize() -> long;

View File

@ -200,6 +200,7 @@ int buffered_file::descriptor() const {
# ifdef _WIN32 # ifdef _WIN32
using mode_t = int; using mode_t = int;
# endif # endif
constexpr mode_t default_open_mode = constexpr mode_t default_open_mode =
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; 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()); 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) { buffered_file file::fdopen(const char* mode) {
// Don't retry as fdopen doesn't return EINTR. // Don't retry as fdopen doesn't return EINTR.
# if defined(__MINGW32__) && defined(_POSIX_) # if defined(__MINGW32__) && defined(_POSIX_)
@ -352,6 +330,24 @@ file file::open_windows_file(wcstring_view path, int oflag) {
} }
# endif # 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__) # if !defined(__MSDOS__)
long getpagesize() { long getpagesize() {
# ifdef _WIN32 # ifdef _WIN32

View File

@ -316,10 +316,9 @@ using fmt::buffered_file;
using fmt::file; using fmt::file;
TEST(output_redirect_test, scoped_redirect) { TEST(output_redirect_test, scoped_redirect) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
{ {
buffered_file file(write_end.fdopen("w")); buffered_file file(pipe.write_end.fdopen("w"));
std::fprintf(file.get(), "[[["); std::fprintf(file.get(), "[[[");
{ {
output_redirect redir(file.get()); output_redirect redir(file.get());
@ -327,16 +326,15 @@ TEST(output_redirect_test, scoped_redirect) {
} }
std::fprintf(file.get(), "]]]"); std::fprintf(file.get(), "]]]");
} }
EXPECT_READ(read_end, "[[[]]]"); EXPECT_READ(pipe.read_end, "[[[]]]");
} }
// Test that output_redirect handles errors in flush correctly. // Test that output_redirect handles errors in flush correctly.
TEST(output_redirect_test, flush_error_in_ctor) { TEST(output_redirect_test, flush_error_in_ctor) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); int write_fd = pipe.write_end.descriptor();
int write_fd = write_end.descriptor(); file write_copy = pipe.write_end.dup(write_fd);
file write_copy = write_end.dup(write_fd); buffered_file f = pipe.write_end.fdopen("w");
buffered_file f = write_end.fdopen("w");
// Put a character in a file buffer. // Put a character in a file buffer.
EXPECT_EQ('x', fputc('x', f.get())); EXPECT_EQ('x', fputc('x', f.get()));
FMT_POSIX(close(write_fd)); FMT_POSIX(close(write_fd));
@ -360,9 +358,8 @@ TEST(output_redirect_test, dup_error_in_ctor) {
} }
TEST(output_redirect_test, restore_and_read) { TEST(output_redirect_test, restore_and_read) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); buffered_file file(pipe.write_end.fdopen("w"));
buffered_file file(write_end.fdopen("w"));
std::fprintf(file.get(), "[[["); std::fprintf(file.get(), "[[[");
output_redirect redir(file.get()); output_redirect redir(file.get());
std::fprintf(file.get(), "censored"); std::fprintf(file.get(), "censored");
@ -370,16 +367,15 @@ TEST(output_redirect_test, restore_and_read) {
EXPECT_EQ("", redir.restore_and_read()); EXPECT_EQ("", redir.restore_and_read());
std::fprintf(file.get(), "]]]"); std::fprintf(file.get(), "]]]");
file = buffered_file(); file = buffered_file();
EXPECT_READ(read_end, "[[[]]]"); EXPECT_READ(pipe.read_end, "[[[]]]");
} }
// Test that OutputRedirect handles errors in flush correctly. // Test that OutputRedirect handles errors in flush correctly.
TEST(output_redirect_test, flush_error_in_restore_and_read) { TEST(output_redirect_test, flush_error_in_restore_and_read) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); int write_fd = pipe.write_end.descriptor();
int write_fd = write_end.descriptor(); file write_copy = pipe.write_end.dup(write_fd);
file write_copy = write_end.dup(write_fd); buffered_file f = pipe.write_end.fdopen("w");
buffered_file f = write_end.fdopen("w");
output_redirect redir(f.get()); output_redirect redir(f.get());
// Put a character in a file buffer. // Put a character in a file buffer.
EXPECT_EQ('x', fputc('x', f.get())); 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) { TEST(output_redirect_test, error_in_dtor) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); int write_fd = pipe.write_end.descriptor();
int write_fd = write_end.descriptor(); file write_copy = pipe.write_end.dup(write_fd);
file write_copy = write_end.dup(write_fd); buffered_file f = pipe.write_end.fdopen("w");
buffered_file f = write_end.fdopen("w");
std::unique_ptr<output_redirect> redir(new output_redirect(f.get())); std::unique_ptr<output_redirect> redir(new output_redirect(f.get()));
// Put a character in a file buffer. // Put a character in a file buffer.
EXPECT_EQ('x', fputc('x', f.get())); EXPECT_EQ('x', fputc('x', f.get()));

View File

@ -17,10 +17,10 @@ output_redirect::output_redirect(FILE* f, bool flush) : file_(f) {
// Create a file object referring to the original file. // Create a file object referring to the original file.
original_ = file::dup(fd); original_ = file::dup(fd);
// Create a pipe. // Create a pipe.
file write_end; auto pipe = fmt::pipe();
file::pipe(read_end_, write_end); read_end_ = std::move(pipe.read_end);
// Connect the passed FILE object to the write end of the pipe. // 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 { output_redirect::~output_redirect() noexcept {

View File

@ -104,7 +104,7 @@ TEST(file_test, open_windows_file) {
using fmt::file; using fmt::file;
bool isclosed(int fd) { auto isclosed(int fd) -> bool {
char buffer; char buffer;
auto result = std::streamsize(); auto result = std::streamsize();
SUPPRESS_ASSERT(result = FMT_POSIX(read(fd, &buffer, 1))); SUPPRESS_ASSERT(result = FMT_POSIX(read(fd, &buffer, 1)));
@ -112,12 +112,11 @@ bool isclosed(int fd) {
} }
// Opens a file for reading. // Opens a file for reading.
file open_file() { auto open_file() -> file {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); pipe.write_end.write(file_content, std::strlen(file_content));
write_end.write(file_content, std::strlen(file_content)); pipe.write_end.close();
write_end.close(); return std::move(pipe.read_end);
return read_end;
} }
// Attempts to write a string to a file. // Attempts to write a string to a file.
@ -427,11 +426,10 @@ TEST(file_test, read_error) {
} }
TEST(file_test, write) { TEST(file_test, write) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); write(pipe.write_end, "test");
write(write_end, "test"); pipe.write_end.close();
write_end.close(); EXPECT_READ(pipe.read_end, "test");
EXPECT_READ(read_end, "test");
} }
TEST(file_test, write_error) { TEST(file_test, write_error) {
@ -489,18 +487,16 @@ TEST(file_test, dup2_noexcept_error) {
} }
TEST(file_test, pipe) { TEST(file_test, pipe) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); EXPECT_NE(-1, pipe.read_end.descriptor());
EXPECT_NE(-1, read_end.descriptor()); EXPECT_NE(-1, pipe.write_end.descriptor());
EXPECT_NE(-1, write_end.descriptor()); write(pipe.write_end, "test");
write(write_end, "test"); EXPECT_READ(pipe.read_end, "test");
EXPECT_READ(read_end, "test");
} }
TEST(file_test, fdopen) { TEST(file_test, fdopen) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); int read_fd = pipe.read_end.descriptor();
int read_fd = read_end.descriptor(); EXPECT_EQ(read_fd, FMT_POSIX(fileno(pipe.read_end.fdopen("r").get())));
EXPECT_EQ(read_fd, FMT_POSIX(fileno(read_end.fdopen("r").get())));
} }
#endif // FMT_USE_FCNTL #endif // FMT_USE_FCNTL

View File

@ -225,9 +225,8 @@ TEST(file_test, open_retry) {
} }
TEST(file_test, close_no_retry_in_dtor) { TEST(file_test, close_no_retry_in_dtor) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); std::unique_ptr<file> f(new file(std::move(pipe.read_end)));
std::unique_ptr<file> f(new file(std::move(read_end)));
int saved_close_count = 0; int saved_close_count = 0;
EXPECT_WRITE( EXPECT_WRITE(
stderr, stderr,
@ -242,10 +241,9 @@ TEST(file_test, close_no_retry_in_dtor) {
} }
TEST(file_test, close_no_retry) { TEST(file_test, close_no_retry) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
close_count = 1; 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); EXPECT_EQ(2, close_count);
close_count = 0; close_count = 0;
} }
@ -283,30 +281,28 @@ TEST(file_test, max_size) {
} }
TEST(file_test, read_retry) { TEST(file_test, read_retry) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
enum { SIZE = 4 }; enum { SIZE = 4 };
write_end.write("test", SIZE); pipe.write_end.write("test", SIZE);
write_end.close(); pipe.write_end.close();
char buffer[SIZE]; char buffer[SIZE];
size_t count = 0; 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"); "cannot read from file");
EXPECT_EQ_POSIX(static_cast<std::streamsize>(SIZE), count); EXPECT_EQ_POSIX(static_cast<std::streamsize>(SIZE), count);
} }
TEST(file_test, write_retry) { TEST(file_test, write_retry) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
enum { SIZE = 4 }; enum { SIZE = 4 };
size_t count = 0; 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"); "cannot write to file");
write_end.close(); pipe.write_end.close();
# ifndef _WIN32 # ifndef _WIN32
EXPECT_EQ(static_cast<std::streamsize>(SIZE), count); EXPECT_EQ(static_cast<std::streamsize>(SIZE), count);
char buffer[SIZE + 1]; char buffer[SIZE + 1];
read_end.read(buffer, SIZE); pipe.read_end.read(buffer, SIZE);
buffer[SIZE] = '\0'; buffer[SIZE] = '\0';
EXPECT_STREQ("test", buffer); EXPECT_STREQ("test", buffer);
# endif # endif
@ -314,27 +310,25 @@ TEST(file_test, write_retry) {
# ifdef _WIN32 # ifdef _WIN32
TEST(file_test, convert_read_count) { TEST(file_test, convert_read_count) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
char c; char c;
size_t size = UINT_MAX; size_t size = UINT_MAX;
if (sizeof(unsigned) != sizeof(size_t)) ++size; if (sizeof(unsigned) != sizeof(size_t)) ++size;
read_count = 1; read_count = 1;
read_nbyte = 0; 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; read_count = 0;
EXPECT_EQ(UINT_MAX, read_nbyte); EXPECT_EQ(UINT_MAX, read_nbyte);
} }
TEST(file_test, convert_write_count) { TEST(file_test, convert_write_count) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
char c; char c;
size_t size = UINT_MAX; size_t size = UINT_MAX;
if (sizeof(unsigned) != sizeof(size_t)) ++size; if (sizeof(unsigned) != sizeof(size_t)) ++size;
write_count = 1; write_count = 1;
write_nbyte = 0; 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; write_count = 0;
EXPECT_EQ(UINT_MAX, write_nbyte); EXPECT_EQ(UINT_MAX, write_nbyte);
} }
@ -372,18 +366,15 @@ TEST(file_test, dup2_no_except_retry) {
} }
TEST(file_test, pipe_no_retry) { TEST(file_test, pipe_no_retry) {
file read_end, write_end;
pipe_count = 1; pipe_count = 1;
EXPECT_SYSTEM_ERROR(file::pipe(read_end, write_end), EINTR, EXPECT_SYSTEM_ERROR(fmt::pipe(), EINTR, "cannot create pipe");
"cannot create pipe");
pipe_count = 0; pipe_count = 0;
} }
TEST(file_test, fdopen_no_retry) { TEST(file_test, fdopen_no_retry) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end);
fdopen_count = 1; 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"); "cannot associate stream with file descriptor");
fdopen_count = 0; fdopen_count = 0;
} }
@ -401,9 +392,8 @@ TEST(buffered_file_test, open_retry) {
} }
TEST(buffered_file_test, close_no_retry_in_dtor) { TEST(buffered_file_test, close_no_retry_in_dtor) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); std::unique_ptr<buffered_file> f(new buffered_file(pipe.read_end.fdopen("r")));
std::unique_ptr<buffered_file> f(new buffered_file(read_end.fdopen("r")));
int saved_fclose_count = 0; int saved_fclose_count = 0;
EXPECT_WRITE( EXPECT_WRITE(
stderr, stderr,
@ -418,9 +408,8 @@ TEST(buffered_file_test, close_no_retry_in_dtor) {
} }
TEST(buffered_file_test, close_no_retry) { TEST(buffered_file_test, close_no_retry) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); buffered_file f = pipe.read_end.fdopen("r");
buffered_file f = read_end.fdopen("r");
fclose_count = 1; fclose_count = 1;
EXPECT_SYSTEM_ERROR(f.close(), EINTR, "cannot close file"); EXPECT_SYSTEM_ERROR(f.close(), EINTR, "cannot close file");
EXPECT_EQ(2, fclose_count); EXPECT_EQ(2, fclose_count);
@ -428,9 +417,8 @@ TEST(buffered_file_test, close_no_retry) {
} }
TEST(buffered_file_test, fileno_no_retry) { TEST(buffered_file_test, fileno_no_retry) {
file read_end, write_end; auto pipe = fmt::pipe();
file::pipe(read_end, write_end); buffered_file f = pipe.read_end.fdopen("r");
buffered_file f = read_end.fdopen("r");
fileno_count = 1; fileno_count = 1;
EXPECT_SYSTEM_ERROR((f.descriptor)(), EINTR, "cannot get file descriptor"); EXPECT_SYSTEM_ERROR((f.descriptor)(), EINTR, "cannot get file descriptor");
EXPECT_EQ(2, fileno_count); EXPECT_EQ(2, fileno_count);

View File

@ -517,9 +517,8 @@ TEST(printf_test, examples) {
} }
TEST(printf_test, printf_error) { TEST(printf_test, printf_error) {
fmt::file read_end, write_end; auto pipe = fmt::pipe();
fmt::file::pipe(read_end, write_end); int result = fmt::fprintf(pipe.read_end.fdopen("r").get(), "test");
int result = fmt::fprintf(read_end.fdopen("r").get(), "test");
EXPECT_LT(result, 0); EXPECT_LT(result, 0);
} }
#endif #endif

View File

@ -139,37 +139,36 @@ TEST(scan_test, end_of_input) {
#if FMT_USE_FCNTL #if FMT_USE_FCNTL
TEST(scan_test, file) { TEST(scan_test, file) {
fmt::file read_end, write_end; auto pipe = fmt::pipe();
fmt::file::pipe(read_end, write_end);
fmt::string_view input = "10 20"; fmt::string_view input = "10 20";
write_end.write(input.data(), input.size()); pipe.write_end.write(input.data(), input.size());
write_end.close(); pipe.write_end.close();
int n1 = 0, n2 = 0; 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); fmt::scan(f.get(), "{} {}", n1, n2);
EXPECT_EQ(n1, 10); EXPECT_EQ(n1, 10);
EXPECT_EQ(n2, 20); EXPECT_EQ(n2, 20);
} }
TEST(scan_test, lock) { TEST(scan_test, lock) {
fmt::file read_end, write_end; auto pipe = fmt::pipe();
fmt::file::pipe(read_end, write_end);
std::thread producer([&]() { std::thread producer([&]() {
fmt::string_view input = "42 "; fmt::string_view input = "42 ";
for (int i = 0; i < 1000; ++i) write_end.write(input.data(), input.size()); for (int i = 0; i < 1000; ++i)
write_end.close(); pipe.write_end.write(input.data(), input.size());
pipe.write_end.close();
}); });
std::atomic<int> count(0); std::atomic<int> count(0);
fmt::buffered_file f = read_end.fdopen("r"); fmt::buffered_file f = pipe.read_end.fdopen("r");
auto fun = [&]() { auto fun = [&]() {
int value = 0; int value = 0;
while (fmt::scan(f.get(), "{}", value)) { while (fmt::scan(f.get(), "{}", value)) {
if (value != 42) { if (value != 42) {
read_end.close(); pipe.read_end.close();
EXPECT_EQ(value, 42); EXPECT_EQ(value, 42);
break; break;
} }

View File

@ -13,11 +13,10 @@ const char* const file_content = "Don't panic!";
fmt::buffered_file open_buffered_file(FILE** fp) { fmt::buffered_file open_buffered_file(FILE** fp) {
#if FMT_USE_FCNTL #if FMT_USE_FCNTL
fmt::file read_end, write_end; auto pipe = fmt::pipe();
fmt::file::pipe(read_end, write_end); pipe.write_end.write(file_content, std::strlen(file_content));
write_end.write(file_content, std::strlen(file_content)); pipe.write_end.close();
write_end.close(); fmt::buffered_file f = pipe.read_end.fdopen("r");
fmt::buffered_file f = read_end.fdopen("r");
if (fp) *fp = f.get(); if (fp) *fp = f.get();
#else #else
fmt::buffered_file f("test-file", "w"); fmt::buffered_file f("test-file", "w");