Shell: Rename {source,dest}_fd to {old,new}_fd

This makes `Rewiring' much more understandable, and un-confuses the uses
of `dup2()'.
Also fixes `dup2()' bugs.

Shell/AST.cpp

@@ -852,13 +852,13 @@ DynamicEvaluate::~DynamicEvaluate()
 void Fd2FdRedirection::dump(int level) const
 {
     Node::dump(level);
-    print_indented(String::format("%d -> %d", m_source_fd, m_dest_fd), level);
+    print_indented(String::format("%d -> %d", m_old_fd, m_new_fd), level);
 }
 
 RefPtr<Value> Fd2FdRedirection::run(RefPtr<Shell>)
 {
     Command command;
-    command.redirections.append(FdRedirection::create(m_source_fd, m_dest_fd, Rewiring::Close::None));
+    command.redirections.append(FdRedirection::create(m_new_fd, m_old_fd, Rewiring::Close::None));
     return create<CommandValue>(move(command));
 }
 
@@ -869,8 +869,8 @@ void Fd2FdRedirection::highlight_in_editor(Line::Editor& editor, Shell&, Highlig
 
 Fd2FdRedirection::Fd2FdRedirection(Position position, int src, int dst)
     : Node(move(position))
-    , m_source_fd(src)
-    , m_dest_fd(dst)
+    , m_old_fd(src)
+    , m_new_fd(dst)
 {
 }
 
@@ -1120,7 +1120,7 @@ void Execute::for_each_entry(RefPtr<Shell> shell, Function<IterationDecision(Non
         }
         auto& last_in_commands = commands.last();
 
-        last_in_commands.redirections.prepend(FdRedirection::create(STDOUT_FILENO, pipefd[1], Rewiring::Close::Destination));
+        last_in_commands.redirections.prepend(FdRedirection::create(pipefd[1], STDOUT_FILENO, Rewiring::Close::Old));
         last_in_commands.should_wait = false;
         last_in_commands.should_notify_if_in_background = false;
         last_in_commands.is_pipe_source = false;
@@ -1685,8 +1685,8 @@ RefPtr<Value> Pipe::run(RefPtr<Shell> shell)
     auto last_in_left = left.take_last();
     auto first_in_right = right.take_first();
 
-    auto pipe_read_end = FdRedirection::create(STDIN_FILENO, -1, Rewiring::Close::Destination);
-    auto pipe_write_end = FdRedirection::create(STDOUT_FILENO, -1, pipe_read_end, Rewiring::Close::RefreshDestination);
+    auto pipe_read_end = FdRedirection::create(-1, STDIN_FILENO, Rewiring::Close::Old);
+    auto pipe_write_end = FdRedirection::create(-1, STDOUT_FILENO, pipe_read_end, Rewiring::Close::RefreshOld);
     first_in_right.redirections.append(pipe_read_end);
     last_in_left.redirections.append(pipe_write_end);
     last_in_left.should_wait = false;
@@ -2810,7 +2810,7 @@ Vector<String> TildeValue::resolve_as_list(RefPtr<Shell> shell)
 
 Result<NonnullRefPtr<Rewiring>, String> CloseRedirection::apply() const
 {
-    return adopt(*new Rewiring(fd, fd, Rewiring::Close::ImmediatelyCloseDestination));
+    return adopt(*new Rewiring(fd, fd, Rewiring::Close::ImmediatelyCloseNew));
 }
 
 CloseRedirection::~CloseRedirection()
@@ -2825,7 +2825,7 @@ Result<NonnullRefPtr<Rewiring>, String> PathRedirection::apply() const
             dbg() << "open() failed for '" << path << "' with " << error;
             return error;
         }
-        return adopt(*new Rewiring(my_fd, fd, Rewiring::Close::Destination));
+        return adopt(*new Rewiring(fd, my_fd, Rewiring::Close::Old));
     };
     switch (direction) {
     case AST::PathRedirection::WriteAppend:

Shell/AST.h

@@ -63,27 +63,28 @@ struct Position {
 
 struct FdRedirection;
 struct Rewiring : public RefCounted<Rewiring> {
-    int source_fd { -1 };
-    int dest_fd { -1 };
+    int old_fd { -1 };
+    int new_fd { -1 };
     FdRedirection* other_pipe_end { nullptr };
     enum class Close {
         None,
-        Source,
-        Destination,
-        RefreshDestination,
-        ImmediatelyCloseDestination,
+        Old,
+        New,
+        RefreshNew,
+        RefreshOld,
+        ImmediatelyCloseNew,
     } fd_action { Close::None };
 
     Rewiring(int source, int dest, Close close = Close::None)
-        : source_fd(source)
-        , dest_fd(dest)
+        : old_fd(source)
+        , new_fd(dest)
         , fd_action(close)
     {
     }
 
     Rewiring(int source, int dest, FdRedirection* other_end, Close close)
-        : source_fd(source)
-        , dest_fd(dest)
+        : old_fd(source)
+        , new_fd(dest)
         , other_pipe_end(other_end)
         , fd_action(close)
     {
@@ -143,25 +144,25 @@ private:
 
 struct FdRedirection : public Redirection {
 public:
-    static NonnullRefPtr<FdRedirection> create(int source, int dest, Rewiring::Close close)
+    static NonnullRefPtr<FdRedirection> create(int old_fd, int new_fd, Rewiring::Close close)
     {
-        return adopt(*new FdRedirection(source, dest, close));
+        return adopt(*new FdRedirection(old_fd, new_fd, close));
     }
 
-    static NonnullRefPtr<FdRedirection> create(int source, int dest, FdRedirection* pipe_end, Rewiring::Close close)
+    static NonnullRefPtr<FdRedirection> create(int old_fd, int new_fd, FdRedirection* pipe_end, Rewiring::Close close)
     {
-        return adopt(*new FdRedirection(source, dest, pipe_end, close));
+        return adopt(*new FdRedirection(old_fd, new_fd, pipe_end, close));
     }
 
     virtual ~FdRedirection();
 
     virtual Result<NonnullRefPtr<Rewiring>, String> apply() const override
     {
-        return adopt(*new Rewiring(source_fd, dest_fd, other_pipe_end, action));
+        return adopt(*new Rewiring(old_fd, new_fd, other_pipe_end, action));
     }
 
-    int source_fd { -1 };
-    int dest_fd { -1 };
+    int old_fd { -1 };
+    int new_fd { -1 };
     FdRedirection* other_pipe_end { nullptr };
     Rewiring::Close action { Rewiring::Close::None };
 
@@ -171,9 +172,9 @@ private:
     {
     }
 
-    FdRedirection(int source, int dest, FdRedirection* pipe_end, Rewiring::Close close)
-        : source_fd(source)
-        , dest_fd(dest)
+    FdRedirection(int old_fd, int new_fd, FdRedirection* pipe_end, Rewiring::Close close)
+        : old_fd(old_fd)
+        , new_fd(new_fd)
         , other_pipe_end(pipe_end)
         , action(close)
     {
@@ -748,8 +749,8 @@ public:
     virtual ~Fd2FdRedirection();
     virtual void visit(NodeVisitor& visitor) override { visitor.visit(this); }
 
-    int source_fd() const { return m_source_fd; }
-    int dest_fd() const { return m_dest_fd; }
+    int source_fd() const { return m_old_fd; }
+    int dest_fd() const { return m_new_fd; }
 
 private:
     NODE(Fd2FdRedirection);
@@ -758,8 +759,8 @@ private:
     virtual void highlight_in_editor(Line::Editor&, Shell&, HighlightMetadata = {}) override;
     virtual bool is_command() const override { return true; }
 
-    int m_source_fd { -1 };
-    int m_dest_fd { -1 };
+    int m_old_fd { -1 };
+    int m_new_fd { -1 };
 };
 
 class FunctionDeclaration final : public Node {

Shell/Builtin.cpp

@@ -823,7 +823,7 @@ bool Shell::run_builtin(const AST::Command& command, const NonnullRefPtrVector<A
     SavedFileDescriptors fds { rewirings };
 
     for (auto& rewiring : rewirings) {
-        int rc = dup2(rewiring.dest_fd, rewiring.source_fd);
+        int rc = dup2(rewiring.old_fd, rewiring.new_fd);
         if (rc < 0) {
             perror("dup2(run)");
             return false;

Shell/Shell.cpp

@@ -582,17 +582,17 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
         }
         auto& rewiring = rewiring_result.value();
 
-        if (rewiring->fd_action != AST::Rewiring::Close::ImmediatelyCloseDestination)
+        if (rewiring->fd_action != AST::Rewiring::Close::ImmediatelyCloseNew)
             rewirings.append(*rewiring);
 
-        if (rewiring->fd_action == AST::Rewiring::Close::Source) {
-            fds.add(rewiring->source_fd);
-        } else if (rewiring->fd_action == AST::Rewiring::Close::Destination) {
-            if (rewiring->dest_fd != -1)
-                fds.add(rewiring->dest_fd);
-        } else if (rewiring->fd_action == AST::Rewiring::Close::ImmediatelyCloseDestination) {
-            fds.add(rewiring->dest_fd);
-        } else if (rewiring->fd_action == AST::Rewiring::Close::RefreshDestination) {
+        if (rewiring->fd_action == AST::Rewiring::Close::Old) {
+            fds.add(rewiring->old_fd);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::New) {
+            if (rewiring->new_fd != -1)
+                fds.add(rewiring->new_fd);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::ImmediatelyCloseNew) {
+            fds.add(rewiring->new_fd);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::RefreshNew) {
             ASSERT(rewiring->other_pipe_end);
 
             int pipe_fd[2];
@@ -601,8 +601,20 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
                 perror("pipe(RedirRefresh)");
                 return IterationDecision::Break;
             }
-            rewiring->dest_fd = pipe_fd[1];
-            rewiring->other_pipe_end->dest_fd = pipe_fd[0]; // This fd will be added to the collection on one of the next iterations.
+            rewiring->new_fd = pipe_fd[1];
+            rewiring->other_pipe_end->new_fd = pipe_fd[0]; // This fd will be added to the collection on one of the next iterations.
+            fds.add(pipe_fd[1]);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::RefreshOld) {
+            ASSERT(rewiring->other_pipe_end);
+
+            int pipe_fd[2];
+            int rc = pipe(pipe_fd);
+            if (rc < 0) {
+                perror("pipe(RedirRefresh)");
+                return IterationDecision::Break;
+            }
+            rewiring->old_fd = pipe_fd[1];
+            rewiring->other_pipe_end->old_fd = pipe_fd[0]; // This fd will be added to the collection on one of the next iterations.
             fds.add(pipe_fd[1]);
         }
         return IterationDecision::Continue;
@@ -611,17 +623,24 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
     auto apply_rewirings = [&] {
         for (auto& rewiring : rewirings) {
 #ifdef SH_DEBUG
-            dbgprintf("in %s<%d>, dup2(%d, %d)\n", command.argv.is_empty() ? "(<Empty>)" : command.argv[0].characters(), getpid(), rewiring.dest_fd, rewiring.source_fd);
+            dbgprintf("in %s<%d>, dup2(%d, %d)\n", command.argv.is_empty() ? "(<Empty>)" : command.argv[0].characters(), getpid(), rewiring.old_fd, rewiring.new_fd);
 #endif
-            int rc = dup2(rewiring.dest_fd, rewiring.source_fd);
+            int rc = dup2(rewiring.old_fd, rewiring.new_fd);
             if (rc < 0) {
                 perror("dup2(run)");
                 return IterationDecision::Break;
             }
-            // dest_fd is closed via the `fds` collector, but rewiring.other_pipe_end->dest_fd
+            // {new,old}_fd is closed via the `fds` collector, but rewiring.other_pipe_end->{new,old}_fd
             // isn't yet in that collector when the first child spawns.
-            if (rewiring.other_pipe_end && close(rewiring.other_pipe_end->dest_fd) < 0)
-                perror("close other pipe end");
+            if (rewiring.other_pipe_end) {
+                if (rewiring.fd_action == AST::Rewiring::Close::RefreshNew) {
+                    if (rewiring.other_pipe_end && close(rewiring.other_pipe_end->new_fd) < 0)
+                        perror("close other pipe end");
+                } else if (rewiring.fd_action == AST::Rewiring::Close::RefreshOld) {
+                    if (rewiring.other_pipe_end && close(rewiring.other_pipe_end->old_fd) < 0)
+                        perror("close other pipe end");
+                }
+            }
         }
 
         return IterationDecision::Continue;
@@ -648,7 +667,7 @@ RefPtr<Job> Shell::run_command(const AST::Command& command)
         SavedFileDescriptors fds { rewirings };
 
         for (auto& rewiring : rewirings) {
-            int rc = dup2(rewiring.dest_fd, rewiring.source_fd);
+            int rc = dup2(rewiring.old_fd, rewiring.new_fd);
             if (rc < 0) {
                 perror("dup2(run)");
                 return nullptr;
@@ -878,7 +897,7 @@ NonnullRefPtrVector<Job> Shell::run_commands(Vector<AST::Command>& commands)
                 dbg() << "redir path " << (int)path_redir->direction << " " << path_redir->path << " <-> " << path_redir->fd;
             } else if (redir.is_fd_redirection()) {
                 auto* fdredir = (const AST::FdRedirection*)&redir;
-                dbg() << "redir fd " << fdredir->source_fd << " -> " << fdredir->dest_fd;
+                dbg() << "redir fd " << fdredir->old_fd << " -> " << fdredir->new_fd;
             } else if (redir.is_close_redirection()) {
                 auto close_redir = (const AST::CloseRedirection*)&redir;
                 dbg() << "close fd " << close_redir->fd;
@@ -1662,7 +1681,7 @@ void FileDescriptionCollector::add(int fd)
 SavedFileDescriptors::SavedFileDescriptors(const NonnullRefPtrVector<AST::Rewiring>& intended_rewirings)
 {
     for (auto& rewiring : intended_rewirings) {
-        int new_fd = dup(rewiring.source_fd);
+        int new_fd = dup(rewiring.new_fd);
         if (new_fd < 0) {
             if (errno != EBADF)
                 perror("dup");
@@ -1676,7 +1695,7 @@ SavedFileDescriptors::SavedFileDescriptors(const NonnullRefPtrVector<AST::Rewiri
         auto rc = fcntl(new_fd, F_SETFL, flags | FD_CLOEXEC);
         ASSERT(rc == 0);
 
-        m_saves.append({ rewiring.source_fd, new_fd });
+        m_saves.append({ rewiring.new_fd, new_fd });
         m_collector.add(new_fd);
     }
 }