WinDDK ATL and MSVC express compatability

chrome/installer/mini_installer/mini_installer.cc

Index: chrome/installer/mini_installer/mini_installer.cc
===================================================================
--- chrome/installer/mini_installer/mini_installer.cc	(revision 78547)
+++ chrome/installer/mini_installer/mini_installer.cc	(working copy)
@@ -6,10 +6,10 @@
 // installed or upgraded. It is designed to be extremely small (~5KB with no
 // extra resources linked) and it has two main jobs:
 //   1) unpack the resources (possibly decompressing some)
-//   2) run the real installer (setup.exe) with appropiate flags.
+//   2) run the real installer (setup.exe) with appropriate flags.
 //
-// In order to be really small we don't link against the CRT and we define the
-// following compiler/linker flags:
+// In order to be really small the app doesn't link against the CRT and
+// defines the following compiler/linker flags:
 //   EnableIntrinsicFunctions="true" compiler: /Oi
 //   BasicRuntimeChecks="0"
 //   BufferSecurityCheck="false" compiler: /GS-
@@ -17,15 +17,8 @@
 //   IgnoreAllDefaultLibraries="true" linker: /NODEFAULTLIB
 //   OptimizeForWindows98="1"  liker: /OPT:NOWIN98
 //   linker: /SAFESEH:NO
-// Also some built-in code that the compiler relies on is not defined so we
-// are forced to manually link against it. It comes in the form of two
-// object files that exist in $(VCInstallDir)\crt\src which are memset.obj and
-// P4_memset.obj. These two object files rely on the existence of a static
-// variable named __sse2_available which indicates the presence of intel sse2
-// extensions. We define it to false which causes a slower but safe code for
-// memcpy and memset intrinsics.
 
-// having the linker merge the sections is saving us ~500 bytes.
+// have the linker merge the sections, saving us ~500 bytes.
 #pragma comment(linker, "/MERGE:.rdata=.text")
 
 #include <windows.h>
@@ -43,9 +36,6 @@
 char (&ArraySizeHelper(T (&array)[N]))[N];
 #define arraysize(array) (sizeof(ArraySizeHelper(array)))
 
-// Required linker symbol. See remarks above.
-extern "C" unsigned int __sse2_available = 0;
-
 namespace mini_installer {
 
 typedef StackString<MAX_PATH> PathString;
@@ -822,3 +812,36 @@
   int result = mini_installer::WMain(::GetModuleHandle(NULL));
   ::ExitProcess(result);
 }
+
+// VC Express editions don't come with the memset CRT obj file
+// and linking to the obj files between versions becomes a bit
+// problematic. Therefore, simply implement memset.
+//
+// This also avoids having to use the __sse2_available hack when
+// linking with linking to the dual x64 and x86 obj files.

[Mark Mentovai, 2011/03/17 20:43:42]

Revise “linking with linking to”—pick one. :)

[RN, 2011/03/18 09:34:24]

Yeah.... rewrote that comment and explained it a bit more.

+// extern "C" __sse2_available =

[Mark Mentovai, 2011/03/17 20:43:42]

This comment looks like code but it’s not. defined(_M_IX86) isn’t even really
correct either. I’d either write

// extern "C" int __sse2_available = 0;

or just leave the codeish bit out altogether.

[RN, 2011/03/18 09:34:24]

Done.

+// defined(_M_IX86) || defined(_M_AMD64);
+extern "C" {
+
+#pragma function(memset)
+void* memset(void* dest, int c, size_t count) {
+  // Typical 32-bit memset c implementation

[Mark Mentovai, 2011/03/17 20:43:42]

This assumes that dest is properly aligned. It’ll work but the performance
advantage may be negated if dest isn’t. I suspect that’s fine given the use
case.

[RN, 2011/03/18 09:34:24]

Put in more comments about this (even if isn't that critical).

+  size_t adjcount = count>>2;

[Mark Mentovai, 2011/03/17 20:43:42]

Style nit: spaces around operators like >>.

[RN, 2011/03/18 09:34:24]

Done.

+  UINT32 fill = (c<<24|c<<16|c<<8|c);

[Mark Mentovai, 2011/03/17 20:43:42]

Here too. (c << 24 | c << 16 | c << 8 | c)

[RN, 2011/03/18 09:34:24]

Done.

+  UINT32* dest32 = (unsigned __int32*)dest;
+  UINT8*  dest8 = (unsigned __int8*)dest;

[Mark Mentovai, 2011/03/17 20:43:42]

No need to line up dest8 and dest32, especially when you haven’t done anything
to line up the rest of the line. Rein this one in.

[RN, 2011/03/18 09:34:24]

Nice catch, done.

+
+  // Take care of the ending 0-3 bytes (binary 11 = 3)
+  switch (count - (adjcount<<2)) {

[Mark Mentovai, 2011/03/17 20:43:42]

Space around that << operator.

[RN, 2011/03/18 09:34:24]

Done.

+    case 3:  dest8[count - 3] = c;

[Mark Mentovai, 2011/03/17 20:43:42]

The assignments here really belong on the next line and suitably indented. We
rarely do this stuff on the same line, and reading it this way makes me think
harder about the “break” omission. (But I see that you do want fall-through.)

[RN, 2011/03/18 09:34:24]

OK. Also added a longish comment about how it works just in case.

+    case 2:  dest8[count - 2] = c;
+    case 1:  dest8[count - 1] = c;
+  }
+
+  while (adjcount-- > 0)
+      *(dest32++) = fill;

[Mark Mentovai, 2011/03/17 20:43:42]

Fix the indent.

[RN, 2011/03/18 09:34:24]

Assuming the 2 spaces rule, done.

[Mark Mentovai, 2011/03/18 14:54:47]

RN wrote:
Yes, 2 spaces. For reference, we write to the style guide at

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

+
+  return dest;
+}
+
+}

[Mark Mentovai, 2011/03/17 20:43:42]

}  // extern "C"

[RN, 2011/03/18 09:34:24]

Done.