Shell: Fix another FdRedirection reference leak

Add a create() factory function to prevent this from happening again.
lanmonster · Aug 12, 2020 · e8d6653 · e8d6653
1 parent 2b51250
commit e8d6653
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 13 deletions.
diff --git a/Shell/AST.cpp b/Shell/AST.cpp
@@ -718,7 +718,7 @@ void Fd2FdRedirection::dump(int level) const
 RefPtr<Value> Fd2FdRedirection::run(RefPtr<Shell>)
 {
  Command command;
- command.redirections.append(*new FdRedirection(source_fd, dest_fd, Rewiring::Close::None));
+ command.redirections.append(FdRedirection::create(source_fd, dest_fd, Rewiring::Close::None));
  return create<CommandValue>(move(command));
 }
 
@@ -889,7 +889,7 @@ RefPtr<Value> Execute::run(RefPtr<Shell> shell)
  }
  auto& last_in_commands = commands.last();
 
- last_in_commands.redirections.prepend(adopt(*new FdRedirection(STDOUT_FILENO, pipefd[1], Rewiring::Close::Destination)));
+ last_in_commands.redirections.prepend(FdRedirection::create(STDOUT_FILENO, pipefd[1], Rewiring::Close::Destination));
  last_in_commands.should_wait = true;
  last_in_commands.should_notify_if_in_background = false;
  last_in_commands.is_pipe_source = false;
@@ -1135,10 +1135,10 @@ RefPtr<Value> Pipe::run(RefPtr<Shell> shell)
  auto last_in_left = left.take_last();
  auto first_in_right = right.take_first();
 
- auto pipe_write_end = new FdRedirection(STDIN_FILENO, -1, Rewiring::Close::Destination);
- auto pipe_read_end = new FdRedirection(STDOUT_FILENO, -1, pipe_write_end, Rewiring::Close::RefreshDestination);
- first_in_right.redirections.append(adopt(*pipe_write_end));
- last_in_left.redirections.append(adopt(*pipe_read_end));
+ auto pipe_write_end = FdRedirection::create(STDIN_FILENO, -1, Rewiring::Close::Destination);
+ auto pipe_read_end = FdRedirection::create(STDOUT_FILENO, -1, pipe_write_end, Rewiring::Close::RefreshDestination);
+ first_in_right.redirections.append(pipe_write_end);
+ last_in_left.redirections.append(pipe_read_end);
  last_in_left.should_wait = false;
  last_in_left.is_pipe_source = true;
 

diff --git a/Shell/AST.h b/Shell/AST.h
@@ -123,11 +123,30 @@ struct PathRedirection : public Redirection {
 };
 
 struct FdRedirection : public Redirection {
+public:
+ static NonnullRefPtr<FdRedirection> create(int source, int dest, Rewiring::Close close)
+ {
+ return adopt(*new FdRedirection(source, dest, close));
+ }
+
+ static NonnullRefPtr<FdRedirection> create(int source, int dest, FdRedirection* pipe_end, Rewiring::Close close)
+ {
+ return adopt(*new FdRedirection(source, dest, 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));
  }
- virtual ~FdRedirection();
+
+ int source_fd { -1 };
+ int dest_fd { -1 };
+ FdRedirection* other_pipe_end { nullptr };
+ Rewiring::Close action { Rewiring::Close::None };
+
+private:
  FdRedirection(int source, int dest, Rewiring::Close close)
  : FdRedirection(source, dest, nullptr, close)
  {
@@ -141,12 +160,6 @@ struct FdRedirection : public Redirection {
  {
  }
 
- int source_fd { -1 };
- int dest_fd { -1 };
- FdRedirection* other_pipe_end { nullptr };
- Rewiring::Close action { Rewiring::Close::None };
-
-private:
  virtual bool is_fd_redirection() const override { return true; }
 };