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
This commit is contained in:
mark@chromium.org 2013-12-03 14:12:08 +00:00
parent b303174e32
commit 036dec3ecc
7 changed files with 26 additions and 19 deletions

View file

@ -349,7 +349,7 @@ CrashGenerationServer::ClientEvent(short revents)
// A nasty process could try and send us too many descriptors and // A nasty process could try and send us too many descriptors and
// force a leak. // force a leak.
for (unsigned i = 0; i < num_fds; ++i) for (unsigned i = 0; i < num_fds; ++i)
HANDLE_EINTR(close(reinterpret_cast<int*>(CMSG_DATA(hdr))[i])); close(reinterpret_cast<int*>(CMSG_DATA(hdr))[i]);
return true; return true;
} else { } else {
signal_fd = reinterpret_cast<int*>(CMSG_DATA(hdr))[0]; signal_fd = reinterpret_cast<int*>(CMSG_DATA(hdr))[0];
@ -363,7 +363,7 @@ CrashGenerationServer::ClientEvent(short revents)
if (crashing_pid == -1 || signal_fd == -1) { if (crashing_pid == -1 || signal_fd == -1) {
if (signal_fd) if (signal_fd)
HANDLE_EINTR(close(signal_fd)); close(signal_fd);
return true; return true;
} }
@ -375,12 +375,12 @@ CrashGenerationServer::ClientEvent(short revents)
ino_t inode_number; ino_t inode_number;
if (!GetInodeForFileDescriptor(&inode_number, signal_fd)) { if (!GetInodeForFileDescriptor(&inode_number, signal_fd)) {
HANDLE_EINTR(close(signal_fd)); close(signal_fd);
return true; return true;
} }
if (!FindProcessHoldingSocket(&crashing_pid, inode_number - 1)) { if (!FindProcessHoldingSocket(&crashing_pid, inode_number - 1)) {
HANDLE_EINTR(close(signal_fd)); close(signal_fd);
return true; return true;
} }
@ -391,7 +391,7 @@ CrashGenerationServer::ClientEvent(short revents)
if (!google_breakpad::WriteMinidump(minidump_filename.c_str(), if (!google_breakpad::WriteMinidump(minidump_filename.c_str(),
crashing_pid, crash_context, crashing_pid, crash_context,
kCrashContextSize)) { kCrashContextSize)) {
HANDLE_EINTR(close(signal_fd)); close(signal_fd);
return true; return true;
} }
@ -410,7 +410,7 @@ CrashGenerationServer::ClientEvent(short revents)
msg.msg_iovlen = 1; msg.msg_iovlen = 1;
HANDLE_EINTR(sendmsg(signal_fd, &msg, MSG_DONTWAIT | MSG_NOSIGNAL)); HANDLE_EINTR(sendmsg(signal_fd, &msg, MSG_DONTWAIT | MSG_NOSIGNAL));
HANDLE_EINTR(close(signal_fd)); close(signal_fd);
return true; return true;
} }

View file

@ -36,7 +36,6 @@
#include "breakpad_googletest_includes.h" #include "breakpad_googletest_includes.h"
#include "client/linux/minidump_writer/cpu_set.h" #include "client/linux/minidump_writer/cpu_set.h"
#include "common/linux/eintr_wrapper.h"
#include "common/tests/auto_testfile.h" #include "common/tests/auto_testfile.h"
using namespace google_breakpad; using namespace google_breakpad;

View file

@ -33,7 +33,6 @@
#include "client/linux/minidump_writer/line_reader.h" #include "client/linux/minidump_writer/line_reader.h"
#include "breakpad_googletest_includes.h" #include "breakpad_googletest_includes.h"
#include "common/linux/eintr_wrapper.h"
#include "common/tests/auto_testfile.h" #include "common/tests/auto_testfile.h"
using namespace google_breakpad; using namespace google_breakpad;

View file

@ -36,7 +36,6 @@
#include "client/linux/minidump_writer/proc_cpuinfo_reader.h" #include "client/linux/minidump_writer/proc_cpuinfo_reader.h"
#include "breakpad_googletest_includes.h" #include "breakpad_googletest_includes.h"
#include "common/linux/eintr_wrapper.h"
#include "common/tests/auto_testfile.h" #include "common/tests/auto_testfile.h"
using namespace google_breakpad; using namespace google_breakpad;

View file

@ -37,11 +37,22 @@
// //
#define HANDLE_EINTR(x) ({ \ #define HANDLE_EINTR(x) ({ \
typeof(x) __eintr_result__; \ typeof(x) eintr_wrapper_result; \
do { \ do { \
__eintr_result__ = x; \ eintr_wrapper_result = (x); \
} while (__eintr_result__ == -1 && errno == EINTR); \ } while (eintr_wrapper_result == -1 && errno == EINTR); \
__eintr_result__;\ 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_ #endif // COMMON_LINUX_EINTR_WRAPPER_H_

View file

@ -37,7 +37,6 @@
#include <string> #include <string>
#include "breakpad_googletest_includes.h" #include "breakpad_googletest_includes.h"
#include "common/linux/eintr_wrapper.h"
#include "common/linux/memory_mapped_file.h" #include "common/linux/memory_mapped_file.h"
#include "common/tests/auto_tempdir.h" #include "common/tests/auto_tempdir.h"
#include "common/tests/file_utils.h" #include "common/tests/file_utils.h"

View file

@ -51,7 +51,7 @@ bool CopyFile(const char* from_path, const char* to_path) {
int outfile = HANDLE_EINTR(creat(to_path, 0666)); int outfile = HANDLE_EINTR(creat(to_path, 0666));
if (outfile < 0) { if (outfile < 0) {
perror("creat"); perror("creat");
if (HANDLE_EINTR(close(infile)) < 0) { if (IGNORE_EINTR(close(infile)) < 0) {
perror("close"); perror("close");
} }
return false; return false;
@ -84,11 +84,11 @@ bool CopyFile(const char* from_path, const char* to_path) {
} while (bytes_written_per_read < bytes_read); } while (bytes_written_per_read < bytes_read);
} }
if (HANDLE_EINTR(close(infile)) == -1) { if (IGNORE_EINTR(close(infile)) == -1) {
perror("close"); perror("close");
result = false; result = false;
} }
if (HANDLE_EINTR(close(outfile)) == -1) { if (IGNORE_EINTR(close(outfile)) == -1) {
perror("close"); perror("close");
result = false; result = false;
} }
@ -112,7 +112,7 @@ bool ReadFile(const char* path, void* buffer, ssize_t* buffer_size) {
ok = false; ok = false;
} }
} }
if (HANDLE_EINTR(close(fd)) == -1) { if (IGNORE_EINTR(close(fd)) == -1) {
perror("close"); perror("close");
ok = false; ok = false;
} }
@ -143,7 +143,7 @@ bool WriteFile(const char* path, const void* buffer, size_t buffer_size) {
bytes_written_total += bytes_written_partial; bytes_written_total += bytes_written_partial;
} }
} }
if (HANDLE_EINTR(close(fd)) == -1) { if (IGNORE_EINTR(close(fd)) == -1) {
perror("close"); perror("close");
ok = false; ok = false;
} }