Do not use ScopedResource for FindFirstFile handle

The current usage of ScopedResource does not work for Android Studio
Profiler, because INVALID_HANDLE_VALUE under their compile configuration
is defined as ((HANDLE)(LONG_PTR)-1), which is a reinterpret_cast, and
reinterpret_casts cannot be used in constexpr. Template arguments are
required to be constexprs.

Since there is just one use case of this, it's easier to call FindClose
directly. If we have many more uses of FindFirstFile handle in the
future, we can look into defining a different scoped object for it.

Bug: 182165266
Change-Id: I27bc917643b1b13d4bfd4c1ad9299ee3324a2b08

2 files changed, 9 insertions, 20 deletions

diff --git a/include/perfetto/ext/base/file_utils.h b/include/perfetto/ext/base/file_utils.h
index 289e5c1bf..17271daf3 100644
--- a/include/perfetto/ext/base/file_utils.h
+++ b/include/perfetto/ext/base/file_utils.h
@@ -34,8 +34,6 @@ namespace base {
 
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
 using FileOpenMode = int;
-
-int CloseFindHandle(PlatformHandle handle);
 #else
 using FileOpenMode = mode_t;
 #endif
diff --git a/src/base/file_utils.cc b/src/base/file_utils.cc
index 9275bfa55..4c0698035 100644
--- a/src/base/file_utils.cc
+++ b/src/base/file_utils.cc
@@ -44,23 +44,8 @@ namespace perfetto {
 namespace base {
 namespace {
 constexpr size_t kBufSize = 2048;
-
-#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
-using ScopedFindHandle = ScopedResource<HANDLE,
-                                        &CloseFindHandle,
-                                        /*InvalidValue=*/INVALID_HANDLE_VALUE,
-                                        /*CheckClose=*/true>;
-#endif
 }  // namespace
 
-#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
-int CloseFindHandle(HANDLE handle) {
-  // FindClose returns non-zero on success, but CloseFunction of ScopedResource
-  // expects 0 on success.
-  return FindClose(handle) != 0 ? 0 : 1;
-}
-#endif
-
 ssize_t Read(int fd, void* dst, size_t dst_size) {
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
   return _read(fd, dst, static_cast<unsigned>(dst_size));
@@ -252,12 +237,17 @@ base::Status ListFilesRecursive(const std::string& dir_path,
     if (glob_path.length() + 1 > MAX_PATH)
       return base::ErrStatus("Directory path %s is too long", dir_path.c_str());
     WIN32_FIND_DATAA ffd;
-    ScopedFindHandle hFind(FindFirstFileA(glob_path.c_str(), &ffd));
-    if (!hFind)
+    // We do not use a ScopedResource for the HANDLE from FindFirstFile because
+    // the invalid value INVALID_HANDLE_VALUE is not a constexpr under some
+    // compile configurations, and thus cannot be used as a template argument.
+    HANDLE hFind = FindFirstFileA(glob_path.c_str(), &ffd);
+    if (hFind == INVALID_HANDLE_VALUE) {
       // For empty directories, there should be at least one entry '.'.
       // If FindFirstFileA returns INVALID_HANDLE_VALUE, this means directory
       // couldn't be accessed.
+      FindClose(hFind);
       return base::ErrStatus("Failed to open directory %s", cur_dir.c_str());
+    }
     do {
       if (strcmp(ffd.cFileName, ".") == 0 || strcmp(ffd.cFileName, "..") == 0)
         continue;
@@ -269,7 +259,8 @@ base::Status ListFilesRecursive(const std::string& dir_path,
         PERFETTO_CHECK(full_path.length() > root_dir_path.length());
         output.push_back(full_path.substr(root_dir_path.length()));
       }
-    } while (FindNextFileA(hFind.get(), &ffd));
+    } while (FindNextFileA(hFind, &ffd));
+    FindClose(hFind);
 #else
     ScopedDir dir = ScopedDir(opendir(cur_dir.c_str()));
     if (!dir) {