From 88dc484858f708ef5ac14c928fc5cf7c3192397a Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 28 Jan 2019 13:26:22 -0800 Subject: [PATCH] Introduce dup2_list_t This represents a "resolved" io_chain_t, where all of the different io_data_t types have been reduced to a sequence of dup2() and close(). This will eliminate a lot of the logic duplication around posix_spawn vs fork, and pave the way for in-process redirections. --- CMakeLists.txt | 2 +- Makefile.in | 2 +- src/fish_tests.cpp | 27 ++++++++++++++ src/io.cpp | 9 ++--- src/io.h | 6 +++ src/redirection.cpp | 90 +++++++++++++++++++++++++++++++++++++++++++++ src/redirection.h | 62 +++++++++++++++++++++++++++++++ 7 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 src/redirection.cpp create mode 100644 src/redirection.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 882888c7a..809f24162 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -77,7 +77,7 @@ SET(FISH_SRCS src/postfork.cpp src/proc.cpp src/reader.cpp src/sanity.cpp src/screen.cpp src/signal.cpp src/tinyexpr.cpp src/tnode.cpp src/tokenizer.cpp src/utf8.cpp src/util.cpp src/wcstringutil.cpp src/wgetopt.cpp src/wildcard.cpp src/wutil.cpp - src/future_feature_flags.cpp + src/future_feature_flags.cpp src/redirection.cpp ) # Header files are just globbed. diff --git a/Makefile.in b/Makefile.in index aa5e0e2ee..059e05847 100644 --- a/Makefile.in +++ b/Makefile.in @@ -119,7 +119,7 @@ FISH_OBJS := obj/autoload.o obj/builtin.o obj/builtin_bg.o obj/builtin_bind.o ob obj/parser_keywords.o obj/path.o obj/postfork.o obj/proc.o obj/reader.o \ obj/sanity.o obj/screen.o obj/signal.o obj/tinyexpr.o obj/tokenizer.o obj/tnode.o obj/utf8.o \ obj/util.o obj/wcstringutil.o obj/wgetopt.o obj/wildcard.o obj/wutil.o \ - obj/future_feature_flags.o + obj/future_feature_flags.o obj/redirection.o FISH_INDENT_OBJS := obj/fish_indent.o obj/print_help.o $(FISH_OBJS) diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 1760944a7..8f266947d 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -64,6 +64,7 @@ #include "path.h" #include "proc.h" #include "reader.h" +#include "redirection.h" #include "screen.h" #include "signal.h" #include "tnode.h" @@ -2339,6 +2340,31 @@ static void test_wcstod() { tod_test(L"nope", "nope"); } +static void test_dup2s() { + using std::make_shared; + io_chain_t chain; + chain.push_back(make_shared(17)); + chain.push_back(make_shared(3, 19, true)); + auto list = dup2_list_t::resolve_chain(chain); + do_test(list.has_value()); + do_test(list->get_actions().size() == 2); + + auto act1 = list->get_actions().at(0); + do_test(act1.src == 17); + do_test(act1.target == -1); + + auto act2 = list->get_actions().at(1); + do_test(act2.src == 19); + do_test(act2.target == 3); + + // Invalid files should fail to open. + // Suppress the debug() message. + scoped_push saved_debug_level(&debug_level, -1); + chain.push_back(make_shared(2, L"/definitely/not/a/valid/path/for/this/test", 0666)); + list = dup2_list_t::resolve_chain(chain); + do_test(!list.has_value()); +} + /// Testing colors. static void test_colors() { say(L"Testing colors"); @@ -5071,6 +5097,7 @@ int main(int argc, char **argv) { if (should_test_function("abbreviations")) test_abbreviations(); if (should_test_function("test")) test_test(); if (should_test_function("wcstod")) test_wcstod(); + if (should_test_function("dup2s")) test_dup2s(); if (should_test_function("path")) test_path(); if (should_test_function("pager_navigation")) test_pager_navigation(); if (should_test_function("pager_layout")) test_pager_layout(); diff --git a/src/io.cpp b/src/io.cpp index 5cc0b733c..091d250f5 100644 --- a/src/io.cpp +++ b/src/io.cpp @@ -163,11 +163,8 @@ void io_print(const io_chain_t &chain) } #endif -/// If the given fd is used by the io chain, duplicates it repeatedly until an fd not used in the io -/// chain is found, or we run out. If we return a new fd or an error, closes the old one. Any fd -/// created is marked close-on-exec. Returns -1 on failure (in which case the given fd is still -/// closed). -static int move_fd_to_unused(int fd, const io_chain_t &io_chain) { + +int move_fd_to_unused(int fd, const io_chain_t &io_chain, bool cloexec) { if (fd < 0 || io_chain.get_io_for_fd(fd).get() == NULL) { return fd; } @@ -188,7 +185,7 @@ static int move_fd_to_unused(int fd, const io_chain_t &io_chain) { // Ok, we have a new candidate fd. Recurse. If we get a valid fd, either it's the same as // what we gave it, or it's a new fd and what we gave it has been closed. If we get a // negative value, the fd also has been closed. - set_cloexec(tmp_fd); + if (cloexec) set_cloexec(tmp_fd); new_fd = move_fd_to_unused(tmp_fd, io_chain); } diff --git a/src/io.h b/src/io.h index 161ec5941..e0200c9f3 100644 --- a/src/io.h +++ b/src/io.h @@ -293,6 +293,12 @@ shared_ptr io_chain_get(io_chain_t &src, int fd); /// set to -1). bool pipe_avoid_conflicts_with_io_chain(int fds[2], const io_chain_t &ios); +/// If the given fd is used by the io chain, duplicates it repeatedly until an fd not used in the io +/// chain is found, or we run out. If we return a new fd or an error, closes the old one. +/// If \p cloexec is set, any fd created is marked close-on-exec. +/// \returns -1 on failure (in which case the given fd is still closed). +int move_fd_to_unused(int fd, const io_chain_t &io_chain, bool cloexec = true); + /// Class representing the output that a builtin can generate. class output_stream_t { private: diff --git a/src/redirection.cpp b/src/redirection.cpp new file mode 100644 index 000000000..1301f1e4a --- /dev/null +++ b/src/redirection.cpp @@ -0,0 +1,90 @@ +#include "config.h" // IWYU pragma: keep + +#include "redirection.h" +#include "wutil.h" + +#include + +/// File descriptor redirection error message. +#define FD_ERROR "An error occurred while redirecting file descriptor %s" + +/// Pipe error message. +#define LOCAL_PIPE_ERROR "An error occurred while setting up pipe" + +#define NOCLOB_ERROR _(L"The file '%s' already exists") + +#define FILE_ERROR _(L"An error occurred while redirecting file '%s'") + +/// Base open mode to pass to calls to open. +#define OPEN_MASK 0666 + +dup2_list_t::~dup2_list_t() = default; + +maybe_t dup2_list_t::resolve_chain(const io_chain_t &io_chain) { + ASSERT_IS_NOT_FORKED_CHILD(); + dup2_list_t result; + for (const auto &io_ref : io_chain) { + switch (io_ref->io_mode) { + case io_mode_t::file: { + // Here we definitely do not want to set CLO_EXEC because our child needs access. + // Open the file. + const io_file_t *io_file = static_cast(io_ref.get()); + int file_fd = open(io_file->filename_cstr, io_file->flags, OPEN_MASK); + if (file_fd < 0) { + if ((io_file->flags & O_EXCL) && (errno == EEXIST)) { + debug(1, NOCLOB_ERROR, io_file->filename_cstr); + } else { + debug(1, FILE_ERROR, io_file->filename_cstr); + if (should_debug(1)) wperror(L"open"); + } + return none(); + } + + // If by chance we got the file we want, we're done. Otherwise move the fd to an unused place and dup2 it. + // Note move_fd_to_unused() will close the incoming file_fd. + if (file_fd != io_file->fd) { + file_fd = move_fd_to_unused(file_fd, io_chain, false /* cloexec */); + if (file_fd < 0) { + debug(1, FILE_ERROR, io_file->filename_cstr); + if (should_debug(1)) wperror(L"dup"); + return none(); + } + } + + // Record that we opened this file, so we will auto-close it. + assert(file_fd >= 0 && "Should have a valid file_fd"); + result.opened_fds_.emplace_back(file_fd); + + // Mark our dup2 and our close actions. + result.add_dup2(file_fd, io_file->fd); + result.add_close(file_fd); + break; + } + + case io_mode_t::close: { + const io_close_t *io = static_cast(io_ref.get()); + result.add_close(io->fd); + break; + } + + case io_mode_t::fd: { + const io_fd_t *io = static_cast(io_ref.get()); + result.add_dup2(io->old_fd, io->fd); + break; + } + + case io_mode_t::buffer: + case io_mode_t::pipe: { + const io_pipe_t *io = static_cast(io_ref.get()); + // 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->is_input ? 0 : 1); + result.add_dup2(io->pipe_fd[write_pipe_idx], io->fd); + if (io->pipe_fd[0] >= 0) result.add_close(io->pipe_fd[0]); + if (io->pipe_fd[1] >= 0) result.add_close(io->pipe_fd[1]); + break; + } + } + } + return result; +} diff --git a/src/redirection.h b/src/redirection.h new file mode 100644 index 000000000..e82d4376d --- /dev/null +++ b/src/redirection.h @@ -0,0 +1,62 @@ +#ifndef FISH_REDIRECTION_H +#define FISH_REDIRECTION_H + +#include "common.h" +#include "maybe.h" +#include "io.h" + +#include + +/// This file supports "applying" redirections. + +/// A class representing a sequence of basic redirections. +class dup2_list_t { + /// A type that represents the action dup2(src, target). + /// If target is negative, this represents close(src). + /// Note none of the fds here are considered 'owned'. + struct action_t { + int src; + int target; + }; + + /// The list of actions. + std::vector actions_; + + /// The list of fds that we opened, and are responsible for closing. + std::vector opened_fds_; + + /// Append a dup2 action. + void add_dup2(int src, int target) { + assert(src >= 0 && target >= 0 && "Invalid fd in add_dup2"); + if (src != target) { + actions_.push_back(action_t{src, target}); + } + } + + /// Append a close action. + void add_close(int fd) { + assert(fd >= 0 && "Invalid fd in add_close"); + actions_.push_back(action_t{fd, -1}); + } + + dup2_list_t() = default; + +public: + ~dup2_list_t(); + + /// Disable copying because we own our fds. + dup2_list_t(const dup2_list_t &) = delete; + void operator=(const dup2_list_t &) = delete; + + dup2_list_t(dup2_list_t &&) = default; + dup2_list_t &operator=(dup2_list_t &&) = default; + + /// \return the list of dup2 actions. + const std::vector &get_actions() const { return actions_; } + + /// Produce a dup_fd_list_t from an io_chain. This may not be called before fork(). + /// The result contains the list of fd actions (dup2 and close), as well as the list of fds opened. + static maybe_t resolve_chain(const io_chain_t &); +}; + +#endif