Move autoclose_pipes_t from io.h to fds.h

src/exec.h

@@ -9,9 +9,6 @@
 #include "common.h"
 #include "proc.h"
 
-/// Pipe redirection error message.
-#define PIPE_ERROR _(L"An error occurred while setting up pipe")
-
 /// Execute the processes specified by \p j in the parser \p.
 /// On a true return, the job was successfully launched and the parser will take responsibility for
 /// cleaning it up. On a false return, the job could not be launched and the caller must clean it

src/fds.cpp

@@ -7,6 +7,7 @@
 #include <errno.h>
 #include <unistd.h>
 
+#include "flog.h"
 #include "wutil.h"
 
 void autoclose_fd_t::close() {
@@ -15,6 +16,53 @@ void autoclose_fd_t::close() {
     fd_ = -1;
 }
 
+autoclose_fd_t move_fd_to_unused(autoclose_fd_t fd, const fd_set_t &fdset) {
+    if (!fd.valid() || !fdset.contains(fd.fd())) {
+        return fd;
+    }
+
+    // We have fd >= 0, and it's a conflict. dup it and recurse. Note that we recurse before
+    // anything is closed; this forces the kernel to give us a new one (or report fd exhaustion).
+    int tmp_fd;
+    do {
+        tmp_fd = dup(fd.fd());
+    } while (tmp_fd < 0 && errno == EINTR);
+
+    assert(tmp_fd != fd.fd());
+    if (tmp_fd < 0) {
+        // Likely fd exhaustion.
+        return autoclose_fd_t{};
+    }
+    // Ok, we have a new candidate fd. Recurse.
+    set_cloexec(tmp_fd);
+    return move_fd_to_unused(autoclose_fd_t{tmp_fd}, fdset);
+}
+
+maybe_t<autoclose_pipes_t> make_autoclose_pipes(const fd_set_t &fdset) {
+    int pipes[2] = {-1, -1};
+
+    if (pipe(pipes) < 0) {
+        FLOGF(warning, PIPE_ERROR);
+        wperror(L"pipe");
+        return none();
+    }
+    set_cloexec(pipes[0]);
+    set_cloexec(pipes[1]);
+
+    autoclose_fd_t read_end{pipes[0]};
+    autoclose_fd_t write_end{pipes[1]};
+
+    // Ensure we have no conflicts.
+    if (!fdset.empty()) {
+        read_end = move_fd_to_unused(std::move(read_end), fdset);
+        if (!read_end.valid()) return none();
+
+        write_end = move_fd_to_unused(std::move(write_end), fdset);
+        if (!write_end.valid()) return none();
+    }
+    return autoclose_pipes_t(std::move(read_end), std::move(write_end));
+}
+
 void exec_close(int fd) {
     assert(fd >= 0 && "Invalid fd");
     while (close(fd) == -1) {

src/fds.h

@@ -4,6 +4,12 @@
 #define FISH_FDS_H
 
 #include <algorithm>
+#include <vector>
+
+#include "maybe.h"
+
+/// Pipe redirection error message.
+#define PIPE_ERROR _(L"An error occurred while setting up pipe")
 
 /// A helper class for managing and automatically closing a file descriptor.
 class autoclose_fd_t {
@@ -46,6 +52,49 @@ class autoclose_fd_t {
     ~autoclose_fd_t() { close(); }
 };
 
+/// Helper type returned from making autoclose pipes.
+struct autoclose_pipes_t {
+    /// Read end of the pipe.
+    autoclose_fd_t read;
+
+    /// Write end of the pipe.
+    autoclose_fd_t write;
+
+    autoclose_pipes_t() = default;
+    autoclose_pipes_t(autoclose_fd_t r, autoclose_fd_t w)
+        : read(std::move(r)), write(std::move(w)) {}
+};
+
+/// A simple set of FDs.
+struct fd_set_t {
+    std::vector<bool> fds;
+
+    void add(int fd) {
+        assert(fd >= 0 && "Invalid fd");
+        if (static_cast<size_t>(fd) >= fds.size()) {
+            fds.resize(fd + 1);
+        }
+        fds[fd] = true;
+    }
+
+    bool contains(int fd) const {
+        assert(fd >= 0 && "Invalid fd");
+        return static_cast<size_t>(fd) < fds.size() && fds[fd];
+    }
+
+    bool empty() const { return fds.empty(); }
+};
+
+/// Call pipe(), populating autoclose fds, avoiding conflicts.
+/// The pipes are marked CLO_EXEC.
+/// \return pipes on success, none() on error.
+maybe_t<autoclose_pipes_t> make_autoclose_pipes(const fd_set_t &fdset);
+
+/// If the given fd is present in \p fdset, duplicates it repeatedly until an fd not used in the set
+/// is found or we run out. If we return a new fd or an error, closes the old one. Marks the fd as
+/// cloexec. \returns invalid fd on failure (in which case the given fd is still closed).
+autoclose_fd_t move_fd_to_unused(autoclose_fd_t fd, const fd_set_t &fdset);
+
 /// Close a file descriptor \p fd, retrying on EINTR.
 void exec_close(int fd);
 

src/io.cpp

@@ -284,53 +284,6 @@ fd_set_t io_chain_t::fd_set() const {
     return result;
 }
 
-autoclose_fd_t move_fd_to_unused(autoclose_fd_t fd, const fd_set_t &fdset) {
-    if (!fd.valid() || !fdset.contains(fd.fd())) {
-        return fd;
-    }
-
-    // We have fd >= 0, and it's a conflict. dup it and recurse. Note that we recurse before
-    // anything is closed; this forces the kernel to give us a new one (or report fd exhaustion).
-    int tmp_fd;
-    do {
-        tmp_fd = dup(fd.fd());
-    } while (tmp_fd < 0 && errno == EINTR);
-
-    assert(tmp_fd != fd.fd());
-    if (tmp_fd < 0) {
-        // Likely fd exhaustion.
-        return autoclose_fd_t{};
-    }
-    // Ok, we have a new candidate fd. Recurse.
-    set_cloexec(tmp_fd);
-    return move_fd_to_unused(autoclose_fd_t{tmp_fd}, fdset);
-}
-
-maybe_t<autoclose_pipes_t> make_autoclose_pipes(const fd_set_t &fdset) {
-    int pipes[2] = {-1, -1};
-
-    if (pipe(pipes) < 0) {
-        FLOGF(warning, PIPE_ERROR);
-        wperror(L"pipe");
-        return none();
-    }
-    set_cloexec(pipes[0]);
-    set_cloexec(pipes[1]);
-
-    autoclose_fd_t read_end{pipes[0]};
-    autoclose_fd_t write_end{pipes[1]};
-
-    // Ensure we have no conflicts.
-    if (!fdset.empty()) {
-        read_end = move_fd_to_unused(std::move(read_end), fdset);
-        if (!read_end.valid()) return none();
-
-        write_end = move_fd_to_unused(std::move(write_end), fdset);
-        if (!write_end.valid()) return none();
-    }
-    return autoclose_pipes_t(std::move(read_end), std::move(write_end));
-}
-
 shared_ptr<const io_data_t> io_chain_t::io_for_fd(int fd) const {
     for (auto iter = rbegin(); iter != rend(); ++iter) {
         const auto &data = *iter;

src/io.h

@@ -24,26 +24,6 @@ using std::shared_ptr;
 
 class job_group_t;
 
-/// A simple set of FDs.
-struct fd_set_t {
-    std::vector<bool> fds;
-
-    void add(int fd) {
-        assert(fd >= 0 && "Invalid fd");
-        if (static_cast<size_t>(fd) >= fds.size()) {
-            fds.resize(fd + 1);
-        }
-        fds[fd] = true;
-    }
-
-    bool contains(int fd) const {
-        assert(fd >= 0 && "Invalid fd");
-        return static_cast<size_t>(fd) < fds.size() && fds[fd];
-    }
-
-    bool empty() const { return fds.empty(); }
-};
-
 /// separated_buffer_t represents a buffer of output from commands, prepared to be turned into a
 /// variable. For example, command substitutions output into one of these. Most commands just
 /// produce a stream of bytes, and those get stored directly. However other commands produce
@@ -51,7 +31,6 @@ struct fd_set_t {
 /// The buffer tracks a sequence of elements. Some elements are explicitly separated and should not
 /// be further split; other elements have inferred separation and may be split by IFS (or not,
 /// depending on its value).
-
 enum class separation_type_t {
     inferred,    // this element should be further separated by IFS
     explicitly,  // this element is explicitly separated and should not be further split
@@ -383,28 +362,6 @@ class io_chain_t : public std::vector<io_data_ref_t> {
     fd_set_t fd_set() const;
 };
 
-/// Helper type returned from making autoclose pipes.
-struct autoclose_pipes_t {
-    /// Read end of the pipe.
-    autoclose_fd_t read;
-
-    /// Write end of the pipe.
-    autoclose_fd_t write;
-
-    autoclose_pipes_t() = default;
-    autoclose_pipes_t(autoclose_fd_t r, autoclose_fd_t w)
-        : read(std::move(r)), write(std::move(w)) {}
-};
-/// Call pipe(), populating autoclose fds, avoiding conflicts.
-/// The pipes are marked CLO_EXEC.
-/// \return pipes on success, none() on error.
-maybe_t<autoclose_pipes_t> make_autoclose_pipes(const fd_set_t &fdset);
-
-/// If the given fd is present in \p fdset, duplicates it repeatedly until an fd not used in the set
-/// is found or we run out. If we return a new fd or an error, closes the old one. Marks the fd as
-/// cloexec. \returns invalid fd on failure (in which case the given fd is still closed).
-autoclose_fd_t move_fd_to_unused(autoclose_fd_t fd, const fd_set_t &fdset);
-
 /// Base class representing the output that a builtin can generate.
 /// This has various subclasses depending on the ultimate output destination.
 class output_stream_t {