Rename CommandLine::GetCommandLineString().

chrome/installer/util/product.cc

Index: chrome/installer/util/product.cc
diff --git a/chrome/installer/util/product.cc b/chrome/installer/util/product.cc
index eb5286d2de38c57dde7fd29ed9a369c0296fdcfc..3c24af64e38d9741628c79d617a97aa15d0b8b54 100644
--- a/chrome/installer/util/product.cc
+++ b/chrome/installer/util/product.cc
@@ -79,15 +79,17 @@ bool Product::LaunchChromeAndWait(const FilePath& application_path,
   bool success = false;
   STARTUPINFOW si = { sizeof(si) };
   PROCESS_INFORMATION pi = {0};
-  // Cast away constness of the command_line_string() since CreateProcess
+  // Create a writable copy of the command line string, since CreateProcess
   // might modify the string (insert \0 to separate the program from the
   // arguments).  Since we're not using the cmd variable beyond this point

[grt (UTC plus 2), 2011/07/19 03:19:34]

The "Since we're not using..." comment isn't relevant anymore, is it?  If this
is the case, please remove it.

[msw, 2011/07/19 07:14:26]

Done.

   // we don't care.
+  scoped_ptr_malloc<wchar_t> writable_command_line_string(

[grt (UTC plus 2), 2011/07/19 03:19:34]

Why not:
std::wstring writable_command_line_string(cmd.GetCommandLineString());
followed by &writable_command_line_string[0] in the call to CreateProcess?  I
find this more readable and it should be more efficient since it makes use of
the string's knowledge of its length (no scan for the terminator).

[msw, 2011/07/19 07:14:26]

Done.

[Evan Martin, 2011/07/19 14:35:05]

Oh yeah, should have remembered this earlier:
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/base/string_util.h&ex...

+      ::_wcsdup(cmd.GetCommandLineString().c_str()));
   if (!::CreateProcess(cmd.GetProgram().value().c_str(),
-                       const_cast<wchar_t*>(cmd.command_line_string().c_str()),
+                       writable_command_line_string.get(),
                        NULL, NULL, FALSE, CREATE_NO_WINDOW, NULL, NULL,
                        &si, &pi)) {
-    PLOG(ERROR) << "Failed to launch: " << cmd.command_line_string();
+    PLOG(ERROR) << "Failed to launch: " << writable_command_line_string.get();

[grt (UTC plus 2), 2011/07/19 03:19:34]

How about either cmd.GetCommandLineString() (to log the whole command line) or
cmd.GetProgram().value() (to log only the program) here to avoid the ambiguity
of what CreateProcess may or may not do to the string?

[msw, 2011/07/19 07:14:26]

Done.

   } else {
     ::CloseHandle(pi.hThread);