Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(31)

Issue 784002: Don't HANDLE_EINTR(close). Either IGNORE_EINTR(close) or just close (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by Mark Mentovai
Modified:
10 years, 11 months ago
Reviewers:
Ted Mielczarek
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

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

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1239

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/client/linux/crash_generation/crash_generation_server.cc View 5 chunks +6 lines, -6 lines 0 comments Download
M src/client/linux/minidump_writer/cpu_set_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/client/linux/minidump_writer/line_reader_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/client/linux/minidump_writer/proc_cpuinfo_reader_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/common/linux/eintr_wrapper.h View 1 chunk +15 lines, -4 lines 0 comments Download
M src/common/linux/memory_mapped_file_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/common/tests/file_utils.cc View 4 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 4
Mark Mentovai
10 years, 11 months ago #1
Mark Mentovai
This is the same principle as Chromium https://codereview.chromium.org/100253002/ .
10 years, 11 months ago #2
Ted Mielczarek
LGTM
10 years, 11 months ago #3
Mark Mentovai
10 years, 11 months ago #4
Message was sent while issue was closed.
Committed patchset #1 manually as r1239 (presubmit successful).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted