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

Issue 415002: Move minidump_extension_linux.h contents into minidump_format.h (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 4 months ago by Ted Mielczarek
Modified:
12 years, 4 months ago
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

I think having these in a separate file is just confusing, especially since it
defines stream types, so it makes avoiding overlap in the numbering harder.

Patch Set 1 #

Total comments: 1

Patch Set 2 : Changed the value of MD_CUSTOM_DATA_STREAM #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
R src/client/linux/minidump_writer/minidump_extension_linux.h View 1 chunk +0 lines, -75 lines 0 comments Download
M src/client/linux/minidump_writer/minidump_writer.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/google_breakpad/common/minidump_format.h View 1 2 chunks +29 lines, -2 lines 0 comments Download
M src/processor/minidump_dump.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/tools/linux/md2core/minidump-2-core.cc View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 7
Ted Mielczarek
12 years, 4 months ago #1
Lei Zhang (chromium)
http://breakpad.appspot.com/415002/diff/1/src/google_breakpad/common/minidump_format.h File src/google_breakpad/common/minidump_format.h (right): http://breakpad.appspot.com/415002/diff/1/src/google_breakpad/common/minidump_format.h#newcode338 src/google_breakpad/common/minidump_format.h:338: MD_LINUX_CPU_INFO = 0x47670003, /* /proc/cpuinfo */ Drive by: MD_LINUX_CPU_INFO ...
12 years, 4 months ago #2
Ted Mielczarek
On Thu, Jul 12, 2012 at 9:20 PM, <thestig@chromium.org> wrote: > > http://breakpad.appspot.com/415002/diff/1/src/google_breakpad/common/minidump_format.h > File ...
12 years, 4 months ago #3
Ted Mielczarek
12 years, 4 months ago #4
Ted Mielczarek
So yeah, we should really take this patch to avoid the duplicate stream numbering. Mark: ...
12 years, 4 months ago #5
Mark Mentovai
LGTM as long as nobody’s started depending on a value that has changed. They probably ...
12 years, 4 months ago #6
Ted Mielczarek
12 years, 4 months ago #7
On Fri, Jul 20, 2012 at 8:34 AM,  <mark@chromium.org> wrote:
> LGTM as long as nobody’s started depending on a value that has changed.
> They probably haven’t been depending on anything since this stuff is
> brand-new.

Yeah, cdn said it wasn't a problem:
http://breakpad.appspot.com/408002/#msg18
Sign in to reply to this message.

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