Enable positional parameters for base::vsnprintf and base::vswprintf on Windows.

This is to avoid bugs like http://code.google.com/p/chromium/issues/detail?id=118064 in the future. Positional parameters are supported by the VC++ runtime via the set of "_p" flavored printf-like routines. MSDN claims that "By default the positional functions behave identically to the non position ones, if no positional formatting is present."

There is a little difference that require some attention though. Currently base::vsnprintf and base::vswprintf wrappers use "_s" functions (so called "security enhanced versions"). Judging by looking at the CRT code, _p functions implement the same checks as _s ones do. The base wrappers call the CRT routines so that count == (sizeOfBuffer - 1). This reduces most of the additional code in _vsnprintf_s (comparing to _vsprintf_p) to no-op. Namely:

1. When truncation happens the tail of the buffer is filled with a pattern:

    _SECURECRT__FILL_STRING(string, sizeInBytes, count + 1);

This does not happen in our case because sizeInBytes == count + 1.

2. The special case check shown below was never true since sizeInBytes != count in our case:

    if (count == 0 && string == NULL && sizeInBytes == 0)

3. The following checks in _vsnprintf_s are also implemented in _vsnprintf_helper:

_VALIDATE_RETURN(format != NULL, EINVAL, -1);
_VALIDATE_RETURN(string != NULL && sizeInBytes > 0, EINVAL, -1);

The only remaining difference between _vsnprintf_s and _vsprintf_p is that the former NULL-terminates the buffer and fills the rest a pattern if _vsnprintf_helper failed because of any reason other than truncation:

    string[0] = 0;
    _SECURECRT__FILL_STRING(string, sizeInBytes, 1);

This CL write NULL to the end of the buffer in any error case (truncation or other reason).

Review URL: http://codereview.chromium.org/9702002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127788 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 27 insertions, 8 deletions

diff --git a/base/string_util_win.h b/base/string_util_win.h
index 8836f74..41bb562 100644
--- a/base/string_util_win.h
+++ b/base/string_util_win.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -35,9 +35,12 @@ inline int strncmp16(const char16* s1, const char16* s2, size_t count) {
 
 inline int vsnprintf(char* buffer, size_t size,
                      const char* format, va_list arguments) {
-  int length = vsnprintf_s(buffer, size, size - 1, format, arguments);
-  if (length < 0)
-    return _vscprintf(format, arguments);
+  int length = _vsprintf_p(buffer, size, format, arguments);
+  if (length < 0) {
+    if (size > 0)
+      buffer[0] = 0;
+    return _vscprintf_p(format, arguments);
+  }
   return length;
 }
 
@@ -45,9 +48,12 @@ inline int vswprintf(wchar_t* buffer, size_t size,
                      const wchar_t* format, va_list arguments) {
   DCHECK(IsWprintfFormatPortable(format));
 
-  int length = _vsnwprintf_s(buffer, size, size - 1, format, arguments);
-  if (length < 0)
-    return _vscwprintf(format, arguments);
+  int length = _vswprintf_p(buffer, size, format, arguments);
+  if (length < 0) {
+    if (size > 0)
+      buffer[0] = 0;
+    return _vscwprintf_p(format, arguments);
+  }
   return length;
 }
 
diff --git a/base/stringprintf_unittest.cc b/base/stringprintf_unittest.cc
index ffb9c77..6f4458b 100644
--- a/base/stringprintf_unittest.cc
+++ b/base/stringprintf_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -149,4 +149,17 @@ TEST(StringPrintfTest, Invalid) {
 }
 #endif
 
+// Test that the positional parameters work.
+TEST(StringPrintfTest, PositionalParameters) {
+  std::string out;
+  SStringPrintf(&out, "%1$s %1$s", "test");
+  EXPECT_STREQ("test test", out.c_str());
+
+#if defined(OS_WIN)
+  std::wstring wout;
+  SStringPrintf(&wout, L"%1$ls %1$ls", L"test");
+  EXPECT_STREQ(L"test test", wout.c_str());
+#endif
+}
+
 }  // namespace base