From 1f83fb47ce0830ee46cf8d3abd95198f7f03f1a5 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Thu, 12 Dec 2019 17:37:36 -0800
Subject: [PATCH] Finish the IO cleanup.

Remove some dead code and add missing dtors.
---
 src/exec.cpp        | 32 +++++++-------------------------
 src/io.cpp          |  4 ++--
 src/io.h            |  3 +++
 src/redirection.cpp |  7 -------
 src/redirection.h   |  5 +----
 5 files changed, 13 insertions(+), 38 deletions(-)

diff --git a/src/exec.cpp b/src/exec.cpp
index 226907953..c5a47e460 100644
--- a/src/exec.cpp
+++ b/src/exec.cpp
@@ -50,12 +50,6 @@
 /// File descriptor redirection error message.
 #define FD_ERROR _(L"An error occurred while redirecting file descriptor %d")
 
-/// File redirection error message.
-#define FILE_ERROR _(L"An error occurred while redirecting file '%ls'")
-
-/// Base open mode to pass to calls to open.
-#define OPEN_MASK 0666
-
 /// Number of calls to fork() or posix_spawn().
 static relaxed_atomic_t<int> s_fork_count{0};
 
@@ -69,9 +63,11 @@ void exec_close(int fd) {
     }
 
     while (close(fd) == -1) {
-        debug(1, FD_ERROR, fd);
-        wperror(L"close");
-        break;
+        if (errno != EINTR) {
+            debug(1, FD_ERROR, fd);
+            wperror(L"close");
+            break;
+        }
     }
 }
 
@@ -181,20 +177,6 @@ static void launch_process_nofork(env_stack_t &vars, process_t *p) {
     safe_launch_process(p, actual_cmd, argv_array.get(), envv);
 }
 
-/// Morph an io redirection chain into redirections suitable for passing to eval, and then call
-/// eval.
-///
-/// \param parsed_source the parsed source code containing the node to evaluate
-/// \param node the node to evaluate
-/// \param ios the io redirections to be performed on this block
-template <typename T>
-void internal_exec_helper(parser_t &parser, parsed_source_ref_t parsed_source, tnode_t<T> node,
-                          job_lineage_t lineage) {
-    assert(parsed_source && node && "exec_helper missing source or without node");
-    parser.eval_node(parsed_source, node, TOP, std::move(lineage));
-    job_reap(parser, false);
-}
-
 // Returns whether we can use posix spawn for a given process in a given job.
 //
 // To avoid the race between the caller calling tcsetpgrp() and the client checking the
@@ -761,7 +743,7 @@ static proc_performer_t get_performer_for_process(process_t *p, const job_t *job
         tnode_t<grammar::statement> node = p->internal_block_node;
         assert(source && node && "Process is missing node info");
         return [=](parser_t &parser) {
-            internal_exec_helper(parser, source, node, lineage);
+            parser.eval_node(source, node, TOP, lineage);
             int status = parser.get_last_status();
             // FIXME: setting the status this way is dangerous nonsense, we need to decode the
             // status properly if it was a signal.
@@ -779,7 +761,7 @@ static proc_performer_t get_performer_for_process(process_t *p, const job_t *job
             const auto &ld = parser.libdata();
             auto saved_exec_count = ld.exec_count;
             const block_t *fb = function_prepare_environment(parser, *argv, *props);
-            internal_exec_helper(parser, props->parsed_source, props->body_node, lineage);
+            parser.eval_node(props->parsed_source, props->body_node, TOP, lineage);
             function_restore_environment(parser, fb);
 
             // If the function did not execute anything, treat it as success.
diff --git a/src/io.cpp b/src/io.cpp
index 2ff31bd90..03797bf91 100644
--- a/src/io.cpp
+++ b/src/io.cpp
@@ -208,9 +208,9 @@ std::shared_ptr<io_buffer_t> io_bufferfill_t::finish(std::shared_ptr<io_bufferfi
 }
 
 io_pipe_t::~io_pipe_t() = default;
-
+io_fd_t::~io_fd_t() = default;
+io_close_t::~io_close_t() = default;
 io_file_t::~io_file_t() = default;
-
 io_bufferfill_t::~io_bufferfill_t() = default;
 
 io_buffer_t::~io_buffer_t() {
diff --git a/src/io.h b/src/io.h
index 24d5e6da9..6f569f08d 100644
--- a/src/io.h
+++ b/src/io.h
@@ -192,6 +192,7 @@ class io_close_t : public io_data_t {
     explicit io_close_t(int f) : io_data_t(io_mode_t::close, f) {}
 
     void print() const override;
+    ~io_close_t() override;
 };
 
 class io_fd_t : public io_data_t {
@@ -206,6 +207,8 @@ class io_fd_t : public io_data_t {
 
     void print() const override;
 
+    ~io_fd_t() override;
+
     io_fd_t(int f, int old, bool us)
         : io_data_t(io_mode_t::fd, f), old_fd(old), user_supplied(us) {}
 };
diff --git a/src/redirection.cpp b/src/redirection.cpp
index 4e0028b92..940ddc014 100644
--- a/src/redirection.cpp
+++ b/src/redirection.cpp
@@ -7,13 +7,6 @@
 #include "io.h"
 #include "wutil.h"
 
-#define NOCLOB_ERROR _(L"The file '%ls' already exists")
-
-#define FILE_ERROR _(L"An error occurred while redirecting file '%ls'")
-
-/// Base open mode to pass to calls to open.
-#define OPEN_MASK 0666
-
 dup2_list_t::~dup2_list_t() = default;
 
 maybe_t<int> redirection_spec_t::get_target_as_fd() const {
diff --git a/src/redirection.h b/src/redirection.h
index 0d43e0882..c8a87e700 100644
--- a/src/redirection.h
+++ b/src/redirection.h
@@ -63,9 +63,6 @@ class dup2_list_t {
     /// The list of actions.
     std::vector<action_t> actions_;
 
-    /// The list of fds that we opened, and are responsible for closing.
-    std::vector<autoclose_fd_t> opened_fds_;
-
     /// Append a dup2 action.
     void add_dup2(int src, int target) {
         assert(src >= 0 && target >= 0 && "Invalid fd in add_dup2");
@@ -85,7 +82,7 @@ class dup2_list_t {
    public:
     ~dup2_list_t();
 
-    /// Disable copying because we own our fds.
+    /// Disable copying.
     dup2_list_t(const dup2_list_t &) = delete;
     void operator=(const dup2_list_t &) = delete;