From d51a1e4fe2c0180f33bed2285068e8d8eeeeaf66 Mon Sep 17 00:00:00 2001 From: Aaron Gyes Date: Sat, 30 Jul 2016 12:01:37 -0700 Subject: [PATCH] kill CAST_INIT, use reinterpret_cast<> on sockaddr Just use static_cast directly instead of inscrutible "shortcut" macro. It was not always used and doesn't seem to do much besides scramble things up; encountering CAST_INIT() in the code seems likely to lead to head scratching due to the transformation taking place. It was added to save folks typing the type twice, now with 100 columns available, let's roll that convenience macro back. sockaddr_dl: Perform reinterpret_cast conversion. The cast affected alignment and looks fishy to a compiler (but it's fine). Ditch C-style cast and communicate we're doing that on purpose. --- src/common.h | 4 ---- src/env_universal_common.cpp | 2 +- src/exec.cpp | 18 ++++++++++-------- src/postfork.cpp | 12 ++++++------ src/proc.cpp | 2 +- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/common.h b/src/common.h index 380e101cf..b41e1600a 100644 --- a/src/common.h +++ b/src/common.h @@ -27,10 +27,6 @@ #define OS_IS_CYGWIN #endif -/// Avoid writing the type name twice in a common "static_cast-initialization". Caveat: This doesn't -/// work with type names containing commas! -#define CAST_INIT(type, dst, src) type dst = static_cast(src) - // Common string type. typedef std::wstring wcstring; typedef std::vector wcstring_list_t; diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 14d8860be..7fbecad61 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -935,7 +935,7 @@ static bool get_mac_address(unsigned char macaddr[MAC_ADDRESS_MAX_LEN], if (p->ifa_addr->sa_family == AF_LINK) { if (p->ifa_name && p->ifa_name[0] && !strcmp((const char *)p->ifa_name, interface)) { - const sockaddr_dl &sdl = *(sockaddr_dl *)p->ifa_addr; + const sockaddr_dl &sdl = *reinterpret_cast(p->ifa_addr); size_t alen = sdl.sdl_alen; if (alen > MAC_ADDRESS_MAX_LEN) alen = MAC_ADDRESS_MAX_LEN; diff --git a/src/exec.cpp b/src/exec.cpp index d10efecd4..f367396fb 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -105,7 +105,7 @@ static bool redirection_is_to_real_file(const io_data_t *io) { bool result = false; if (io != NULL && io->io_mode == IO_FILE) { // It's a file redirection. Compare the path to /dev/null. - CAST_INIT(const io_file_t *, io_file, io); + const io_file_t *io_file = static_cast(io); const char *path = io_file->filename_cstr; if (strcmp(path, "/dev/null") != 0) { // It's not /dev/null. @@ -268,7 +268,7 @@ static bool io_transmogrify(const io_chain_t &in_chain, io_chain_t *out_chain, case IO_FILE: { // Transmogrify file redirections. int fd; - CAST_INIT(io_file_t *, in_file, in.get()); + io_file_t *in_file = static_cast(in.get()); if ((fd = open(in_file->filename_cstr, in_file->flags, OPEN_MASK)) == -1) { debug(1, FILE_ERROR, in_file->filename_cstr); @@ -404,7 +404,7 @@ void exec_job(parser_t &parser, job_t *j) { const shared_ptr &io = all_ios.at(idx); if ((io->io_mode == IO_BUFFER)) { - CAST_INIT(io_buffer_t *, io_buffer, io.get()); + io_buffer_t *io_buffer = static_cast(io.get()); assert(!io_buffer->is_input); } } @@ -450,7 +450,7 @@ void exec_job(parser_t &parser, job_t *j) { for (size_t i = 0; i < all_ios.size(); i++) { io_data_t *io = all_ios.at(i).get(); if (io->io_mode == IO_BUFFER) { - CAST_INIT(io_buffer_t *, io_buffer, io); + io_buffer_t *io_buffer = static_cast(io); if (!io_buffer->avoid_conflicts_with_io_chain(all_ios)) { // We could not avoid conflicts, probably due to fd exhaustion. Mark an error. exec_error = true; @@ -715,7 +715,7 @@ void exec_job(parser_t &parser, job_t *j) { if (in) { switch (in->io_mode) { case IO_FD: { - CAST_INIT(const io_fd_t *, in_fd, in.get()); + const io_fd_t *in_fd = static_cast(in.get()); // Ignore user-supplied fd redirections from an fd other than the // standard ones. e.g. in source <&3 don't actually read from fd 3, // which is internal to fish. We still respect this redirection in @@ -730,13 +730,13 @@ void exec_job(parser_t &parser, job_t *j) { break; } case IO_PIPE: { - CAST_INIT(const io_pipe_t *, in_pipe, in.get()); + const io_pipe_t *in_pipe = static_cast(in.get()); local_builtin_stdin = in_pipe->pipe_fd[0]; break; } case IO_FILE: { // Do not set CLO_EXEC because child needs access. - CAST_INIT(const io_file_t *, in_file, in.get()); + const io_file_t *in_file = static_cast(in.get()); local_builtin_stdin = open(in_file->filename_cstr, in_file->flags, OPEN_MASK); if (local_builtin_stdin == -1) { @@ -932,8 +932,10 @@ void exec_job(parser_t &parser, job_t *j) { // performance quite a bit in complex completion code. debug(3, L"Skipping fork: buffered output for internal builtin '%ls'", p->argv0()); - CAST_INIT(io_buffer_t *, io_buffer, stdout_io.get()); + + io_buffer_t *io_buffer = static_cast(stdout_io.get()); const std::string res = wcs2string(builtin_io_streams->out.buffer()); + io_buffer->out_buffer_append(res.data(), res.size()); fork_was_skipped = true; } else if (stdout_io.get() == NULL && stderr_io.get() == NULL) { diff --git a/src/postfork.cpp b/src/postfork.cpp index f88934b7e..1c246a975 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -137,7 +137,7 @@ static int handle_child_io(const io_chain_t &io_chain) { case IO_FILE: { // Here we definitely do not want to set CLO_EXEC because our child needs access. - CAST_INIT(const io_file_t *, io_file, io); + const io_file_t *io_file = static_cast(io); int tmp = open(io_file->filename_cstr, io_file->flags, OPEN_MASK); if (tmp < 0) { if ((io_file->flags & O_EXCL) && (errno == EEXIST)) { @@ -181,7 +181,7 @@ static int handle_child_io(const io_chain_t &io_chain) { case IO_BUFFER: case IO_PIPE: { - CAST_INIT(const io_pipe_t *, io_pipe, io); + const io_pipe_t *io_pipe = static_cast(io); // If write_pipe_idx is 0, it means we're connecting to the read end (first pipe // fd). If it's 1, we're connecting to the write end (second pipe fd). unsigned int write_pipe_idx = (io_pipe->is_input ? 0 : 1); @@ -340,7 +340,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, const shared_ptr io = io_chain.at(idx); if (io->io_mode == IO_FD) { - CAST_INIT(const io_fd_t *, io_fd, io.get()); + const io_fd_t *io_fd = static_cast(io.get()); if (io->fd == io_fd->old_fd) continue; } @@ -351,7 +351,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, } case IO_FILE: { - CAST_INIT(const io_file_t *, io_file, io.get()); + const io_file_t *io_file = static_cast(io.get()); if (!err) err = posix_spawn_file_actions_addopen(actions, io->fd, io_file->filename_cstr, io_file->flags /* mode */, OPEN_MASK); @@ -359,7 +359,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, } case IO_FD: { - CAST_INIT(const io_fd_t *, io_fd, io.get()); + const io_fd_t *io_fd = static_cast(io.get()); if (!err) err = posix_spawn_file_actions_adddup2(actions, io_fd->old_fd /* from */, io->fd /* to */); @@ -368,7 +368,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr, case IO_BUFFER: case IO_PIPE: { - CAST_INIT(const io_pipe_t *, io_pipe, io.get()); + const io_pipe_t *io_pipe = static_cast(io.get()); unsigned int write_pipe_idx = (io_pipe->is_input ? 0 : 1); int from_fd = io_pipe->pipe_fd[write_pipe_idx]; int to_fd = io->fd; diff --git a/src/proc.cpp b/src/proc.cpp index 8b2c7407b..d69493510 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -711,7 +711,7 @@ static int select_try(job_t *j) { for (size_t idx = 0; idx < chain.size(); idx++) { const io_data_t *io = chain.at(idx).get(); if (io->io_mode == IO_BUFFER) { - CAST_INIT(const io_pipe_t *, io_pipe, io); + const io_pipe_t *io_pipe = static_cast(io); int fd = io_pipe->pipe_fd[0]; // fwprintf( stderr, L"fd %d on job %ls\n", fd, j->command ); FD_SET(fd, &fds);