Add new Mach-O reader class

I can't edit Jim's issue, so I'm attaching his second patch here, and I'll
attach my updated copy of it so you can interdiff them.

[Ted Mielczarek]

Updated with mark's comments

[Mark Mentovai]

LGO

You’re shepherding this one now?

http://breakpad.appspot.com/115001/diff/2001/3001
File src/common/mac/macho_reader.cc (right):

http://breakpad.appspot.com/115001/diff/2001/3001#newcode90
Line 90: if (!(cursor >> object_files_count_)) { // nfat_arch
Style nit: at least two spaces between the last live character on a line and the
//.

http://breakpad.appspot.com/115001/diff/2001/3001#newcode132
Line 132: >> object_files_->cpusubtype)) {
Readability: line the >> up with the one on the previous line.

http://breakpad.appspot.com/115001/diff/2001/3001#newcode238
Line 238: case CPU_TYPE_I386:
Indent within the switch. It should look like:

  switch (c) {
    case K:
      s(1);
      break;
    default:
      s(2);
      break;
  }

http://breakpad.appspot.com/115001/diff/2001/3002
File src/common/mac/macho_reader.h (right):

http://breakpad.appspot.com/115001/diff/2001/3002#newcode148
Line 148: struct fat_arch *object_files_;
If object_files_ is a std::vector<struct fat_arch>, then &object_files_[0]
should still return a struct fat_arch* suitable for use with NXFindBestFatArch.

(Assuming, as always with std::vector, that you are careful to not use &v[0]
notation when v.size() == 0.)

If that works for you, it seems better than doing things old-school and doing
that malloc with the ugly cast. It would also let you get rid of the destructor,
and you wouldn’t need to track object_files_count_ separately.

http://breakpad.appspot.com/115001/diff/2001/3002#newcode440
Line 440: cpu_subtype_t cpu_subtype_; // mach_header[_64].cpusubtype
Two spaces between the ; and //. Also on 449. Also on the other comments here if
you intend to keep them lined up.

http://breakpad.appspot.com/115001/diff/2001/3003
File src/common/mac/macho_reader_unittest.cc (right):

http://breakpad.appspot.com/115001/diff/2001/3003#newcode1898
Line 1898: 
Nuke the blank line at the end of this file.

http://breakpad.appspot.com/115001/diff/2001/3004
File src/common/test_assembler.cc (right):

http://breakpad.appspot.com/115001/diff/2001/3004#newcode35
Line 35: #include <cassert>
<assert.h>, <stdio.h>, and then a blank line before <iterator>.

http://breakpad.appspot.com/115001/diff/2001/3004#newcode42
Line 42: namespace TestAssembler {
test_assembler

http://breakpad.appspot.com/115001/diff/2001/3006
File src/tools/mac/dump_syms/macho_dump.cc (right):

http://breakpad.appspot.com/115001/diff/2001/3006#newcode1
Line 1: 
Eliminate.

[Ted Mielczarek]

On 2010/06/16 16:57:41, Mark Mentovai wrote:
> LGO

Thanks.
 
> You’re shepherding this one now?

Yeah, Jim has been pulled off on other tasks (and is on vacation currently).
I'll have the other patch that's not fully reviewed yet up for you soon, and
then hopefully I can land most of his patch queue.

[Ted Mielczarek]

http://breakpad.appspot.com/115001/diff/2001/3001
File src/common/mac/macho_reader.cc (right):

http://breakpad.appspot.com/115001/diff/2001/3001#newcode90
Line 90: if (!(cursor >> object_files_count_)) { // nfat_arch
Just a note to myself: reading from a ByteCursor into a size_t is a bad idea and
will make you spend way too much time trying to figure out why this code breaks
on x86-64.