Detecting and fixing stringprintf.h format bugs

chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc

Index: chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc
diff --git a/chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc b/chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc
index 861bd1bd8e47d54becded7bab26cb4119cb1b21b..e69c48e61eb08d27e5533016072fe80c07cff762 100644
--- a/chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc
+++ b/chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc
@@ -49,13 +49,24 @@ policy::Schema StorageSchemaManifestHandler::GetSchema(
   }
   file = extension->path().AppendASCII(path);
   if (!base::PathExists(file)) {
+#ifdef OS_WIN

[Lei Zhang, 2015/09/29 03:16:17]

#if defined(OS_WIN), ditto elsewhere, but maybe just do:

#if defined(OS_WIN)
#define FP_MACRO ls
#else
#define FP_MACRO s
#endif
*error = base::StringPrintf("File does not exist: %" FP_MACRO,
file.value().c_str());

[brucedawson, 2015/09/29 21:39:39]

Yes. That is much better. It is cleaner and by defining FP_MACRO near
FilePath::StringType it localizes that knowledge.

[Lei Zhang, 2015/09/29 21:42:37]

SGTM

+    // FilePath::StringType is std::wstring on Windows
+    *error =
+        base::StringPrintf("File does not exist: %ls", file.value().c_str());
+#else
     *error =
         base::StringPrintf("File does not exist: %s", file.value().c_str());
+#endif
     return policy::Schema();
   }
   std::string content;
   if (!base::ReadFileToString(file, &content)) {
+#ifdef OS_WIN
+    // FilePath::StringType is std::wstring on Windows
+    *error = base::StringPrintf("Can't read %ls", file.value().c_str());
+#else
     *error = base::StringPrintf("Can't read %s", file.value().c_str());
+#endif
     return policy::Schema();
   }
   return policy::Schema::Parse(content, error);