From 3f9706a7f350fcac43fc389b0b52b3601269e2e9 Mon Sep 17 00:00:00 2001 From: Cheer Xiao Date: Mon, 4 Feb 2013 20:07:16 +0800 Subject: [PATCH] Make io_data_t::fd const In exec(), pipe_{write,read} no longer get reused. --- exec.cpp | 53 +++++++++++++++++++++-------------------------------- io.cpp | 8 ++++++-- io.h | 6 ++++-- reader.cpp | 8 ++------ 4 files changed, 33 insertions(+), 42 deletions(-) diff --git a/exec.cpp b/exec.cpp index 7fc94b0c7..11bf08145 100644 --- a/exec.cpp +++ b/exec.cpp @@ -622,15 +622,6 @@ void exec(parser_t &parser, job_t *j) } - // This is a pipe that the "current" process in our loop below reads from - // Only pipe_read->pipe_fd[0] is used - shared_ptr pipe_read(new io_pipe_t(0, true)); - - // This is the pipe that the "current" process in our loop below writes to - shared_ptr pipe_write(new io_pipe_t(1, false)); - - j->io.push_back(pipe_write); - signal_block(); /* @@ -714,16 +705,29 @@ void exec(parser_t &parser, job_t *j) pipe_current_read = pipe_next_read; pipe_next_read = -1; - /* Record the current read in pipe_read */ - pipe_read->pipe_fd[0] = pipe_current_read; - /* See if we need a pipe */ const bool pipes_to_next_command = (p->next != NULL); - pipe_write->fd = p->pipe_write_fd; - pipe_read->fd = p->pipe_read_fd; + /* The pipes the current process write to and read from. + Unfortunately these can't be just allocated on the stack, since + j->io wants shared_ptr. */ + shared_ptr pipe_write(new io_pipe_t(p->pipe_write_fd, false)); + shared_ptr pipe_read(new io_pipe_t(p->pipe_read_fd, true)); + + /* Record the current read in pipe_read */ + pipe_read->pipe_fd[0] = pipe_current_read; + // debug( 0, L"Pipe created from fd %d to fd %d", pipe_write->fd, pipe_read->fd ); + if (p != j->first_process) + { + j->io.push_back(pipe_read); + } + + if (p->next) + { + j->io.push_back(pipe_write); + } /* This call is used so the global environment variable array @@ -742,12 +746,6 @@ void exec(parser_t &parser, job_t *j) Set up fd:s that will be used in the pipe */ - if (p == j->first_process->next) - { - /* We are the first process that could possibly read from a pipe (aka the second process), so add the pipe read redirection */ - j->io.push_back(pipe_read); - } - if (pipes_to_next_command) { // debug( 1, L"%ls|%ls" , p->argv[0], p->next->argv[0]); @@ -772,16 +770,6 @@ void exec(parser_t &parser, job_t *j) assert(pipe_next_read == -1); pipe_next_read = local_pipe[0]; } - else - { - /* - This is the last element of the pipeline. - Remove the io redirection for pipe output. - */ - io_chain_t::iterator where = std::find(j->io.begin(), j->io.end(), pipe_write); - if (where != j->io.end()) - j->io.erase(where); - } switch (p->type) { @@ -1396,6 +1384,9 @@ void exec(parser_t &parser, job_t *j) exec_close(pipe_current_write); pipe_current_write = -1; } + + j->io.remove(pipe_write); + j->io.remove(pipe_read); } /* Clean up any file descriptors we left open */ @@ -1416,8 +1407,6 @@ void exec(parser_t &parser, job_t *j) debug(3, L"Job is constructed"); - io_remove(j->io, pipe_read); - job_set_flag(j, JOB_CONSTRUCTED, 1); if (!job_get_flag(j, JOB_FOREGROUND)) diff --git a/io.cpp b/io.cpp index f1d664f8f..669752fd9 100644 --- a/io.cpp +++ b/io.cpp @@ -131,10 +131,14 @@ void io_buffer_t::read() } -io_buffer_t *io_buffer_t::create(bool is_input) +io_buffer_t *io_buffer_t::create(bool is_input, int fd) { bool success = true; - io_buffer_t *buffer_redirect = new io_buffer_t(is_input ? 0 : 1, is_input); + if (fd == -1) + { + fd = is_input ? 0 : 1; + } + io_buffer_t *buffer_redirect = new io_buffer_t(fd, is_input); if (exec_pipe(buffer_redirect->pipe_fd) == -1) { diff --git a/io.h b/io.h index b007bbf65..a156c0e18 100644 --- a/io.h +++ b/io.h @@ -32,7 +32,7 @@ class io_data_t /** Type of redirect */ const io_mode_t io_mode; /** FD to redirect */ - int fd; + const int fd; virtual void print() const = 0; virtual ~io_data_t() = 0; @@ -167,8 +167,10 @@ class io_buffer_t : public io_pipe_t \param is_input set this parameter to zero if the buffer should be used to buffer the output of a command, or non-zero to buffer the input to a command. + + \param fd when -1, determined from is_input. */ - static io_buffer_t *create(bool is_input); + static io_buffer_t *create(bool is_input, int fd = -1); }; class io_chain_t : public std::vector > diff --git a/reader.cpp b/reader.cpp index 92d0aa521..8689fe643 100644 --- a/reader.cpp +++ b/reader.cpp @@ -1113,8 +1113,8 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vector in(io_buffer_t::create(true)); - shared_ptr out(io_buffer_t::create(false)); + shared_ptr in(io_buffer_t::create(true, 3)); + shared_ptr out(io_buffer_t::create(false, 4)); // The above may fail e.g. if we have too many open fds if (in.get() == NULL || out.get() == NULL) @@ -1137,8 +1137,6 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vectorfd = 3; - escaped_separator = escape(COMPLETE_SEP_STR, 1); for (size_t i=0; i< comp.size(); i++) @@ -1203,8 +1201,6 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vectorout_buffer_append(foo, strlen(foo)); free(foo); - out->fd = 4; - term_donate(); parser_t &parser = parser_t::principal_parser(); io_chain_t io_chain;