Cleans up mac projects, and builds everything 64 bit

[dmaclach]

Passes all the unittests, not sure what else you guys do to test a given
build...

[Mark Mentovai]

LGTM by and large.

Generally we’d prefer C++<style>(casts) to (cstyle)casts. I didn’t mark these
inline, though. They happened all over the place.

http://breakpad.appspot.com/130001/diff/2001/89
File src/client/mac/sender/crash_report_sender.h (right):

http://breakpad.appspot.com/130001/diff/2001/89#newcode38
Line 38: #import "GTMDefines.h"
Really?

http://breakpad.appspot.com/130001/diff/2001/88
File src/client/mac/testapp/Controller.m (right):

http://breakpad.appspot.com/130001/diff/2001/88#newcode34
Line 34: #import "GTMDefines.h"
Again…

When is NSInteger not present? Old SDKs?

http://breakpad.appspot.com/130001/diff/2001/74
File src/common/mac/dump_syms.h (right):

http://breakpad.appspot.com/130001/diff/2001/74#newcode87
Line 87: 
Excess vertical whitespace.

http://breakpad.appspot.com/130001/diff/2001/69
File src/common/mac/dump_syms.mm (right):

http://breakpad.appspot.com/130001/diff/2001/69#newcode58
Line 58: #define CPU_TYPE_ARM		((cpu_type_t) 12)
Is there a reason for all of that horizontal whitespace?

http://breakpad.appspot.com/130001/diff/2001/69#newcode59
Line 59: #endif // CPU_TYPE_ARM
Nit: at least two spaces between the last “thing” and the //.

http://breakpad.appspot.com/130001/diff/2001/68
File src/common/mac/macho_id.cc (right):

http://breakpad.appspot.com/130001/diff/2001/68#newcode56
Line 56: MachoID::MachoID(const char *path) :
This was correct as it was before, without any indentation on this line.

The correct form for a C++ initializer list split across multiple lines is:

MachoID::MachoID(const char *path)
    : file_(0),
      crc_(0),
      ...

(ref. C++ style guide.)

http://breakpad.appspot.com/130001/diff/2001/57
File src/tools/mac/crash_report/on_demand_symbol_supplier.mm (right):

http://breakpad.appspot.com/130001/diff/2001/57#newcode252
Line 252: if(dump.Read(module_str)) {
if<space>(dump.Read...

[dmaclach]

Fixed up my casts as well, although it's a mixed bag in there to say the least
;-)

http://breakpad.appspot.com/130001/diff/2001/89
File src/client/mac/sender/crash_report_sender.h (right):

http://breakpad.appspot.com/130001/diff/2001/89#newcode38
Line 38: #import "GTMDefines.h"
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> Really?
 
Yep... needed for NSUInteger on 10.4

http://breakpad.appspot.com/130001/diff/2001/88
File src/client/mac/testapp/Controller.m (right):

http://breakpad.appspot.com/130001/diff/2001/88#newcode34
Line 34: #import "GTMDefines.h"
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> Again…
> 
> When is NSInteger not present? Old SDKs?
tiger

http://breakpad.appspot.com/130001/diff/2001/74
File src/common/mac/dump_syms.h (right):

http://breakpad.appspot.com/130001/diff/2001/74#newcode87
Line 87: 
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> Excess vertical whitespace.

Done.

http://breakpad.appspot.com/130001/diff/2001/69
File src/common/mac/dump_syms.mm (right):

http://breakpad.appspot.com/130001/diff/2001/69#newcode58
Line 58: #define CPU_TYPE_ARM		((cpu_type_t) 12)
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> Is there a reason for all of that horizontal whitespace?

Done.

http://breakpad.appspot.com/130001/diff/2001/69#newcode59
Line 59: #endif // CPU_TYPE_ARM
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> Nit: at least two spaces between the last “thing” and the //.

Done.

http://breakpad.appspot.com/130001/diff/2001/68
File src/common/mac/macho_id.cc (right):

http://breakpad.appspot.com/130001/diff/2001/68#newcode56
Line 56: MachoID::MachoID(const char *path) :
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> This was correct as it was before, without any indentation on this line.

Stupid xcode

> The correct form for a C++ initializer list split across multiple lines is:
> 
> MachoID::MachoID(const char *path)
>     : file_(0),
>       crc_(0),
>       ...
> 
> (ref. C++ style guide.)

done

http://breakpad.appspot.com/130001/diff/2001/57
File src/tools/mac/crash_report/on_demand_symbol_supplier.mm (right):

http://breakpad.appspot.com/130001/diff/2001/57#newcode252
Line 252: if(dump.Read(module_str)) {
On 2010/07/16 21:36:55, Mark Mentovai wrote:
> if<space>(dump.Read...

nice catch

[Mark Mentovai]

Cast-wise, you’re right, a lot of this code is a mixed bag.

http://breakpad.appspot.com/130001/diff/11001/12048
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/130001/diff/11001/12048#newcode974
Line 974: info_ptr->processor_max_mhz = static_cast<u_int32_t>(speed /
(1000000));
These three were marginally easier before when they were 1000 * 1000. It’s
easier to count three zeroes by eye than six. “Wait, did he write six or seven?”
It’s hard to count them without holding a mouse cursor up to the number. (You
did write six.)

http://breakpad.appspot.com/130001/diff/11001/12016
File src/common/mac/dump_syms.mm (right):

http://breakpad.appspot.com/130001/diff/11001/12016#newcode58
Line 58: #define CPU_TYPE_ARM ((cpu_type_t) 12)
Missed a cast?

[Mark Mentovai]

LGTM

[jimb]

Hi --- I had a few comments.

http://breakpad.appspot.com/130001/diff/54002/50032
File src/common/dwarf/bytereader.cc (right):

http://breakpad.appspot.com/130001/diff/54002/50032#newcode126
Line 126: uint64_t skew = section_base_ & (AddressSize() - 1);
It seemed best to me to do this computation in size_t, since we are talking
about indices into real buffers in our memory, and uint64_t may be slower than
size_t.

http://breakpad.appspot.com/130001/diff/54002/50023
File src/common/mac/dump_syms.h (right):

http://breakpad.appspot.com/130001/diff/54002/50023#newcode95
Line 95: bool SetArchitecture(const std::string &arch_name);
I didn't see where this got used. Is it?

http://breakpad.appspot.com/130001/diff/54002/50034
File src/common/module.h (right):

http://breakpad.appspot.com/130001/diff/54002/50034#newcode277
Line 277: typedef set<Function *, FunctionCompare> FunctionSet;
Type definitions are supposed to appear earlier.  Can you move this to after
FileByNameMap?