Make the windows_version.h functions threadsafe by using a singleton.

base/win/windows_version.h

Index: base/win/windows_version.h
===================================================================
--- base/win/windows_version.h	(revision 80667)
+++ base/win/windows_version.h	(working copy)
@@ -7,62 +7,89 @@
 #pragma once
 
 #include "base/base_api.h"
+#include "base/memory/singleton.h"
 
 typedef void* HANDLE;
 
 namespace base {
 namespace win {
 
+// The running version of Windows.  This is declared outside OSInfo for
+// syntactic sugar reasons; see the declaration of GetVersion() below.
 // NOTE: Keep these in order so callers can do things like
-// "if (GetWinVersion() > WINVERSION_2000) ...".  It's OK to change the values,
-// though.
+// "if (base::win::GetVersion() >= base::win::VERSION_VISTA) ...".
 enum Version {
-  VERSION_PRE_2000 = 0,     // Not supported
-  VERSION_2000 = 1,         // Not supported
-  VERSION_XP = 2,
-  VERSION_SERVER_2003 = 3,  // Also includes Windows XP Professional x64 edition
-  VERSION_VISTA = 4,
-  VERSION_2008 = 5,
-  VERSION_WIN7 = 6,
+  VERSION_PRE_XP = 0,   // Not supported.
+  VERSION_XP,
+  VERSION_SERVER_2003,  // Also includes Windows XP Professional x64.
+  VERSION_VISTA,
+  VERSION_SERVER_2008,
+  VERSION_WIN7,
 };
 
-// Returns the running version of Windows.
-BASE_API Version GetVersion();
+// A Singleton that can be used to query various pieces of information about the
+// OS and process state.
+class BASE_API OSInfo {
+ public:
+  // The processor architecture this copy of Windows natively uses, which may
+  // not be the same as the processor's "native architecture".  For example,

[rvargas (doing something else), 2011/04/06 23:28:05]

nit: could you remove the part about the "processor's "native architecture""?.
It took me a while to decode why the example agrees with the comment, and that
would have been obvious if said piece was not there.

[Peter Kasting, 2011/04/07 00:29:19]

Done.

+  // given an x64-capable processor, we have three possibilities:
+  //   32-bit Chrome running on 32-bit Windows:           X86_ARCHITECTURE
+  //   32-bit Chrome running on 64-bit Windows via WOW64: X64_ARCHITECTURE
+  //   64-bit Chrome running on 64-bit Windows:           X64_ARCHITECTURE
+  enum WindowsArchitecture {
+    X86_ARCHITECTURE,
+    X64_ARCHITECTURE,
+    IA64_ARCHITECTURE,
+    OTHER_ARCHITECTURE,
+  };
 
-// Returns the major and minor version of the service pack installed.
-BASE_API void GetServicePackLevel(int* major, int* minor);
+  // Whether a process is running under WOW64 (the wrapper that allows 32-bit
+  // processes to run on 64-bit versions of Windows).  This will return
+  // WOW64_DISABLED for both "32-bit Chrome on 32-bit Windows" and "64-bit
+  // Chrome on 64-bit Windows".  WOW64_UNKNOWN means "an error occurred", e.g.
+  // the process does not have sufficient access rights to determine this.
+  enum WOW64Status {
+    WOW64_DISABLED,
+    WOW64_ENABLED,
+    WOW64_UNKNOWN,
+  };
 
-enum WindowsArchitecture {
-  X86_ARCHITECTURE,
-  X64_ARCHITECTURE,
-  IA64_ARCHITECTURE,
-  OTHER_ARCHITECTURE,
-};
+  static OSInfo* GetInstance();
 
-// Returns the processor architecture this copy of Windows natively uses.
-// For example, given an x64-capable processor, we have three possibilities:
-//   32-bit Chrome running on 32-bit Windows:           X86_ARCHITECTURE
-//   32-bit Chrome running on 64-bit Windows via WOW64: X64_ARCHITECTURE
-//   64-bit Chrome running on 64-bit Windows:           X64_ARCHITECTURE
-BASE_API WindowsArchitecture GetWindowsArchitecture();
+  Version version() const { return version_; }
+  // The next two functions return arrays of values, [major, minor(, build)].

[rvargas (doing something else), 2011/04/06 23:28:05]

Have you considered defining a couple of structs here and returning a pointer to
that instead? Having the callers just indexing an array feels wrong.

[Peter Kasting, 2011/04/07 00:29:19]

Good idea!  Done.

+  const int* version_number() const { return version_number_; }
+  const int* service_pack() const { return service_pack_; }
+  WindowsArchitecture architecture() const { return architecture_; }
+  int processors() const { return processors_; }
+  size_t allocation_granularity() const { return allocation_granularity_; }
+  WOW64Status wow64_status() const { return wow64_status_; }
 
-enum WOW64Status {
-  WOW64_DISABLED,
-  WOW64_ENABLED,
-  WOW64_UNKNOWN,
+  // Like wow64_status(), but for the supplied handle instead of the current
+  // process.  This doesn't touch member state, so you can bypass the singleton.
+  static WOW64Status GetWOW64StatusForProcess(HANDLE process_handle);
+
+ private:
+  OSInfo();
+  ~OSInfo();
+
+  Version version_;
+  int version_number_[3];
+  int service_pack_[2];
+  WindowsArchitecture architecture_;
+  int processors_;
+  size_t allocation_granularity_;
+  WOW64Status wow64_status_;
+
+  friend struct DefaultSingletonTraits<OSInfo>;
+  DISALLOW_COPY_AND_ASSIGN(OSInfo);
 };
 
-// Returns whether this process is running under WOW64 (the wrapper that allows
-// 32-bit processes to run on 64-bit versions of Windows).  This will return
-// WOW64_DISABLED for both "32-bit Chrome on 32-bit Windows" and "64-bit Chrome
-// on 64-bit Windows".  WOW64_UNKNOWN means "an error occurred", e.g. the
-// process does not have sufficient access rights to determine this.
-BASE_API WOW64Status GetWOW64Status();
+// Because this is by far the most commonly-requested value from the above
+// singleton, we add a global-scope accessor here as syntactic sugar.
+BASE_API Version GetVersion();
 
-// Like GetWOW64Status(), but for the supplied handle instead of the current
-// process.
-BASE_API WOW64Status GetWOW64StatusForProcess(HANDLE process_handle);
-
 }  // namespace win
 }  // namespace base