From 6ce85aebc65459210fb5dab63c190f64cf0a3dc0 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 9 Jun 2019 17:43:25 -0700 Subject: [PATCH] Switch file_io_t to store a wcstring We no longer use file_io_t after fork(). We don't need to use a malloc'd string any more. Use a wcstring. --- src/exec.cpp | 14 +++++++------- src/io.cpp | 2 +- src/io.h | 10 +++++----- src/redirection.cpp | 12 ++++++------ src/wutil.cpp | 5 +++++ src/wutil.h | 3 +++ 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index dd98fac23..0cda24b00 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -52,7 +52,7 @@ #define WRITE_ERROR _(L"An error occurred while writing output") /// File redirection error message. -#define FILE_ERROR _(L"An error occurred while redirecting file '%s'") +#define FILE_ERROR _(L"An error occurred while redirecting file '%ls'") /// Base open mode to pass to calls to open. #define OPEN_MASK 0666 @@ -81,8 +81,8 @@ static bool redirection_is_to_real_file(const shared_ptr &io) { bool result = false; if (io && io->io_mode == io_mode_t::file) { // It's a file redirection. Compare the path to /dev/null. - const char *path = static_cast(io.get())->filename_cstr; - if (std::strcmp(path, "/dev/null") != 0) { + const wcstring &path = static_cast(io.get())->filename; + if (path != L"/dev/null") { // It's not /dev/null. result = true; } @@ -223,9 +223,9 @@ static bool resolve_file_redirections_to_fds(const io_chain_t &in_chain, io_chai case io_mode_t::file: { // We have a path-based redireciton. Resolve it to a file. io_file_t *in_file = static_cast(in.get()); - int fd = open(in_file->filename_cstr, in_file->flags, OPEN_MASK); + int fd = wopen(in_file->filename, in_file->flags, OPEN_MASK); if (fd < 0) { - debug(1, FILE_ERROR, in_file->filename_cstr); + debug(1, FILE_ERROR, in_file->filename.c_str()); wperror(L"open"); success = false; break; @@ -510,9 +510,9 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr(in.get()); locally_opened_stdin = - autoclose_fd_t{open(in_file->filename_cstr, in_file->flags, OPEN_MASK)}; + autoclose_fd_t{wopen(in_file->filename, in_file->flags, OPEN_MASK)}; if (!locally_opened_stdin.valid()) { - debug(1, FILE_ERROR, in_file->filename_cstr); + debug(1, FILE_ERROR, in_file->filename.c_str()); wperror(L"open"); } local_builtin_stdin = locally_opened_stdin.fd(); diff --git a/src/io.cpp b/src/io.cpp index d1a53ff97..edbf06e9b 100644 --- a/src/io.cpp +++ b/src/io.cpp @@ -22,7 +22,7 @@ void io_close_t::print() const { std::fwprintf(stderr, L"close %d\n", fd); } void io_fd_t::print() const { std::fwprintf(stderr, L"FD map %d -> %d\n", old_fd, fd); } -void io_file_t::print() const { std::fwprintf(stderr, L"file (%s)\n", filename_cstr); } +void io_file_t::print() const { std::fwprintf(stderr, L"file (%ls)\n", filename.c_str()); } void io_pipe_t::print() const { std::fwprintf(stderr, L"pipe {%d} (input: %s)\n", pipe_fd(), is_input_ ? "yes" : "no"); diff --git a/src/io.h b/src/io.h index 042e8a9bf..89fcdc810 100644 --- a/src/io.h +++ b/src/io.h @@ -191,17 +191,17 @@ class io_fd_t : public io_data_t { 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 *const filename_cstr; + /// The filename. + wcstring filename; /// file creation flags to send to open. const int flags; void print() const override; - io_file_t(int f, const wcstring &fname, int fl = 0) - : io_data_t(io_mode_t::file, f), filename_cstr(wcs2str(fname)), flags(fl) {} + io_file_t(int f, wcstring fname, int fl = 0) + : io_data_t(io_mode_t::file, f), filename(std::move(fname)), flags(fl) {} - ~io_file_t() override { free((void *)filename_cstr); } + ~io_file_t() override = default; }; /// Represents (one end) of a pipe. diff --git a/src/redirection.cpp b/src/redirection.cpp index c0c1b6985..5a30ea740 100644 --- a/src/redirection.cpp +++ b/src/redirection.cpp @@ -8,9 +8,9 @@ /// File descriptor redirection error message. #define FD_ERROR "An error occurred while redirecting file descriptor %s" -#define NOCLOB_ERROR _(L"The file '%s' already exists") +#define NOCLOB_ERROR _(L"The file '%ls' already exists") -#define FILE_ERROR _(L"An error occurred while redirecting file '%s'") +#define FILE_ERROR _(L"An error occurred while redirecting file '%ls'") /// Base open mode to pass to calls to open. #define OPEN_MASK 0666 @@ -26,12 +26,12 @@ maybe_t dup2_list_t::resolve_chain(const io_chain_t &io_chain) { // Here we definitely do not want to set CLO_EXEC because our child needs access. // Open the file. const io_file_t *io_file = static_cast(io_ref.get()); - int file_fd = open(io_file->filename_cstr, io_file->flags, OPEN_MASK); + int file_fd = wopen(io_file->filename, io_file->flags, OPEN_MASK); if (file_fd < 0) { if ((io_file->flags & O_EXCL) && (errno == EEXIST)) { - debug(1, NOCLOB_ERROR, io_file->filename_cstr); + debug(1, NOCLOB_ERROR, io_file->filename.c_str()); } else { - debug(1, FILE_ERROR, io_file->filename_cstr); + debug(1, FILE_ERROR, io_file->filename.c_str()); if (should_debug(1)) wperror(L"open"); } return none(); @@ -43,7 +43,7 @@ maybe_t dup2_list_t::resolve_chain(const io_chain_t &io_chain) { if (file_fd != io_file->fd) { file_fd = move_fd_to_unused(file_fd, io_chain, false /* cloexec */); if (file_fd < 0) { - debug(1, FILE_ERROR, io_file->filename_cstr); + debug(1, FILE_ERROR, io_file->filename.c_str()); if (should_debug(1)) wperror(L"dup"); return none(); } diff --git a/src/wutil.cpp b/src/wutil.cpp index 74b91013a..3ebcf87d1 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -210,6 +210,11 @@ int open_cloexec(const std::string &cstring, int flags, mode_t mode, bool cloexe return fd; } +int wopen(const wcstring &pathname, int flags, mode_t mode) { + cstring tmp = wcs2string(pathname); + return open(tmp.c_str(), flags, mode); +} + int wopen_cloexec(const wcstring &pathname, int flags, mode_t mode) { cstring tmp = wcs2string(pathname); return open_cloexec(tmp, flags, mode, true); diff --git a/src/wutil.h b/src/wutil.h index 955ea5e21..52c20c653 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -24,6 +24,9 @@ FILE *wfopen(const wcstring &path, const char *mode); /// Sets CLO_EXEC on a given fd. bool set_cloexec(int fd); +/// Wide character version of open(). +int wopen(const wcstring &pathname, int flags, mode_t mode = 0); + /// Wide character version of open() that also sets the close-on-exec flag (atomically when /// possible). int wopen_cloexec(const wcstring &pathname, int flags, mode_t mode = 0);