SystemMonitor: Pull device type into the device id

chrome/browser/media_gallery/media_device_notifications_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // MediaDeviceNotificationsLinux implementation.
 
 #include "chrome/browser/media_gallery/media_device_notifications_linux.h"
 
 #include <libudev.h>
 #include <mntent.h>
 #include <stdio.h>
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/memory/scoped_generic_obj.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/system_monitor/system_monitor.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/media_gallery/media_device_notifications_utils.h"
+#include "chrome/browser/media_gallery/media_storage_util.h"
 
 namespace {
 
 // List of file systems we care about.
 const char* const kKnownFileSystems[] = {
   "ext2",
   "ext3",
   "ext4",
   "fat",
   "hfsplus",
@@ skipping 40 matching lines @@
 // ScopedGenericObj functor for UdevDeviceObjectRelease().
 class ScopedReleaseUdevDeviceObject {
  public:
   void operator()(struct udev_device* device) const {
     udev_device_unref(device);
   }
 };
 
 // Get the device information using udev library.
 // On success, returns true and fill in |id| and |name|.
-bool GetDeviceInfo(const std::string& device_path,
-                   std::string* id,
+bool GetDeviceInfo(const std::string& device_path, std::string* id,
                    string16* name) {
   DCHECK(!device_path.empty());
 
   ScopedGenericObj<struct udev*, ScopedReleaseUdevObject> udev_obj(udev_new());
   if (!udev_obj.get())
     return false;
 
   struct stat device_stat;
   if (stat(device_path.c_str(), &device_stat) < 0)
     return false;
 
   char device_type;
   if (S_ISCHR(device_stat.st_mode))
     device_type = 'c';
   else if (S_ISBLK(device_stat.st_mode))
     device_type = 'b';
   else
     return false;  // Not a supported type.
 
   ScopedGenericObj<struct udev_device*, ScopedReleaseUdevDeviceObject>
       device(udev_device_new_from_devnum(udev_obj, device_type,
                                          device_stat.st_rdev));
   if (!device.get())
     return false;
 
   // Construct a device name using label or manufacturer(vendor and model)
   // details.
-  std::string device_label;
-  const char* device_name = NULL;
-  if ((device_name = udev_device_get_property_value(device, kLabel)) ||
-      (device_name = udev_device_get_property_value(device, kSerial))) {
-    device_label = device_name;
-  } else {
-    // Format: VendorInfo ModelInfo
-    // E.g.: KnCompany Model2010
-    const char* vendor_name = NULL;
-    if ((vendor_name = udev_device_get_property_value(device, kVendor)))
-      device_label = vendor_name;
+  if (name) {

[kmadhusu, 2012/08/20 17:13:50]

Are you planning to call this function with NULL output params? 

From my understanding, this function is always called with valid output params.
So, it is sufficient to validate the params before assigning some value to it
"DCHECK(name) and DCHECK(id)".

+    std::string device_label;
+    const char* device_name = NULL;
+    if ((device_name = udev_device_get_property_value(device, kLabel)) ||
+        (device_name = udev_device_get_property_value(device, kSerial))) {
+      device_label = device_name;
+    } else {
+      // Format: VendorInfo ModelInfo
+      // E.g.: KnCompany Model2010
+      const char* vendor_name = NULL;
+      if ((vendor_name = udev_device_get_property_value(device, kVendor)))
+        device_label = vendor_name;
 
-    const char* model_name = NULL;
-    if ((model_name = udev_device_get_property_value(device, kModel))) {
-      if (!device_label.empty())
-        device_label += kSpaceDelim;
-      device_label += model_name;
+      const char* model_name = NULL;
+      if ((model_name = udev_device_get_property_value(device, kModel))) {
+        if (!device_label.empty())
+          device_label += kSpaceDelim;
+        device_label += model_name;
+      }
     }
+
+    if (IsStringUTF8(device_label))
+      *name = UTF8ToUTF16(device_label);
   }
 
-  if (IsStringUTF8(device_label))
-    *name = UTF8ToUTF16(device_label);
+  if (id) {
+    std::string unique_id;
+    const char* uuid  = NULL;
+    if ((uuid = udev_device_get_property_value(device, kFsUUID))) {
+      unique_id = std::string(kFSUniqueIdPrefix) + uuid;
+    } else {
+      // If one of the vendor, model, serial information is missing, its value
+      // in the string is empty.
+      // Format: VendorModelSerial:VendorInfo:ModelInfo:SerialShortInfo
+      // E.g.: VendorModelSerial:Kn:DataTravel_12.10:8000000000006CB02CDB
+      const char* vendor = udev_device_get_property_value(device, kVendorID);
+      const char* model = udev_device_get_property_value(device, kModelID);
+      const char* serial_short = udev_device_get_property_value(device,
+                                                                kSerialShort);
+      if (!vendor && !model && !serial_short)
+        return false;
 
-  const char* uuid  = NULL;
-  if ((uuid = udev_device_get_property_value(device, kFsUUID))) {
-    *id = std::string(kFSUniqueIdPrefix) + uuid;
-  } else {
-    // If one of the vendor, model, serial information is missing, its value in
-    // the string is empty.
-    // Format: VendorModelSerial:VendorInfo:ModelInfo:SerialShortInfo
-    // E.g.: VendorModelSerial:Kn:DataTravel_12.10:8000000000006CB02CDB
-    const char* vendor = udev_device_get_property_value(device, kVendorID);
-    const char* model = udev_device_get_property_value(device, kModelID);
-    const char* serial_short = udev_device_get_property_value(device,
-                                                              kSerialShort);
-    if (!vendor && !model && !serial_short)
-      return false;
-
-    *id = std::string(kVendorModelSerialPrefix) +
-          (vendor ? vendor : "") + kNonSpaceDelim +
-          (model ? model : "") + kNonSpaceDelim +
-          (serial_short ? serial_short : "");
+      unique_id = std::string(kVendorModelSerialPrefix) +
+                  (vendor ? vendor : "") + kNonSpaceDelim +
+                  (model ? model : "") + kNonSpaceDelim +
+                  (serial_short ? serial_short : "");
+    }
+    *id = chrome::MediaStorageUtil::MakeDeviceId(
+        chrome::MediaStorageUtil::USB_MASS_STORAGE_WITH_DCIM, unique_id);
   }
 
   return true;
 }
 
 }  // namespace
 
 namespace chrome {
 
 using base::SystemMonitor;
@@ skipping 166 matching lines @@
     }
   }
 }
 
 void MediaDeviceNotificationsLinux::CheckAndAddMediaDevice(
     const std::string& mount_device,
     const std::string& mount_point) {
   if (!IsMediaDevice(mount_point))
     return;
 
-  std::string device_id;
-  string16 device_name;
-  bool result = (*get_device_info_func_)(mount_device, &device_id,
-                                         &device_name);
+  std::string id;
+  string16 name;
+  bool result = (*get_device_info_func_)(mount_device, &id, &name);
 
   // Keep track of GetDeviceInfo result, to see how often we fail to get device
   // details.
   UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_info_available",
                         result);
   if (!result)
     return;
 
   // Keep track of device uuid, to see how often we receive empty values.
   UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_uuid_available",
-                        !device_id.empty());
+                        !id.empty());
   UMA_HISTOGRAM_BOOLEAN("MediaDeviceNotification.device_name_available",
-                        !device_name.empty());
+                        !name.empty());
 
-  if (device_id.empty() || device_name.empty())
+  if (id.empty() || name.empty())
     return;
 
   MountDeviceAndId mount_device_and_id;
   mount_device_and_id.mount_device = mount_device;
-  mount_device_and_id.device_id = device_id;
+  mount_device_and_id.device_id = id;
   mount_info_map_[mount_point] = mount_device_and_id;
 
   base::SystemMonitor* system_monitor = base::SystemMonitor::Get();
-  system_monitor->ProcessMediaDeviceAttached(device_id,
-                                             device_name,
-                                             SystemMonitor::TYPE_PATH,
-                                             mount_point);
+  system_monitor->ProcessMediaDeviceAttached(id, name, mount_point);
 }
 
 void MediaDeviceNotificationsLinux::RemoveOldDevice(
     const std::string& device_id) {
   base::SystemMonitor* system_monitor = base::SystemMonitor::Get();
   system_monitor->ProcessMediaDeviceDetached(device_id);
 }
 
 }  // namespace chrome