Implement SysInfo for iOS

base/sys_info_ios.mm

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/sys_info.h"
+
+#import <UIKit/UIKit.h>
+#include <mach/mach_host.h>

[Mark Mentovai, 2012/07/09 18:22:47]

Instead of #including these two mach headers, just #include <mach/mach.h>.

[Chen Yu, 2012/07/10 15:51:10]

Done.

+#include <mach/mach_init.h>
+
+#include "base/logging.h"
+#include "base/mac/scoped_nsautorelease_pool.h"
+#include "base/string_split.h"

[Mark Mentovai, 2012/07/09 18:22:47]

You don’t seem to use this.

[Chen Yu, 2012/07/10 15:51:10]

Done.

+#include "base/sys_string_conversions.h"
+
+namespace base {
+
+// static
+std::string SysInfo::OperatingSystemName() {
+  base::mac::ScopedNSAutoreleasePool pool;
+  return SysNSStringToUTF8([[UIDevice currentDevice] systemName]);

[Mark Mentovai, 2012/07/09 18:22:47]

I’m curious: what does this return on iPhone? iPad? iPod Touch?

[Chen Yu, 2012/07/10 15:51:10]

I saw "iPhone OS" when running it on an iPhone device (I don't have other types
of device to check it out).

+}
+
+// static
+std::string SysInfo::OperatingSystemVersion() {
+  base::mac::ScopedNSAutoreleasePool pool;
+  return SysNSStringToUTF8([[UIDevice currentDevice] systemVersion]);
+}
+
+// static
+void SysInfo::OperatingSystemVersionNumbers(int32 *major_version,

[Mark Mentovai, 2012/07/09 18:22:47]

Chrome style puts the * on the type name. I don’t know why you changed this from
the _mac.cc file. You seem to put the * on the type name correctly later in this
function. See
http://www.chromium.org/developers/coding-style#TOC-Code-formatting .

[Chen Yu, 2012/07/10 15:51:10]

Done.

[stuartmorgan, 2012/07/11 09:46:04]

FWIW, the answer is that we didn't; it was wrong everywhere at the time this
file was forked in our repo, and then Peter fixed it later in
http://codereview.chromium.org/6816027 (but obviously not in our copy).

+                                            int32 *minor_version,
+                                            int32 *bugfix_version) {

[Mark Mentovai, 2012/07/09 18:22:47]

Don’t you want a ScopedNSAutoreleasePool here too? You wanted one last time you
wrote [[UIDevice currentDevice] systemVersion], and you have the same call in
this function.

[Chen Yu, 2012/07/10 15:51:10]

Done.

+  static int32 sSystemVersionMajor = 0;

[Mark Mentovai, 2012/07/09 18:22:47]

1. There is no need to initialize this to 0. That’s the default behavior for a
static local variable.
2. I don’t necessarily see much benefit in making this a static local.
3. If you do want to cache the value, this isn’t a great way to do it. Implement
an “uncached” function like OperatingSystemVersionNumbersInternal, and then in
OperatingSystemVersionNumbers, call the internal function in an initialization
to a static variable. The compiler-generated guard variable will be unambiguous,
as compared to what you’ve written here, which will continue to re-fetch the
version number even if a value was previously cached and the first component was
0.
4. We don’t use Hungarian notation, even for static local variables. These
should just be named_normally_like_this.

[Chen Yu, 2012/07/10 15:51:10]

Here it is changed to not to cache the information.

On 2012/07/09 18:22:47, Mark Mentovai wrote:

+  static int32 sSystemVersionMinor;
+  static int32 sSystemVersionBugFix;
+
+  // iOS doesn't have Gestalt, so parse the version string.

[Mark Mentovai, 2012/07/09 18:22:47]

This comment only makes sense in comparison to the _mac.cc file, but that’s not
necessarily a comparison anyone will be making. If this is the correct way to
fetch the operating system version on iOS, this comment isn’t necessary at all.

[Chen Yu, 2012/07/10 15:51:10]

Removed.
On 2012/07/09 18:22:47, Mark Mentovai wrote:

+  if (!sSystemVersionMajor) {
+    NSString* version = [[UIDevice currentDevice] systemVersion];
+    NSArray* versionInfo = [version componentsSeparatedByString:@"."];
+    NSUInteger length = [versionInfo count];
+    sSystemVersionMajor = [[versionInfo objectAtIndex:0] intValue];
+    sSystemVersionMinor = [[versionInfo objectAtIndex:1] intValue];

[Mark Mentovai, 2012/07/09 18:22:47]

This is only safe if length >= 2. The docs don’t make any particular guarantee
about the format of -[UIDevice systemVersion].

[Chen Yu, 2012/07/10 15:51:10]

Done.

+    if (length == 3) {

[Mark Mentovai, 2012/07/09 18:22:47]

This can be >= instead of ==.

[Chen Yu, 2012/07/10 15:51:10]

Done.

+      sSystemVersionBugFix = [[versionInfo objectAtIndex:2] intValue];
+    }
+  }
+
+  *major_version = sSystemVersionMajor;
+  *minor_version = sSystemVersionMinor;
+  *bugfix_version = sSystemVersionBugFix;
+}
+
+// static
+int64 SysInfo::AmountOfPhysicalMemory() {
+  struct host_basic_info hostinfo;
+  mach_msg_type_number_t count = HOST_BASIC_INFO_COUNT;
+  int result = host_info(mach_host_self(),
+                         HOST_BASIC_INFO,
+                         reinterpret_cast<host_info_t>(&hostinfo),
+                         &count);
+  DCHECK_EQ(HOST_BASIC_INFO_COUNT, count);

[Mark Mentovai, 2012/07/09 18:22:47]

This should actually follow the “result != KERN_SUCCESS” check. You can make the
same change in the _mac.cc file.

[Chen Yu, 2012/07/10 15:51:10]

Done.

+  if (result != KERN_SUCCESS) {
+    NOTREACHED();
+    return 0;
+  }
+
+  return static_cast<int64>(hostinfo.max_mem);
+}
+
+}  // namespace base