base: various cleanups in base/files/drive_info

Bug: 325307453
Change-Id: I76aa699fbcae880c018d35d140c7985863e4ce36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6038998
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Sean Maher <spvw@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387161}

base/BUILD.gn

@@ -1532,6 +1532,7 @@ component("base") {
       "debug/crash_logging.h",
       "debug/stack_trace.cc",
       "debug/stack_trace.h",
+      "files/drive_info.cc",
       "files/drive_info.h",
       "files/file_enumerator.cc",
       "files/file_enumerator.h",

base/files/drive_info.cc

@@ -4,68 +4,11 @@
 
 #include "base/files/drive_info.h"
 
-#include "base/files/file.h"
-#include "base/files/file_path.h"
-#include "build/build_config.h"
-
-#if BUILDFLAG(IS_MAC)
-#include <IOKit/IOKitLib.h>
-#endif
-
 namespace base {
 
 DriveInfo::DriveInfo() = default;
+DriveInfo::DriveInfo(DriveInfo&&) noexcept = default;
+DriveInfo& DriveInfo::operator=(DriveInfo&&) noexcept = default;
 DriveInfo::~DriveInfo() = default;
-DriveInfo::DriveInfo(DriveInfo&&) = default;
-DriveInfo& DriveInfo::operator=(DriveInfo&&) = default;
-
-#if BUILDFLAG(IS_WIN)
-std::optional<DriveInfo> GetFileDriveInfo(const FilePath& path) {
-  std::vector<base::FilePath::StringType> components = path.GetComponents();
-
-  base::File volume(base::FilePath(L"\\\\.\\" + components[0]),
-                    base::File::FLAG_OPEN);
-  if (!volume.IsValid()) {
-    return std::nullopt;
-  }
-
-  STORAGE_PROPERTY_QUERY seek_query = {
-      .PropertyId = StorageDeviceSeekPenaltyProperty,
-      .QueryType = PropertyStandardQuery};
-
-  DEVICE_SEEK_PENALTY_DESCRIPTOR seek_result;
-  DWORD bytes_returned;
-
-  BOOL success =
-      DeviceIoControl(volume.GetPlatformFile(), IOCTL_STORAGE_QUERY_PROPERTY,
-                      &seek_query, sizeof(seek_query), &seek_result,
-                      sizeof(seek_result), &bytes_returned, nullptr);
-
-  if (success == FALSE || bytes_returned < sizeof(seek_result)) {
-    return std::nullopt;
-  }
-
-  STORAGE_PROPERTY_QUERY bus_query = {.PropertyId = StorageDeviceProperty,
-                                      .QueryType = PropertyStandardQuery};
-
-  STORAGE_DEVICE_DESCRIPTOR bus_result;
-
-  success =
-      DeviceIoControl(volume.GetPlatformFile(), IOCTL_STORAGE_QUERY_PROPERTY,
-                      &bus_query, sizeof(bus_query), &bus_result,
-                      sizeof(bus_result), &bytes_returned, nullptr);
-
-  if (success == FALSE || bytes_returned < sizeof(bus_result)) {
-    return std::nullopt;
-  }
-
-  DriveInfo info;
-  info.has_seek_penalty = seek_result.IncursSeekPenalty != FALSE;
-  info.volume_size_bytes = bus_result.Size;
-  info.is_usb = bus_result.BusType == BusTypeUsb;
-  info.is_removable = bus_result.RemovableMedia == TRUE;
-  return info;
-}
-#endif
 
 }  // namespace base

base/files/drive_info.h

@@ -6,6 +6,8 @@
 #define BASE_FILES_DRIVE_INFO_H_
 
 #include <optional>
+#include <string>
+#include <string_view>
 
 #include "base/base_export.h"
 #include "base/files/file_path.h"
@@ -16,6 +18,7 @@
 #endif
 
 namespace base {
+
 // Used to hold information about either a drive, or of a combination of a
 // partition residing on a drive and the drive itself, depending on how the
 // object was constructed. In general, when calling GetFileDriveInfo(), this
@@ -24,16 +27,18 @@ namespace base {
 // obtained via IOServiceGetMatchingService() with `kIOMediaWholeKey` set to
 // `true`.
 //
+// Each of these parameters can fail to be retrieved from the OS, and so they
+// can each be empty.
+//
 // If you add more fields to this structure (platform-specific fields are OK),
 // make sure to update all functions that use it in
 // file_util_{win|posix|apple}.{cc,mm}, too.
-class BASE_EXPORT DriveInfo {
- public:
+struct BASE_EXPORT DriveInfo {
   DriveInfo();
   DriveInfo(const DriveInfo&) = delete;
-  DriveInfo(DriveInfo&&);
+  DriveInfo(DriveInfo&&) noexcept;
   DriveInfo& operator=(const DriveInfo&) = delete;
-  DriveInfo& operator=(DriveInfo&&);
+  DriveInfo& operator=(DriveInfo&&) noexcept;
   ~DriveInfo();
 
   // Whether the drive has a seek penalty (i.e. is or is not a spinning disk).
@@ -73,16 +78,14 @@ class BASE_EXPORT DriveInfo {
 };
 
 // Returns information about the drive on which sits the given file. Also see
-// |DriveInfo| - in particular, both the drive itself as well as
-// individual attributes thereof can fail to be collected, depending on the
-// drive.
+// `DriveInfo`.
 BASE_EXPORT std::optional<DriveInfo> GetFileDriveInfo(
     const FilePath& file_path);
 
 #if BUILDFLAG(IS_MAC)
 // BSD name is the file found under `/dev`, not the full path including "/dev".
 BASE_EXPORT std::optional<DriveInfo> GetBSDNameDriveInfo(
-    const std::string_view bsd_name);
+    std::string_view bsd_name);
 
 // The IO Object is the underlying handle to the drive device. This function can
 // be used if already iterating over drives matching certain characteristics.

base/files/drive_info_apple.mm

@@ -4,6 +4,12 @@
 
 #include "base/files/drive_info.h"
 
+#include <IOKit/IOTypes.h>
+
+#include "base/files/file.h"
+#include "base/files/file_path.h"
+#include "build/build_config.h"
+
 #if BUILDFLAG(IS_MAC)
 #include <CoreFoundation/CoreFoundation.h>
 #include <DiskArbitration/DiskArbitration.h>
@@ -13,15 +19,7 @@
 #include <IOKit/storage/IOMedia.h>
 #include <IOKit/storage/IOStorageDeviceCharacteristics.h>
 #include <sys/stat.h>
-#endif
 
-#include <IOKit/IOTypes.h>
-
-#include "base/files/file.h"
-#include "base/files/file_path.h"
-#include "build/build_config.h"
-
-#if BUILDFLAG(IS_MAC)
 #include "base/apple/bridging.h"
 #include "base/apple/foundation_util.h"
 #include "base/apple/mach_logging.h"
@@ -48,17 +46,14 @@ T QueryParentsForProperty(io_object_t io_object, const char* key) {
 
 namespace base {
 
-DriveInfo::DriveInfo() = default;
-DriveInfo::~DriveInfo() = default;
-DriveInfo::DriveInfo(DriveInfo&&) = default;
-DriveInfo& DriveInfo::operator=(DriveInfo&&) = default;
-
 std::optional<DriveInfo> GetIOObjectDriveInfo(io_object_t io_object) {
 #if BUILDFLAG(IS_IOS)
   DriveInfo info;
   info.has_seek_penalty = false;
   return info;
-#else  // BUILDFLAG(IS_MAC)
+#else
+  static_assert(BUILDFLAG(IS_MAC));
+
   if (!IOObjectConformsTo(io_object, kIOMediaClass)) {
     return std::nullopt;
   }
@@ -213,7 +208,7 @@ std::optional<DriveInfo> GetIOObjectDriveInfo(io_object_t io_object) {
   }
 
   return drive_info;
-#endif
+#endif  // BUILDFLAG(IS_MAC)
 }
 
 std::optional<DriveInfo> GetBSDNameDriveInfo(const std::string_view bsd_name) {
@@ -222,6 +217,7 @@ std::optional<DriveInfo> GetBSDNameDriveInfo(const std::string_view bsd_name) {
   info.has_seek_penalty = false;
   return info;
 #else  // BUILDFLAG(IS_MAC)
+  CHECK_NE(bsd_name.find("/dev/"), 0UL);
   std::string full_name("/dev/");
   full_name.append(bsd_name);
   apple::ScopedCFTypeRef<DASessionRef> session(
@@ -251,7 +247,8 @@ std::optional<DriveInfo> GetFileDriveInfo(const FilePath& file_path) {
     return std::nullopt;
   }
   char devname_buf[256];
-  const char* dev_name = devname_r(path_stat.st_dev, S_IFBLK, devname_buf, 256);
+  const char* dev_name =
+      devname_r(path_stat.st_dev, S_IFBLK, devname_buf, sizeof(devname_buf));
   if (!dev_name) {
     return std::nullopt;
   }

base/files/drive_info_posix.cc

@@ -21,11 +21,6 @@
 
 namespace base {
 
-DriveInfo::DriveInfo() = default;
-DriveInfo::~DriveInfo() = default;
-DriveInfo::DriveInfo(DriveInfo&&) = default;
-DriveInfo& DriveInfo::operator=(DriveInfo&&) = default;
-
 std::optional<DriveInfo> GetFileDriveInfo(const FilePath& file_path) {
   DriveInfo drive_info;
 #if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA)

base/files/drive_info_win.cc

@@ -10,20 +10,13 @@
 
 #include "base/files/file.h"
 #include "base/files/file_path.h"
-#include "build/build_config.h"
 
 namespace base {
 
-DriveInfo::DriveInfo() = default;
-DriveInfo::~DriveInfo() = default;
-DriveInfo::DriveInfo(DriveInfo&&) = default;
-DriveInfo& DriveInfo::operator=(DriveInfo&&) = default;
-
 std::optional<DriveInfo> GetFileDriveInfo(const FilePath& path) {
-  std::vector<base::FilePath::StringType> components = path.GetComponents();
+  std::vector<FilePath::StringType> components = path.GetComponents();
 
-  base::File volume(base::FilePath(L"\\\\.\\" + components[0]),
-                    base::File::FLAG_OPEN);
+  File volume(FilePath(L"\\\\.\\" + components[0]), File::FLAG_OPEN);
   if (!volume.IsValid()) {
     return std::nullopt;
   }