commit 9bad4301edb9bc187b239920e18d36e53c3cbcea
author Primiano Tucci <primiano@google.com> Mon Jul 31 19:07:46 2023 +0100
committer Primiano Tucci <primiano@google.com> Tue Aug 01 09:59:06 2023 +0000
tree 5fa7837e40b1d7014eed4ca632cc898fd89197b1
parent 3c525f435e61c4762739513cae5d8b1ffa4a752e

Fix tests on Windows

_mktemp_s() needs to see the full path, otherwise will
keep generating deterministically the same "unique" name,
causing later a failure when creating a 2nd TempFile
because the path exists already.
This bug was unvealed after lalitm@ added a test that
makes use of more than one TempFile within a test.

Test: manual, build and run perfetto_unittests.exe
Bug: 293871774
Change-Id: I17b24a5428a257371e750f72c086343383b708ef

src/base/temp_file.cc

diff --git a/src/base/temp_file.cc b/src/base/temp_file.cc
index 11e3f37..26092d3 100644
--- a/src/base/temp_file.cc
+++ b/src/base/temp_file.cc
@@ -35,19 +35,20 @@
 #include "perfetto/ext/base/file_utils.h"
 #include "perfetto/ext/base/string_utils.h"
 
+namespace perfetto {
+namespace base {
+
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
 namespace {
-std::string GetTempName() {
-  char name[] = "perfetto-XXXXXX";
-  PERFETTO_CHECK(_mktemp_s(name, sizeof(name)) == 0);
-  return name;
+std::string GetTempFilePathWin() {
+  std::string tmplt = GetSysTempDir() + "\\perfetto-XXXXXX";
+  StackString<255> name("%s\\perfetto-XXXXXX", GetSysTempDir().c_str());
+  PERFETTO_CHECK(_mktemp_s(name.mutable_data(), name.len() + 1) == 0);
+  return name.ToStdString();
 }
 }  // namespace
 #endif
 
-namespace perfetto {
-namespace base {
-
 std::string GetSysTempDir() {
   const char* tmpdir = nullptr;
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
@@ -71,7 +72,7 @@
 TempFile TempFile::Create() {
   TempFile temp_file;
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
-  temp_file.path_ = GetSysTempDir() + "\\" + GetTempName();
+  temp_file.path_ = GetTempFilePathWin();
   // Several tests want to read-back the temp file while still open. On Windows,
   // that requires FILE_SHARE_READ. FILE_SHARE_READ is NOT settable when using
   // the POSIX-compat equivalent function _open(). Hence the CreateFileA +
@@ -133,7 +134,7 @@
 TempDir TempDir::Create() {
   TempDir temp_dir;
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
-  temp_dir.path_ = GetSysTempDir() + "\\" + GetTempName();
+  temp_dir.path_ = GetTempFilePathWin();
   PERFETTO_CHECK(_mkdir(temp_dir.path_.c_str()) == 0);
 #else
   temp_dir.path_ = GetSysTempDir() + "/perfetto-XXXXXXXX";

src/protozero/filtering/filter_util_unittest.cc

diff --git a/src/protozero/filtering/filter_util_unittest.cc b/src/protozero/filtering/filter_util_unittest.cc
index 684709e..831e4f4 100644
--- a/src/protozero/filtering/filter_util_unittest.cc
+++ b/src/protozero/filtering/filter_util_unittest.cc
@@ -36,15 +36,16 @@
 
 std::string FilterToText(FilterUtil& filter,
                          std::optional<std::string> bytecode = {}) {
-  perfetto::base::TempFile tmp = perfetto::base::TempFile::Create();
+  std::string tmp_path = perfetto::base::TempFile::Create().path();
   {
-    perfetto::base::ScopedFstream tmp_stream(fopen(tmp.path().c_str(), "w"));
+    perfetto::base::ScopedFstream tmp_stream(fopen(tmp_path.c_str(), "wb"));
+    PERFETTO_CHECK(!!tmp_stream);
     filter.set_print_stream_for_testing(*tmp_stream);
     filter.PrintAsText(bytecode);
     filter.set_print_stream_for_testing(stdout);
   }
   std::string output;
-  PERFETTO_CHECK(perfetto::base::ReadFile(tmp.path(), &output));
+  PERFETTO_CHECK(perfetto::base::ReadFile(tmp_path, &output));
   // Make the output a bit more compact.
   output = std::regex_replace(output, std::regex(" +"), " ");
   return std::regex_replace(output, std::regex(" +\\n"), "\n");