Fork breakpad's dump_syms and related code. Generate XOR of first page of mem...

Fork breakpad's dump_syms and related code. Generate XOR of first page of memory instead of MD5 of text section.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=17031

[agl, 2009-05-27 17:38:48]

LGTM: nits only.

http://codereview.chromium.org/113893/diff/16/1006
File breakpad/linux/dump_symbols.cc (right):

http://codereview.chromium.org/113893/diff/16/1006#newcode1
Line 1: // Copyright (c) 2006, 2009, Google Inc.
Assuming that this file is untouched.

http://codereview.chromium.org/113893/diff/16/1007
File breakpad/linux/dump_symbols.h (right):

http://codereview.chromium.org/113893/diff/16/1007#newcode36
Line 36: #include <string>
(Note, you might want to ignore the two comments below to avoid forking. Just
including them for completeness.)

http://codereview.chromium.org/113893/diff/16/1007#newcode42
Line 42: bool WriteSymbolFile(const std::string &obj_file,
static?

http://codereview.chromium.org/113893/diff/16/1007#newcode42
Line 42: bool WriteSymbolFile(const std::string &obj_file,
& on the left

http://codereview.chromium.org/113893/diff/16/1004
File breakpad/linux/file_id.cc (right):

http://codereview.chromium.org/113893/diff/16/1004#newcode63
Line 63: void *base = mmap(NULL, mapped_len,
* on the left (I hate it, but Evan always gets me :)

http://codereview.chromium.org/113893/diff/16/1004#newcode64
Line 64: PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
You should close(fd) right after mmap. If it was successful, you don't need it
any more and, if it failed, you want to close it anyway.

http://codereview.chromium.org/113893/diff/16/1004#newcode70
Line 70: for (unsigned i = 0; i < kMDGUIDSize; i++)
Maybe replace with memset?

http://codereview.chromium.org/113893/diff/16/1004#newcode71
Line 71: identifier[i] = '\0';
... but if you don't: '\0' -> 0 - it's a uint8_t, not a char

[Lei Zhang, 2009-05-27 18:19:29]

http://codereview.chromium.org/113893/diff/16/1006
File breakpad/linux/dump_symbols.cc (right):

http://codereview.chromium.org/113893/diff/16/1006#newcode1
Line 1: // Copyright (c) 2006, 2009, Google Inc.
On 2009/05/27 17:38:48, agl wrote:
> Assuming that this file is untouched.

Just different includes: breakpad/linux/foo.h instead of common/linux/foo.h.

http://codereview.chromium.org/113893/diff/16/1007
File breakpad/linux/dump_symbols.h (right):

http://codereview.chromium.org/113893/diff/16/1007#newcode36
Line 36: #include <string>
On 2009/05/27 17:38:48, agl wrote:
> (Note, you might want to ignore the two comments below to avoid forking. Just
> including them for completeness.)

It feels weird forking the .cc file but not the .h file.

http://codereview.chromium.org/113893/diff/16/1007#newcode42
Line 42: bool WriteSymbolFile(const std::string &obj_file,
On 2009/05/27 17:38:48, agl wrote:
> & on the left

Done and done.

http://codereview.chromium.org/113893/diff/16/1004
File breakpad/linux/file_id.cc (right):

http://codereview.chromium.org/113893/diff/16/1004#newcode63
Line 63: void *base = mmap(NULL, mapped_len,
On 2009/05/27 17:38:48, agl wrote:
> * on the left (I hate it, but Evan always gets me :)

Fixed all instances.

http://codereview.chromium.org/113893/diff/16/1004#newcode64
Line 64: PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
On 2009/05/27 17:38:48, agl wrote:
> You should close(fd) right after mmap. If it was successful, you don't need it
> any more and, if it failed, you want to close it anyway.

Done.

http://codereview.chromium.org/113893/diff/16/1004#newcode71
Line 71: identifier[i] = '\0';
On 2009/05/27 17:38:48, agl wrote:
> ... but if you don't: '\0' -> 0 - it's a uint8_t, not a char

Replaced with a memset. (It started out as a char.)