From 036dec3eccf7fd2a6d4edf8bb1f0f4242c360766 Mon Sep 17 00:00:00 2001 From: "mark@chromium.org" Date: Tue, 3 Dec 2013 14:12:08 +0000 Subject: [PATCH] Don't HANDLE_EINTR(close). Either IGNORE_EINTR(close) or just close. It is incorrect to wrap close in HANDLE_EINTR on Linux. Unnecessary #includes of eintr_wrapper.h are also removed. The variable naming within the macro is also updated per Chromium r178174. einter_wrapper.h contains a non-mechanical change. Mechanical changes were generated by running: sed -E -i '' \ -e 's/((=|if|return|CHECK|EXPECT|ASSERT).*)HANDLE(_EINTR\(.*close)/\1IGNORE\3/' \ -e 's/(ignore_result|void ?)\(HANDLE_EINTR\((.*close\(.*)\)\)/\2/' \ -e 's/(\(void\) ?)?HANDLE_EINTR\((.*close\(.*)\)/\2/' \ $(grep -rl HANDLE_EINTR.*close . --exclude-dir=.svn) sed -E -i '' -e '/#include.*eintr_wrapper\.h"/d' \ $(grep -EL '(HANDLE|IGNORE)_EINTR' \ $(grep -Elr '#include.*eintr_wrapper\.h"' . --exclude-dir=.svn)) BUG=chromium:269623 R=ted.mielczarek@gmail.com Review URL: https://breakpad.appspot.com/784002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1239 4c0a9323-5329-0410-9bdc-e9ce6186880e --- .../crash_generation_server.cc | 12 ++++++------ .../linux/minidump_writer/cpu_set_unittest.cc | 1 - .../minidump_writer/line_reader_unittest.cc | 1 - .../proc_cpuinfo_reader_unittest.cc | 1 - src/common/linux/eintr_wrapper.h | 19 +++++++++++++++---- .../linux/memory_mapped_file_unittest.cc | 1 - src/common/tests/file_utils.cc | 10 +++++----- 7 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/client/linux/crash_generation/crash_generation_server.cc b/src/client/linux/crash_generation/crash_generation_server.cc index f1e16b1d..8ec95038 100644 --- a/src/client/linux/crash_generation/crash_generation_server.cc +++ b/src/client/linux/crash_generation/crash_generation_server.cc @@ -349,7 +349,7 @@ CrashGenerationServer::ClientEvent(short revents) // A nasty process could try and send us too many descriptors and // force a leak. for (unsigned i = 0; i < num_fds; ++i) - HANDLE_EINTR(close(reinterpret_cast(CMSG_DATA(hdr))[i])); + close(reinterpret_cast(CMSG_DATA(hdr))[i]); return true; } else { signal_fd = reinterpret_cast(CMSG_DATA(hdr))[0]; @@ -363,7 +363,7 @@ CrashGenerationServer::ClientEvent(short revents) if (crashing_pid == -1 || signal_fd == -1) { if (signal_fd) - HANDLE_EINTR(close(signal_fd)); + close(signal_fd); return true; } @@ -375,12 +375,12 @@ CrashGenerationServer::ClientEvent(short revents) ino_t inode_number; if (!GetInodeForFileDescriptor(&inode_number, signal_fd)) { - HANDLE_EINTR(close(signal_fd)); + close(signal_fd); return true; } if (!FindProcessHoldingSocket(&crashing_pid, inode_number - 1)) { - HANDLE_EINTR(close(signal_fd)); + close(signal_fd); return true; } @@ -391,7 +391,7 @@ CrashGenerationServer::ClientEvent(short revents) if (!google_breakpad::WriteMinidump(minidump_filename.c_str(), crashing_pid, crash_context, kCrashContextSize)) { - HANDLE_EINTR(close(signal_fd)); + close(signal_fd); return true; } @@ -410,7 +410,7 @@ CrashGenerationServer::ClientEvent(short revents) msg.msg_iovlen = 1; HANDLE_EINTR(sendmsg(signal_fd, &msg, MSG_DONTWAIT | MSG_NOSIGNAL)); - HANDLE_EINTR(close(signal_fd)); + close(signal_fd); return true; } diff --git a/src/client/linux/minidump_writer/cpu_set_unittest.cc b/src/client/linux/minidump_writer/cpu_set_unittest.cc index 6c0fa43c..38fbefd3 100644 --- a/src/client/linux/minidump_writer/cpu_set_unittest.cc +++ b/src/client/linux/minidump_writer/cpu_set_unittest.cc @@ -36,7 +36,6 @@ #include "breakpad_googletest_includes.h" #include "client/linux/minidump_writer/cpu_set.h" -#include "common/linux/eintr_wrapper.h" #include "common/tests/auto_testfile.h" using namespace google_breakpad; diff --git a/src/client/linux/minidump_writer/line_reader_unittest.cc b/src/client/linux/minidump_writer/line_reader_unittest.cc index 928626e1..e3d4a83d 100644 --- a/src/client/linux/minidump_writer/line_reader_unittest.cc +++ b/src/client/linux/minidump_writer/line_reader_unittest.cc @@ -33,7 +33,6 @@ #include "client/linux/minidump_writer/line_reader.h" #include "breakpad_googletest_includes.h" -#include "common/linux/eintr_wrapper.h" #include "common/tests/auto_testfile.h" using namespace google_breakpad; diff --git a/src/client/linux/minidump_writer/proc_cpuinfo_reader_unittest.cc b/src/client/linux/minidump_writer/proc_cpuinfo_reader_unittest.cc index 1f7d0a73..d127b06f 100644 --- a/src/client/linux/minidump_writer/proc_cpuinfo_reader_unittest.cc +++ b/src/client/linux/minidump_writer/proc_cpuinfo_reader_unittest.cc @@ -36,7 +36,6 @@ #include "client/linux/minidump_writer/proc_cpuinfo_reader.h" #include "breakpad_googletest_includes.h" -#include "common/linux/eintr_wrapper.h" #include "common/tests/auto_testfile.h" using namespace google_breakpad; diff --git a/src/common/linux/eintr_wrapper.h b/src/common/linux/eintr_wrapper.h index 20b6bed1..35e5e245 100644 --- a/src/common/linux/eintr_wrapper.h +++ b/src/common/linux/eintr_wrapper.h @@ -37,11 +37,22 @@ // #define HANDLE_EINTR(x) ({ \ - typeof(x) __eintr_result__; \ + typeof(x) eintr_wrapper_result; \ do { \ - __eintr_result__ = x; \ - } while (__eintr_result__ == -1 && errno == EINTR); \ - __eintr_result__;\ + eintr_wrapper_result = (x); \ + } while (eintr_wrapper_result == -1 && errno == EINTR); \ + eintr_wrapper_result; \ +}) + +#define IGNORE_EINTR(x) ({ \ + typeof(x) eintr_wrapper_result; \ + do { \ + eintr_wrapper_result = (x); \ + if (eintr_wrapper_result == -1 && errno == EINTR) { \ + eintr_wrapper_result = 0; \ + } \ + } while (0); \ + eintr_wrapper_result; \ }) #endif // COMMON_LINUX_EINTR_WRAPPER_H_ diff --git a/src/common/linux/memory_mapped_file_unittest.cc b/src/common/linux/memory_mapped_file_unittest.cc index 7c34e708..4fa50cf9 100644 --- a/src/common/linux/memory_mapped_file_unittest.cc +++ b/src/common/linux/memory_mapped_file_unittest.cc @@ -37,7 +37,6 @@ #include #include "breakpad_googletest_includes.h" -#include "common/linux/eintr_wrapper.h" #include "common/linux/memory_mapped_file.h" #include "common/tests/auto_tempdir.h" #include "common/tests/file_utils.h" diff --git a/src/common/tests/file_utils.cc b/src/common/tests/file_utils.cc index 80a6b650..1c041777 100644 --- a/src/common/tests/file_utils.cc +++ b/src/common/tests/file_utils.cc @@ -51,7 +51,7 @@ bool CopyFile(const char* from_path, const char* to_path) { int outfile = HANDLE_EINTR(creat(to_path, 0666)); if (outfile < 0) { perror("creat"); - if (HANDLE_EINTR(close(infile)) < 0) { + if (IGNORE_EINTR(close(infile)) < 0) { perror("close"); } return false; @@ -84,11 +84,11 @@ bool CopyFile(const char* from_path, const char* to_path) { } while (bytes_written_per_read < bytes_read); } - if (HANDLE_EINTR(close(infile)) == -1) { + if (IGNORE_EINTR(close(infile)) == -1) { perror("close"); result = false; } - if (HANDLE_EINTR(close(outfile)) == -1) { + if (IGNORE_EINTR(close(outfile)) == -1) { perror("close"); result = false; } @@ -112,7 +112,7 @@ bool ReadFile(const char* path, void* buffer, ssize_t* buffer_size) { ok = false; } } - if (HANDLE_EINTR(close(fd)) == -1) { + if (IGNORE_EINTR(close(fd)) == -1) { perror("close"); ok = false; } @@ -143,7 +143,7 @@ bool WriteFile(const char* path, const void* buffer, size_t buffer_size) { bytes_written_total += bytes_written_partial; } } - if (HANDLE_EINTR(close(fd)) == -1) { + if (IGNORE_EINTR(close(fd)) == -1) { perror("close"); ok = false; }