From a2aab24db70fcc7e52ea323d5e6b9d8c2f7595de Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 31 Jan 2019 12:12:46 -0800 Subject: [PATCH] Switch io_mode to an enum class --- src/exec.cpp | 36 ++++++++++++++++++------------------ src/io.cpp | 2 +- src/io.h | 20 +++++++++++--------- src/postfork.cpp | 30 +++++++++++++++--------------- src/proc.cpp | 4 ++-- 5 files changed, 47 insertions(+), 45 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index 294f6eef7..d180ef826 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -102,7 +102,7 @@ int exec_pipe(int fd[2]) { /// Returns true if the redirection is a file redirection to a file other than /dev/null. static bool redirection_is_to_real_file(const io_data_t *io) { bool result = false; - if (io != NULL && io->io_mode == IO_FILE) { + if (io != NULL && io->io_mode == io_mode_t::file) { // It's a file redirection. Compare the path to /dev/null. const io_file_t *io_file = static_cast(io); const char *path = io_file->filename_cstr; @@ -256,15 +256,15 @@ static bool io_transmogrify(const io_chain_t &in_chain, io_chain_t *out_chain, shared_ptr out; // gets allocated via new switch (in->io_mode) { - case IO_PIPE: - case IO_FD: - case IO_BUFFER: - case IO_CLOSE: { + case io_mode_t::pipe: + case io_mode_t::fd: + case io_mode_t::buffer: + case io_mode_t::close: { // These redirections don't need transmogrification. They can be passed through. out = in; break; } - case IO_FILE: { + case io_mode_t::file: { // Transmogrify file redirections. int fd; io_file_t *in_file = static_cast(in.get()); @@ -452,7 +452,7 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptrpipe_fd[0]; } else if (const auto in = proc_io_chain.get_io_for_fd(STDIN_FILENO)) { switch (in->io_mode) { - case IO_FD: { + case io_mode_t::fd: { const io_fd_t *in_fd = static_cast(in.get()); // Ignore user-supplied fd redirections from an fd other than the // standard ones. e.g. in source <&3 don't actually read from fd 3, @@ -466,12 +466,12 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr(in.get()); local_builtin_stdin = in_pipe->pipe_fd[0]; break; } - case IO_FILE: { + case io_mode_t::file: { // Do not set CLO_EXEC because child needs access. const io_file_t *in_file = static_cast(in.get()); local_builtin_stdin = open(in_file->filename_cstr, in_file->flags, OPEN_MASK); @@ -484,7 +484,7 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr stdin_io = io_chain_get(p->io_chain(), STDIN_FILENO); - stdin_is_directly_redirected = stdin_io && stdin_io->io_mode != IO_CLOSE; + stdin_is_directly_redirected = stdin_io && stdin_io->io_mode != io_mode_t::close; } streams.stdin_fd = local_builtin_stdin; @@ -568,7 +568,7 @@ static bool handle_builtin_output(const std::shared_ptr &j, process_t *p, if (!must_fork && p->is_last_in_job) { // We are handling reads directly in the main loop. Note that we may still end // up forking. - const bool stdout_is_to_buffer = stdout_io && stdout_io->io_mode == IO_BUFFER; + const bool stdout_is_to_buffer = stdout_io && stdout_io->io_mode == io_mode_t::buffer; const bool no_stdout_output = stdout_stream.empty(); const bool no_stderr_output = stderr_stream.empty(); const bool stdout_discarded = stdout_stream.buffer().discarded(); @@ -1012,15 +1012,15 @@ bool exec_job(parser_t &parser, shared_ptr j) { } } - // Verify that all IO_BUFFERs are output. We used to support a (single, hacked-in) magical input - // IO_BUFFER used by fish_pager, but now the claim is that there are no more clients and it is - // removed. This assertion double-checks that. + // Verify that all io_mode_t::buffers are output. We used to support a (single, hacked-in) + // magical input io_mode_t::buffer used by fish_pager, but now the claim is that there are no + // more clients and it is removed. This assertion double-checks that. size_t stdout_read_limit = 0; const io_chain_t all_ios = j->all_io_redirections(); for (size_t idx = 0; idx < all_ios.size(); idx++) { const shared_ptr &io = all_ios.at(idx); - if ((io->io_mode == IO_BUFFER)) { + if ((io->io_mode == io_mode_t::buffer)) { io_buffer_t *io_buffer = static_cast(io.get()); assert(!io_buffer->is_input); stdout_read_limit = io_buffer->buffer().limit(); @@ -1036,7 +1036,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { // with a redireciton like <&3; we may also have chosen 3 as the fd for our pipe. Ensure we have // no conflicts. for (const auto io : all_ios) { - if (io->io_mode == IO_BUFFER) { + if (io->io_mode == io_mode_t::buffer) { auto *io_buffer = static_cast(io.get()); if (!io_buffer->avoid_conflicts_with_io_chain(all_ios)) { // We could not avoid conflicts, probably due to fd exhaustion. Mark an error. diff --git a/src/io.cpp b/src/io.cpp index dc916c0b8..2f8fa32af 100644 --- a/src/io.cpp +++ b/src/io.cpp @@ -43,7 +43,7 @@ void io_buffer_t::append_from_stream(const output_stream_t &stream) { void io_buffer_t::read() { exec_close(pipe_fd[1]); - if (io_mode == IO_BUFFER) { + if (io_mode == io_mode_t::buffer) { debug(4, L"io_buffer_t::read: blocking read on fd %d", pipe_fd[0]); while (1) { char b[4096]; diff --git a/src/io.h b/src/io.h index f6475d363..cee99ccab 100644 --- a/src/io.h +++ b/src/io.h @@ -150,7 +150,7 @@ class separated_buffer_t { }; /// Describes what type of IO operation an io_data_t represents. -enum io_mode_t { IO_FILE, IO_PIPE, IO_FD, IO_BUFFER, IO_CLOSE }; +enum class io_mode_t { file, pipe, fd, buffer, close }; /// Represents an FD redirection. class io_data_t { @@ -174,7 +174,7 @@ class io_data_t { class io_close_t : public io_data_t { public: - explicit io_close_t(int f) : io_data_t(IO_CLOSE, f) {} + explicit io_close_t(int f) : io_data_t(io_mode_t::close, f) {} void print() const override; }; @@ -191,7 +191,8 @@ class io_fd_t : public io_data_t { void print() const override; - io_fd_t(int f, int old, bool us) : io_data_t(IO_FD, f), old_fd(old), user_supplied(us) {} + io_fd_t(int f, int old, bool us) + : io_data_t(io_mode_t::fd, f), old_fd(old), user_supplied(us) {} }; class io_file_t : public io_data_t { @@ -204,7 +205,7 @@ class io_file_t : public io_data_t { void print() const override; io_file_t(int f, const wcstring &fname, int fl = 0) - : io_data_t(IO_FILE, f), filename_cstr(wcs2str(fname)), flags(fl) {} + : io_data_t(io_mode_t::file, f), filename_cstr(wcs2str(fname)), flags(fl) {} ~io_file_t() override { free((void *)filename_cstr); } }; @@ -221,7 +222,9 @@ class io_pipe_t : public io_data_t { void print() const override; - io_pipe_t(int f, bool i) : io_data_t(IO_PIPE, f), is_input(i) { pipe_fd[0] = pipe_fd[1] = -1; } + io_pipe_t(int f, bool i) : io_data_t(io_mode_t::pipe, f), is_input(i) { + pipe_fd[0] = pipe_fd[1] = -1; + } }; class io_chain_t; @@ -231,8 +234,7 @@ class io_buffer_t : public io_pipe_t { separated_buffer_t buffer_; explicit io_buffer_t(int f, size_t limit) - : io_pipe_t(IO_BUFFER, f, false /* not input */), - buffer_(limit) { + : io_pipe_t(io_mode_t::buffer, f, false /* not input */), buffer_(limit) { // Explicitly reset the discard flag because we share this buffer. buffer_.reset_discard(); } @@ -258,8 +260,8 @@ class io_buffer_t : public io_pipe_t { /// Marks the receiver as discarded if the stream was discarded. void append_from_stream(const output_stream_t &stream); - /// Create a IO_BUFFER type io redirection, complete with a pipe and a vector for output. - /// The default file descriptor used is STDOUT_FILENO for buffering. + /// Create a io_mode_t::buffer type io redirection, complete with a pipe and a vector for + /// output. The default file descriptor used is STDOUT_FILENO for buffering. /// /// \param fd the fd that will be mapped in the child process, typically STDOUT_FILENO /// \param conflicts A set of IO redirections. The function ensures that any pipe it makes does diff --git a/src/postfork.cpp b/src/postfork.cpp index dccef08bc..7a6f655c0 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -187,12 +187,12 @@ static int handle_child_io(const io_chain_t &io_chain) { for (size_t idx = 0; idx < io_chain.size(); idx++) { const io_data_t *io = io_chain.at(idx).get(); - if (io->io_mode == IO_FD && io->fd == static_cast(io)->old_fd) { + if (io->io_mode == io_mode_t::fd && io->fd == static_cast(io)->old_fd) { continue; } switch (io->io_mode) { - case IO_CLOSE: { + case io_mode_t::close: { if (log_redirections) fwprintf(stderr, L"%d: close %d\n", getpid(), io->fd); if (close(io->fd)) { debug_safe_int(0, "Failed to close file descriptor %s", io->fd); @@ -201,7 +201,7 @@ static int handle_child_io(const io_chain_t &io_chain) { break; } - case IO_FILE: { + case io_mode_t::file: { // Here we definitely do not want to set CLO_EXEC because our child needs access. const io_file_t *io_file = static_cast(io); int tmp = open(io_file->filename_cstr, io_file->flags, OPEN_MASK); @@ -229,7 +229,7 @@ static int handle_child_io(const io_chain_t &io_chain) { break; } - case IO_FD: { + case io_mode_t::fd: { int old_fd = static_cast(io)->old_fd; if (log_redirections) fwprintf(stderr, L"%d: fd dup %d to %d\n", getpid(), old_fd, io->fd); @@ -245,20 +245,20 @@ static int handle_child_io(const io_chain_t &io_chain) { break; } - case IO_BUFFER: - case IO_PIPE: { + case io_mode_t::buffer: + case io_mode_t::pipe: { const io_pipe_t *io_pipe = static_cast(io); // If write_pipe_idx is 0, it means we're connecting to the read end (first pipe // fd). If it's 1, we're connecting to the write end (second pipe fd). unsigned int write_pipe_idx = (io_pipe->is_input ? 0 : 1); #if 0 debug(0, L"%ls %ls on fd %d (%d %d)", write_pipe?L"write":L"read", - (io->io_mode == IO_BUFFER)?L"buffer":L"pipe", io->fd, io->pipe_fd[0], + (io->io_mode == io_mode_t::buffer)?L"buffer":L"pipe", io->fd, io->pipe_fd[0], io->pipe_fd[1]); #endif if (log_redirections) fwprintf(stderr, L"%d: %s dup %d to %d\n", getpid(), - io->io_mode == IO_BUFFER ? "buffer" : "pipe", + io->io_mode == io_mode_t::buffer ? "buffer" : "pipe", io_pipe->pipe_fd[write_pipe_idx], io->fd); if (dup2(io_pipe->pipe_fd[write_pipe_idx], io->fd) != io->fd) { debug_safe(1, LOCAL_PIPE_ERROR); @@ -280,7 +280,7 @@ int setup_child_process(process_t *p, const io_chain_t &io_chain) { bool ok = true; if (ok) { - // In the case of IO_FILE, this can hang until data is available to read/write! + // In the case of io_mode_t::file, this can hang until data is available to read/write! ok = (0 == handle_child_io(io_chain)); if (p != 0 && !ok) { debug_safe(4, "handle_child_io failed in setup_child_process"); @@ -405,18 +405,18 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, for (size_t idx = 0; idx < io_chain.size(); idx++) { const shared_ptr io = io_chain.at(idx); - if (io->io_mode == IO_FD) { + if (io->io_mode == io_mode_t::fd) { const io_fd_t *io_fd = static_cast(io.get()); if (io->fd == io_fd->old_fd) continue; } switch (io->io_mode) { - case IO_CLOSE: { + case io_mode_t::close: { if (!err) err = posix_spawn_file_actions_addclose(actions, io->fd); break; } - case IO_FILE: { + case io_mode_t::file: { const io_file_t *io_file = static_cast(io.get()); if (!err) err = posix_spawn_file_actions_addopen(actions, io->fd, io_file->filename_cstr, @@ -424,7 +424,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, break; } - case IO_FD: { + case io_mode_t::fd: { const io_fd_t *io_fd = static_cast(io.get()); if (!err) err = posix_spawn_file_actions_adddup2(actions, io_fd->old_fd /* from */, @@ -432,8 +432,8 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, break; } - case IO_BUFFER: - case IO_PIPE: { + case io_mode_t::buffer: + case io_mode_t::pipe: { const io_pipe_t *io_pipe = static_cast(io.get()); unsigned int write_pipe_idx = (io_pipe->is_input ? 0 : 1); int from_fd = io_pipe->pipe_fd[write_pipe_idx]; diff --git a/src/proc.cpp b/src/proc.cpp index 8aa59fde1..162ae35f6 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -850,7 +850,7 @@ static select_try_t select_try(job_t *j) { const io_chain_t chain = j->all_io_redirections(); for (const auto &io : chain) { - if (io->io_mode == IO_BUFFER) { + if (io->io_mode == io_mode_t::buffer) { auto io_pipe = static_cast(io.get()); int fd = io_pipe->pipe_fd[0]; FD_SET(fd, &fds); @@ -886,7 +886,7 @@ static void read_try(job_t *j) { const io_chain_t chain = j->all_io_redirections(); for (size_t idx = 0; idx < chain.size(); idx++) { io_data_t *d = chain.at(idx).get(); - if (d->io_mode == IO_BUFFER) { + if (d->io_mode == io_mode_t::buffer) { buff = static_cast(d); } }