Detecting and fixing stringprintf.h format bugs
The print functions in stringprintf.h were not annotated for /analyze so
13 Windows specific format-string bugs accumulated. This annotates the
functions so that the /analyze builder will find the problems and fixes
the bugs.
R=thestig@chromium.org,wfh@chromium.org,jam@chromium.org
Skipping wstring presubmit checks - no new wstring usage is added
NOPRESUBMIT=true
BUG=427616
Review URL: https://codereview.chromium.org/1372153002
Cr-Commit-Position: refs/heads/master@{#352659}
CrOS-Libchrome-Original-Commit: 5604a11d546e02859db5dc75af72ce27bc14c158
diff --git a/base/files/file_path.h b/base/files/file_path.h
index 25b8391..fba2f98 100644
--- a/base/files/file_path.h
+++ b/base/files/file_path.h
@@ -124,6 +124,15 @@
#define FILE_PATH_USES_WIN_SEPARATORS
#endif // OS_WIN
+// To print path names portably use PRIsFP (based on PRIuS and friends from
+// C99 and format_macros.h) like this:
+// base::StringPrintf("Path is %" PRIsFP ".\n", path.value().c_str());
+#if defined(OS_POSIX)
+#define PRIsFP "s"
+#elif defined(OS_WIN)
+#define PRIsFP "ls"
+#endif // OS_WIN
+
namespace base {
class Pickle;
diff --git a/base/strings/stringprintf.h b/base/strings/stringprintf.h
index 523f7ee..4b2eab1 100644
--- a/base/strings/stringprintf.h
+++ b/base/strings/stringprintf.h
@@ -11,15 +11,26 @@
#include "base/base_export.h"
#include "base/compiler_specific.h"
+#include "build/build_config.h"
+
+#ifdef COMPILER_MSVC
+// For _Printf_format_string_.
+#include <sal.h>
+#else
+// For nacl builds when sal.h is not available.
+#define _Printf_format_string_
+#endif
namespace base {
// Return a C++ string given printf-like input.
-BASE_EXPORT std::string StringPrintf(const char* format, ...)
+BASE_EXPORT std::string StringPrintf(_Printf_format_string_ const char* format,
+ ...)
PRINTF_FORMAT(1, 2) WARN_UNUSED_RESULT;
#if defined(OS_WIN)
-BASE_EXPORT std::wstring StringPrintf(const wchar_t* format, ...)
- WPRINTF_FORMAT(1, 2) WARN_UNUSED_RESULT;
+BASE_EXPORT std::wstring StringPrintf(
+ _Printf_format_string_ const wchar_t* format,
+ ...) WPRINTF_FORMAT(1, 2) WARN_UNUSED_RESULT;
#endif
// Return a C++ string given vprintf-like input.
@@ -27,21 +38,25 @@
PRINTF_FORMAT(1, 0) WARN_UNUSED_RESULT;
// Store result into a supplied string and return it.
-BASE_EXPORT const std::string& SStringPrintf(std::string* dst,
- const char* format, ...)
- PRINTF_FORMAT(2, 3);
+BASE_EXPORT const std::string& SStringPrintf(
+ std::string* dst,
+ _Printf_format_string_ const char* format,
+ ...) PRINTF_FORMAT(2, 3);
#if defined(OS_WIN)
-BASE_EXPORT const std::wstring& SStringPrintf(std::wstring* dst,
- const wchar_t* format, ...)
- WPRINTF_FORMAT(2, 3);
+BASE_EXPORT const std::wstring& SStringPrintf(
+ std::wstring* dst,
+ _Printf_format_string_ const wchar_t* format,
+ ...) WPRINTF_FORMAT(2, 3);
#endif
// Append result to a supplied string.
-BASE_EXPORT void StringAppendF(std::string* dst, const char* format, ...)
- PRINTF_FORMAT(2, 3);
+BASE_EXPORT void StringAppendF(std::string* dst,
+ _Printf_format_string_ const char* format,
+ ...) PRINTF_FORMAT(2, 3);
#if defined(OS_WIN)
-BASE_EXPORT void StringAppendF(std::wstring* dst, const wchar_t* format, ...)
- WPRINTF_FORMAT(2, 3);
+BASE_EXPORT void StringAppendF(std::wstring* dst,
+ _Printf_format_string_ const wchar_t* format,
+ ...) WPRINTF_FORMAT(2, 3);
#endif
// Lower-level routine that takes a va_list and appends to a specified
diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc
index 3baf2ee..8341e51 100644
--- a/ipc/ipc_message_utils.cc
+++ b/ipc/ipc_message_utils.cc
@@ -966,7 +966,7 @@
}
void ParamTraits<HANDLE>::Log(const param_type& p, std::string* l) {
- l->append(base::StringPrintf("0x%X", p));
+ l->append(base::StringPrintf("0x%p", p));
}
void ParamTraits<LOGFONT>::Write(Message* m, const param_type& p) {
diff --git a/mojo/fetcher/network_fetcher.cc b/mojo/fetcher/network_fetcher.cc
index 603cf50..c9a37f2 100644
--- a/mojo/fetcher/network_fetcher.cc
+++ b/mojo/fetcher/network_fetcher.cc
@@ -90,8 +90,8 @@
base::FilePath map_path = temp_dir.AppendASCII(map_name);
// TODO(eseidel): Paths or URLs with spaces will need quoting.
- std::string map_entry =
- base::StringPrintf("%s %s\n", path.value().c_str(), url.spec().c_str());
+ std::string map_entry = base::StringPrintf(
+ "%" PRIsFP " %s\n", path.value().c_str(), url.spec().c_str());
// TODO(eseidel): AppendToFile is missing O_CREAT, crbug.com/450696
if (!PathExists(map_path)) {
base::WriteFile(map_path, map_entry.data(),