Enforce lowercase switches when calling CommandLine::HasSwitch(const char*).

base/command_line.cc

 // 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/command_line.h"
 
 #include <algorithm>
 #include <ostream>
 
 #include "base/basictypes.h"
@@ skipping 75 matching lines @@
                                        switch_value);
 #elif defined(OS_POSIX)
       command_line->AppendSwitchNative(switch_string, switch_value);
 #endif
     } else {
       command_line->AppendArgNative(arg);
     }
   }
 }
 
+bool ContainsUpperASCII(const char* string) {
+  for (; *string; ++string) {
+    if (*string >= 'A' && *string <= 'Z')
+      return true;
+  }
+  return false;
+}
+
 // Lowercase switches for backwards compatiblity *on Windows*.
 #if defined(OS_WIN)
 std::string LowerASCIIOnWindows(const std::string& string) {
   return StringToLowerASCII(string);
 }
 #elif defined(OS_POSIX)
 const std::string& LowerASCIIOnWindows(const std::string& string) {
   return string;
 }
 #endif
@@ skipping 149 matching lines @@
 }
 
 FilePath CommandLine::GetProgram() const {
   return FilePath(argv_[0]);
 }
 
 void CommandLine::SetProgram(const FilePath& program) {
   TrimWhitespace(program.value(), TRIM_ALL, &argv_[0]);
 }
 
 bool CommandLine::HasSwitch(const std::string& switch_string) const {

[tapted, 2015/04/02 01:46:14]

How many times is the std::string version called now? Maybe it's not worth
changing this fn.

[jackhou1, 2015/04/02 04:51:51]

Agreed. 1% of calls.

-  return switches_.find(LowerASCIIOnWindows(switch_string)) != switches_.end();
+#if defined(OS_WIN)
+  if (ContainsUpperASCII(switch_string.c_str()))
+    return switches_.find(LowerASCIIOnWindows(switch_string)) !=
+           switches_.end();
+#endif
+
+  return switches_.find(switch_string) != switches_.end();
 }
 
 bool CommandLine::HasSwitch(const char string_constant[]) const {
-  return HasSwitch(std::string(string_constant));
+  DCHECK(!ContainsUpperASCII(string_constant));

[tapted, 2015/04/02 01:46:14]

I think we can avoid the extra function with something like

DCHECK_EQ(StringToLowerASCII(string_constant), string_constant);

[jackhou1, 2015/04/02 04:51:51]

Done.

+  return switches_.find(std::string(string_constant)) != switches_.end();

[tapted, 2015/04/02 01:46:15]

is the std::string(..) still needed?

[jackhou1, 2015/04/02 04:51:51]

Nope, but the one in the DEBUG_EQ is.

 }
 
 std::string CommandLine::GetSwitchValueASCII(
     const std::string& switch_string) const {
   StringType value = GetSwitchValueNative(switch_string);
   if (!IsStringASCII(value)) {
     DLOG(WARNING) << "Value of switch (" << switch_string << ") must be ASCII.";
     return std::string();
   }
 #if defined(OS_WIN)
   return UTF16ToASCII(value);
 #else
   return value;
 #endif
 }
 
 FilePath CommandLine::GetSwitchValuePath(
     const std::string& switch_string) const {
   return FilePath(GetSwitchValueNative(switch_string));
 }
 
 CommandLine::StringType CommandLine::GetSwitchValueNative(
     const std::string& switch_string) const {
-  SwitchMap::const_iterator result =
-    switches_.find(LowerASCIIOnWindows(switch_string));
+#if defined(OS_WIN)

[tapted, 2015/04/02 01:46:14]

I think we should defer this until there's a char[] overload of this function
and we have baseline on that

[jackhou1, 2015/04/02 04:51:51]

This method is about 9% of lookups during startup, but yeah, there's probably
more to gain by adding a char[] overload.

+  if (ContainsUpperASCII(switch_string.c_str())) {
+    SwitchMap::const_iterator result =
+        switches_.find(LowerASCIIOnWindows(switch_string));
+    return result == switches_.end() ? StringType() : result->second;
+  }
+#endif
+
+  SwitchMap::const_iterator result = switches_.find(switch_string);
   return result == switches_.end() ? StringType() : result->second;
 }
 
 void CommandLine::AppendSwitch(const std::string& switch_string) {
   AppendSwitchNative(switch_string, StringType());
 }
 
 void CommandLine::AppendSwitchPath(const std::string& switch_string,
                                    const FilePath& path) {
   AppendSwitchNative(switch_string, path.value());
@@ skipping 140 matching lines @@
 #if defined(OS_WIN)
       arg = QuoteForCommandLineToArgvW(arg, quote_placeholders);
 #endif
       params.append(arg);
     }
   }
   return params;
 }
 
 }  // namespace base