From 4b6bd7cae5e731f8e2bafeabca59e5aaabd6749d Mon Sep 17 00:00:00 2001 From: Cheer Xiao Date: Tue, 15 Jan 2013 16:18:03 +0800 Subject: [PATCH] Split out io_file_t --- exec.cpp | 17 ++++++----- io.cpp | 8 ++++-- io.h | 51 +++++++++++++++++++-------------- parser.cpp | 81 ++++++++++++++++++++++++---------------------------- postfork.cpp | 14 +++++---- 5 files changed, 90 insertions(+), 81 deletions(-) diff --git a/exec.cpp b/exec.cpp index 2aa13dc1b..725b40bec 100644 --- a/exec.cpp +++ b/exec.cpp @@ -407,11 +407,12 @@ static bool io_transmogrify(const io_chain_t &in_chain, io_chain_t &out_chain, s case IO_FILE: { int fd; - if ((fd=open(in->filename_cstr, in->param2.flags, OPEN_MASK))==-1) + CAST_INIT(io_file_t *, in_file, in.get()); + if ((fd=open(in_file->filename_cstr, in_file->flags, OPEN_MASK))==-1) { debug(1, FILE_ERROR, - in->filename_cstr); + in_file->filename_cstr); wperror(L"open"); success = false; @@ -516,7 +517,8 @@ static bool can_use_posix_spawn_for_job(const job_t *job, const process_t *proce const shared_ptr &io = job->io.at(idx); if (io->io_mode == IO_FILE) { - const char *path = io->filename_cstr; + CAST_INIT(const io_file_t *, io_file, io.get()); + const char *path = io_file->filename_cstr; /* This IO action is a file redirection. Only allow /dev/null, which is a common case we assume won't fail. */ if (strcmp(path, "/dev/null") != 0) { @@ -860,13 +862,14 @@ void exec(parser_t &parser, job_t *j) case IO_FILE: { /* Do not set CLO_EXEC because child needs access */ - builtin_stdin=open(in->filename_cstr, - in->param2.flags, OPEN_MASK); + CAST_INIT(const io_file_t *, in_file, in.get()); + builtin_stdin=open(in_file->filename_cstr, + in_file->flags, OPEN_MASK); if (builtin_stdin == -1) { debug(1, FILE_ERROR, - in->filename_cstr); + in_file->filename_cstr); wperror(L"open"); } else @@ -1158,7 +1161,7 @@ void exec(parser_t &parser, job_t *j) for (io_chain_t::iterator iter = j->io.begin(); iter != j->io.end(); iter++) { const shared_ptr &tmp_io = *iter; - if (tmp_io->io_mode == IO_FILE && strcmp(tmp_io->filename_cstr, "/dev/null") != 0) + if (tmp_io->io_mode == IO_FILE && strcmp(static_cast(tmp_io.get())->filename_cstr, "/dev/null") != 0) { skip_fork = false; break; diff --git a/io.cpp b/io.cpp index 8bbc07438..9eb29f2be 100644 --- a/io.cpp +++ b/io.cpp @@ -55,9 +55,6 @@ void io_data_t::print() const { switch (io_mode) { - case IO_FILE: - fprintf(stderr, "file (%s)\n", filename_cstr); - break; case IO_PIPE: fprintf(stderr, "pipe {%d, %d}\n", param1.pipe_fd[0], param1.pipe_fd[1]); break; @@ -77,6 +74,11 @@ void io_fd_t::print() const fprintf(stderr, "FD map %d -> %d\n", old_fd, fd); } +void io_file_t::print() const +{ + fprintf(stderr, "file (%s)\n", filename_cstr); +} + void io_buffer_read(io_data_t *d) { exec_close(d->param1.pipe_fd[1]); diff --git a/io.h b/io.h index 627bfc39c..e13f961c5 100644 --- a/io.h +++ b/io.h @@ -39,24 +39,6 @@ public: int pipe_fd[2]; } param1; - - /** Second type-specific paramter for redirection */ - union - { - /** file creation flags to send to open for IO_FILE */ - int flags; - } param2; - - /** Filename IO_FILE. malloc'd. This needs to be used after fork, so don't use wcstring here. */ - const char *filename_cstr; - - /** Convenience to set filename_cstr via wcstring */ - void set_filename(const wcstring &str) - { - free((void *)filename_cstr); - filename_cstr = wcs2str(str.c_str()); - } - /** Function to create the output buffer */ void out_buffer_create() { @@ -100,15 +82,12 @@ public: io_mode(m), fd(f), param1(), - param2(), - filename_cstr(NULL), is_input(0) { } virtual ~io_data_t() { - free((void *)filename_cstr); } }; @@ -141,6 +120,36 @@ public: } }; +class io_file_t : public io_data_t +{ +public: + /** Filename, malloc'd. This needs to be used after fork, so don't use wcstring here. */ + const char *filename_cstr; + /** file creation flags to send to open */ + int flags; + + /** Convenience to set filename_cstr via wcstring */ + void set_filename(const wcstring &str) + { + free((void *)filename_cstr); + filename_cstr = wcs2str(str.c_str()); + } + + virtual void print() const; + + io_file_t(int f, const char *fname = NULL, int fl = 0) : + io_data_t(IO_FILE, f), + filename_cstr(fname ? strdup(fname) : NULL), + flags(fl) + { + } + + virtual ~io_file_t() + { + free((void *)filename_cstr); + } +}; + class io_chain_t : public std::vector > { public: diff --git a/parser.cpp b/parser.cpp index 1329cea62..f2ed3f2d7 100644 --- a/parser.cpp +++ b/parser.cpp @@ -1568,67 +1568,60 @@ void parser_t::parse_job_argument_list(process_t *p, _(L"Invalid IO redirection")); tok_next(tok); } + else if (type == TOK_REDIRECT_FD) + { + if (target == L"-") + { + new_io.reset(new io_close_t(fd)); + } + else + { + wchar_t *end; + + errno = 0; + + int old_fd = fish_wcstoi(target.c_str(), &end, 10); + + if (old_fd < 0 || errno || *end) + { + error(SYNTAX_ERROR, + tok_get_pos(tok), + _(L"Requested redirection to something that is not a file descriptor %ls"), + target.c_str()); + + tok_next(tok); + } + else + { + new_io.reset(new io_fd_t(fd, old_fd)); + } + } + } else { - + int flags = 0; switch (type) { case TOK_REDIRECT_APPEND: - new_io.reset(new io_data_t(IO_FILE, fd)); - new_io->param2.flags = O_CREAT | O_APPEND | O_WRONLY; - new_io->set_filename(target); + flags = O_CREAT | O_APPEND | O_WRONLY; break; case TOK_REDIRECT_OUT: - new_io.reset(new io_data_t(IO_FILE, fd)); - new_io->param2.flags = O_CREAT | O_WRONLY | O_TRUNC; - new_io->set_filename(target); + flags = O_CREAT | O_WRONLY | O_TRUNC; break; case TOK_REDIRECT_NOCLOB: - new_io.reset(new io_data_t(IO_FILE, fd)); - new_io->param2.flags = O_CREAT | O_EXCL | O_WRONLY; - new_io->set_filename(target); + flags = O_CREAT | O_EXCL | O_WRONLY; break; case TOK_REDIRECT_IN: - new_io.reset(new io_data_t(IO_FILE, fd)); - new_io->param2.flags = O_RDONLY; - new_io->set_filename(target); + flags = O_RDONLY; break; - case TOK_REDIRECT_FD: - { - if (target == L"-") - { - new_io.reset(new io_close_t(fd)); - } - else - { - wchar_t *end; - - errno = 0; - - int old_fd = fish_wcstoi(target.c_str(), &end, 10); - - if (old_fd < 0 || errno || *end) - { - error(SYNTAX_ERROR, - tok_get_pos(tok), - _(L"Requested redirection to something that is not a file descriptor %ls"), - target.c_str()); - - tok_next(tok); - } - else - { - new_io.reset(new io_fd_t(fd, old_fd)); - } - } - break; - } } - + io_file_t *new_io_file = new io_file_t(fd, NULL, flags); + new_io_file->set_filename(target); + new_io.reset(new_io_file); } } diff --git a/postfork.cpp b/postfork.cpp index a5602ad19..17a46350a 100644 --- a/postfork.cpp +++ b/postfork.cpp @@ -189,17 +189,18 @@ static int handle_child_io(io_chain_t &io_chain) case IO_FILE: { // Here we definitely do not want to set CLO_EXEC because our child needs access - if ((tmp=open(io->filename_cstr, - io->param2.flags, OPEN_MASK))==-1) + CAST_INIT(io_file_t *, io_file, io); + if ((tmp=open(io_file->filename_cstr, + io_file->flags, OPEN_MASK))==-1) { - if ((io->param2.flags & O_EXCL) && + if ((io_file->flags & O_EXCL) && (errno ==EEXIST)) { - debug_safe(1, NOCLOB_ERROR, io->filename_cstr); + debug_safe(1, NOCLOB_ERROR, io_file->filename_cstr); } else { - debug_safe(1, FILE_ERROR, io->filename_cstr); + debug_safe(1, FILE_ERROR, io_file->filename_cstr); safe_perror("open"); } @@ -467,8 +468,9 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, posix_spawn_fil case IO_FILE: { + CAST_INIT(const io_file_t *, io_file, io.get()); if (! err) - err = posix_spawn_file_actions_addopen(actions, io->fd, io->filename_cstr, io->param2.flags /* mode */, OPEN_MASK); + err = posix_spawn_file_actions_addopen(actions, io->fd, io_file->filename_cstr, io_file->flags /* mode */, OPEN_MASK); break; }