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

Issue 2814002: Fix microdump_writer and add unittest. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 6 months ago by primiano
Modified:
6 years, 5 months ago
Reviewers:
mmandlis
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk
Visibility:
Public.

Description

Fix microdump_writer and add unittest.

This adds some small fixes to the microdump writer and introduces
a unittest.

BUG=chromium:410294
R=mmandlis@chromium.org

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

Patch Set 1 #

Patch Set 2 : Remove compile checks (break some comilers) #

Patch Set 3 : #

Patch Set 4 : Enable test only on Linux #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M Makefile.am View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M Makefile.in View 1 2 3 9 chunks +36 lines, -9 lines 0 comments Download
M src/client/linux/microdump_writer/microdump_writer.cc View 1 2 9 chunks +20 lines, -10 lines 2 comments Download
A src/client/linux/microdump_writer/microdump_writer_unittest.cc View 1 2 3 1 chunk +130 lines, -0 lines 0 comments Download

Messages

Total messages: 4
primiano
PTAL. Very small fixes, but I need this to roll breakpad and enable it in ...
6 years, 5 months ago #1
mmandlis
lgtm https://breakpad.appspot.com/2814002/diff/110001/src/client/linux/microdump_writer/microdump_writer.cc File src/client/linux/microdump_writer/microdump_writer.cc (right): https://breakpad.appspot.com/2814002/diff/110001/src/client/linux/microdump_writer/microdump_writer.cc#newcode101 src/client/linux/microdump_writer/microdump_writer.cc:101: #if !defined(__ANDROID__) can this be #if defined (__linux__) ...
6 years, 5 months ago #2
primiano
Committed patchset #4 (id:110001) manually as 1404 (presubmit successful).
6 years, 5 months ago #3
primiano
6 years, 5 months ago #4
Message was sent while issue was closed.
https://breakpad.appspot.com/2814002/diff/110001/src/client/linux/microdump_w...
File src/client/linux/microdump_writer/microdump_writer.cc (right):

https://breakpad.appspot.com/2814002/diff/110001/src/client/linux/microdump_w...
src/client/linux/microdump_writer/microdump_writer.cc:101: #if
!defined(__ANDROID__)
On 2014/11/25 01:43:41, mmandlis wrote:
> can this be #if defined (__linux__) instead? just wondering..
Hmm I used the same logic of logger::write (in log.cc) which is defined as
follows:

#if defined(__ANDROID__)
  return __android_log_write(ANDROID_LOG_WARN, "google-breakpad", buf);
#else
  return sys_write(2, buf, nbytes);
#endif

And I think it's better to keep it consistent with that.
Sign in to reply to this message.

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