Do not use mach_vm.h on iOS.

Do not use mach_vm.h on iOS.

 mach_vm.h has been removed from iOS5. Use #define to use vm_ replacement of
mach_vm_ functions on iOS.

 Do not use mach_vm_allocate -> use a stack variable instead.

[Mark Mentovai]

http://breakpad.appspot.com/321001/diff/2001/3003
File src/client/mac/handler/mach_vm.h (right):

http://breakpad.appspot.com/321001/diff/2001/3003#newcode1
Line 1: // Copyright (c) 2011, Google Inc.
Give this file another name, like mach_vm_compat.h. Some users of Breakpad
(including, perhaps, the checked-in .xcodeproj) might be using header maps, and
header maps are bad when you need to #include different files that have the same
leaf name, like (project-local) mach_vm.h and /usr/include/mach/mach_vm.h.

This is one reason I hate header maps and always turn this misfeature off.

http://breakpad.appspot.com/321001/diff/2001/3003#newcode29
Line 29: 
Add #include guards.

http://breakpad.appspot.com/321001/diff/2001/3003#newcode32
Line 32: #if TARGET_OS_IPHONE
You need a short comment here explaining what this is all about.

http://breakpad.appspot.com/321001/diff/2001/3004
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/321001/diff/2001/3004#newcode1382
Line 1382: int returnValue = sysctl(mib, mibsize, &proc, &size, NULL, 0);
Use Google C++-style naming in this .cc file: return_value.

http://breakpad.appspot.com/321001/diff/2001/3004#newcode1383
Line 1383: if (!returnValue || (returnValue == ENOMEM && size == sizeof(proc)))
{
Test explicitly |return_value == 0|, not |!return_value|.

Get rid of the ENOMEM check. If sysctl fails and sets ENOMEM, it didn’t write
anything to the single kinfo_proc you gave it. Don’t be concerned about
kinfo_proc changing size without you knowing. It can’t do that because it’s used
in arrays of kinfo_proc for other KERN_PROC sysctl variants, so it needs to
remain a fixed size for binary compatibility.

[qsr]

http://breakpad.appspot.com/321001/diff/2001/3003
File src/client/mac/handler/mach_vm.h (right):

http://breakpad.appspot.com/321001/diff/2001/3003#newcode1
Line 1: // Copyright (c) 2011, Google Inc.
On 2011/10/24 14:25:45, Mark Mentovai wrote:
> Give this file another name, like mach_vm_compat.h. Some users of Breakpad
> (including, perhaps, the checked-in .xcodeproj) might be using header maps,
and
> header maps are bad when you need to #include different files that have the
same
> leaf name, like (project-local) mach_vm.h and /usr/include/mach/mach_vm.h.
> 
> This is one reason I hate header maps and always turn this misfeature off.

Done.

http://breakpad.appspot.com/321001/diff/2001/3003#newcode29
Line 29: 
On 2011/10/24 14:25:45, Mark Mentovai wrote:
> Add #include guards.

Done.

http://breakpad.appspot.com/321001/diff/2001/3003#newcode32
Line 32: #if TARGET_OS_IPHONE
On 2011/10/24 14:25:45, Mark Mentovai wrote:
> You need a short comment here explaining what this is all about.

Done.

http://breakpad.appspot.com/321001/diff/2001/3004
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/321001/diff/2001/3004#newcode1383
Line 1383: if (!returnValue || (returnValue == ENOMEM && size == sizeof(proc)))
{
On 2011/10/24 14:25:45, Mark Mentovai wrote:
> Test explicitly |return_value == 0|, not |!return_value|.
> 
> Get rid of the ENOMEM check. If sysctl fails and sets ENOMEM, it didn’t write
> anything to the single kinfo_proc you gave it. Don’t be concerned about
> kinfo_proc changing size without you knowing. It can’t do that because it’s
used
> in arrays of kinfo_proc for other KERN_PROC sysctl variants, so it needs to
> remain a fixed size for binary compatibility.

Done.

[Mark Mentovai]

LGTM with these two changes.

http://breakpad.appspot.com/321001/diff/6001/2004
File src/client/mac/handler/mach_vm_compat.h (right):

http://breakpad.appspot.com/321001/diff/6001/2004#newcode30
Line 30: #ifndef CLIENT_MAC_GENERATOR_MACH_VM_COMPAT_H__
The C++ style guide has specified one trailing underscore for #include guards
for a few years. Two trailing underscores are reserved for the system and
compiler. Lots of Breakpad code predates that change, but new files should use
the new one-underscore format.

http://breakpad.appspot.com/321001/diff/6001/2005
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/321001/diff/6001/2005#newcode1382
Line 1382: if (!sysctl(mib, mibsize, &proc, &size, NULL, 0)) {
As I mentioned earlier, it’s clearer to use an explicit |sysctl() == 0| rather
than |!sysctl()|. I usually think of successful system calls like this as
“returning zero” but ! makes it look like something’s failing, even though the
compiled machine code will be identical in both cases.

[qsr]

http://breakpad.appspot.com/321001/diff/6001/2004
File src/client/mac/handler/mach_vm_compat.h (right):

http://breakpad.appspot.com/321001/diff/6001/2004#newcode30
Line 30: #ifndef CLIENT_MAC_GENERATOR_MACH_VM_COMPAT_H__
On 2011/10/24 14:55:03, Mark Mentovai wrote:
> The C++ style guide has specified one trailing underscore for #include guards
> for a few years. Two trailing underscores are reserved for the system and
> compiler. Lots of Breakpad code predates that change, but new files should use
> the new one-underscore format.

Done.

http://breakpad.appspot.com/321001/diff/6001/2005
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/321001/diff/6001/2005#newcode1382
Line 1382: if (!sysctl(mib, mibsize, &proc, &size, NULL, 0)) {
On 2011/10/24 14:55:03, Mark Mentovai wrote:
> As I mentioned earlier, it’s clearer to use an explicit |sysctl() == 0| rather
> than |!sysctl()|. I usually think of successful system calls like this as
> “returning zero” but ! makes it look like something’s failing, even though the
> compiled machine code will be identical in both cases.

Done.