[Linux][Udisks2] Fix crash on disk unmount

8 files changed, 66 insertions, 24 deletions

diff --git a/xbmc/platform/linux/storage/UDevProvider.cpp b/xbmc/platform/linux/storage/UDevProvider.cpp
index aba9a85a6d..b42e94d58d 100644
--- a/xbmc/platform/linux/storage/UDevProvider.cpp
+++ b/xbmc/platform/linux/storage/UDevProvider.cpp
@@ -232,7 +232,7 @@ bool CUDevProvider::PumpDriveChangeEvents(IStorageEventsCallback *callback)
     const char *action  = udev_device_get_action(dev);
     if (action)
     {
-      MEDIA_DETECT::StorageDevice storageDevice;
+      MEDIA_DETECT::STORAGE::StorageDevice storageDevice;
       const char *udev_label = udev_device_get_property_value(dev, "ID_FS_LABEL");
       const char *mountpoint = get_mountpoint(udev_device_get_devnode(dev));
       if (udev_label)
@@ -263,7 +263,8 @@ bool CUDevProvider::PumpDriveChangeEvents(IStorageEventsCallback *callback)
       {
         const char *optical = udev_device_get_property_value(dev, "ID_CDROM");
         bool isOptical = optical && (strcmp(optical, "1") != 0);
-        storageDevice.optical = isOptical;
+        storageDevice.type =
+            isOptical ? MEDIA_DETECT::STORAGE::Type::OPTICAL : MEDIA_DETECT::STORAGE::Type::UNKNOWN;
 
         if (mountpoint && !isOptical)
         {
diff --git a/xbmc/platform/linux/storage/UDisks2Provider.cpp b/xbmc/platform/linux/storage/UDisks2Provider.cpp
index fb82dd5e50..b3431c7325 100644
--- a/xbmc/platform/linux/storage/UDisks2Provider.cpp
+++ b/xbmc/platform/linux/storage/UDisks2Provider.cpp
@@ -78,12 +78,12 @@ std::string CUDisks2Provider::Filesystem::ToString() const
                              BOOL2SZ(m_isMounted), m_mountPoint.empty() ? "none" : m_mountPoint);
 }
 
-MEDIA_DETECT::StorageDevice CUDisks2Provider::Filesystem::ToStorageDevice() const
+MEDIA_DETECT::STORAGE::StorageDevice CUDisks2Provider::Filesystem::ToStorageDevice() const
 {
-  MEDIA_DETECT::StorageDevice device;
+  MEDIA_DETECT::STORAGE::StorageDevice device;
   device.label = GetDisplayName();
   device.path = GetMountPoint();
-  device.optical = IsOptical();
+  device.type = GetStorageType();
   return device;
 }
 
@@ -97,6 +97,17 @@ bool CUDisks2Provider::Filesystem::IsOptical() const
   return m_block->m_drive->IsOptical();
 }
 
+MEDIA_DETECT::STORAGE::Type CUDisks2Provider::Filesystem::GetStorageType() const
+{
+  if (m_block == nullptr || !m_block->IsReady())
+    return MEDIA_DETECT::STORAGE::Type::UNKNOWN;
+
+  if (IsOptical())
+    return MEDIA_DETECT::STORAGE::Type::OPTICAL;
+
+  return MEDIA_DETECT::STORAGE::Type::UNKNOWN;
+}
+
 std::string CUDisks2Provider::Filesystem::GetMountPoint() const
 {
   return m_mountPoint;
diff --git a/xbmc/platform/linux/storage/UDisks2Provider.h b/xbmc/platform/linux/storage/UDisks2Provider.h
index ab4cc4d1c3..b4c85eeb1e 100644
--- a/xbmc/platform/linux/storage/UDisks2Provider.h
+++ b/xbmc/platform/linux/storage/UDisks2Provider.h
@@ -95,6 +95,12 @@ class CUDisks2Provider : public IStorageProvider
     */
     bool IsOptical() const;
 
+    /*! \brief Get the storage type of this device
+     * @return the storage type (e.g. OPTICAL) or UNKNOWN if
+     * the type couldn't be detected
+    */
+    MEDIA_DETECT::STORAGE::Type GetStorageType() const;
+
     /*! \brief Get the device mount point
      * @return the device mount point
     */
@@ -127,7 +133,7 @@ class CUDisks2Provider : public IStorageProvider
     /*! \brief Get a representation of the device as a storage device abstraction
      * @return the storage device abstraction of the device
     */
-    MEDIA_DETECT::StorageDevice ToStorageDevice() const;
+    MEDIA_DETECT::STORAGE::StorageDevice ToStorageDevice() const;
 
   private:
     bool m_isMounted = false;
diff --git a/xbmc/platform/linux/storage/UDisksProvider.cpp b/xbmc/platform/linux/storage/UDisksProvider.cpp
index 704251851a..7a2a0347d7 100644
--- a/xbmc/platform/linux/storage/UDisksProvider.cpp
+++ b/xbmc/platform/linux/storage/UDisksProvider.cpp
@@ -155,6 +155,14 @@ bool CUDiskDevice::IsOptical() const
   return m_isOptical;
 }
 
+MEDIA_DETECT::STORAGE::Type CUDiskDevice::GetStorageType() const
+{
+  if (IsOptical())
+    return MEDIA_DETECT::STORAGE::Type::OPTICAL;
+
+  return MEDIA_DETECT::STORAGE::Type::UNKNOWN;
+}
+
 bool CUDiskDevice::IsMounted() const
 {
   return m_isMounted;
@@ -175,12 +183,12 @@ bool CUDiskDevice::IsSystemInternal() const
   return m_isSystemInternal;
 }
 
-MEDIA_DETECT::StorageDevice CUDiskDevice::ToStorageDevice() const
+MEDIA_DETECT::STORAGE::StorageDevice CUDiskDevice::ToStorageDevice() const
 {
-  MEDIA_DETECT::StorageDevice device;
+  MEDIA_DETECT::STORAGE::StorageDevice device;
   device.label = GetDisplayName();
   device.path = GetMountPoint();
-  device.optical = IsOptical();
+  device.type = GetStorageType();
   return device;
 }
 
diff --git a/xbmc/platform/linux/storage/UDisksProvider.h b/xbmc/platform/linux/storage/UDisksProvider.h
index d1c831c421..c5372f8a35 100644
--- a/xbmc/platform/linux/storage/UDisksProvider.h
+++ b/xbmc/platform/linux/storage/UDisksProvider.h
@@ -30,6 +30,12 @@ public:
   */
   bool IsApproved() const;
 
+  /*! \brief Get the storage type of this device
+   * @return the storage type (e.g. OPTICAL) or UNKNOWN if
+   * the type couldn't be detected
+  */
+  MEDIA_DETECT::STORAGE::Type GetStorageType() const;
+
   /*! \brief Check if the device is optical
     * @return true if the device is optical, false otherwise
   */
@@ -68,7 +74,7 @@ public:
   /*! \brief Get a representation of the device as a storage device abstraction
     * @return the storage device abstraction of the device
   */
-  MEDIA_DETECT::StorageDevice ToStorageDevice() const;
+  MEDIA_DETECT::STORAGE::StorageDevice ToStorageDevice() const;
 
 private:
   std::string m_UDI;
diff --git a/xbmc/storage/IStorageProvider.h b/xbmc/storage/IStorageProvider.h
index f5ed900bb0..981e81d082 100644
--- a/xbmc/storage/IStorageProvider.h
+++ b/xbmc/storage/IStorageProvider.h
@@ -18,16 +18,26 @@
 
 namespace MEDIA_DETECT
 {
+namespace STORAGE
+{
+/*! \brief Abstracts a generic storage device type*/
+enum class Type
+{
+  UNKNOWN, /*!< the storage type is unknown */
+  OPTICAL /*!< an optical device (e.g. DVD or Bluray) */
+};
+
 /*! \brief Abstracts a generic storage device */
 struct StorageDevice
 {
   /*! Device name/label */
-  std::string label;
+  std::string label{};
   /*! Device mountpoint/path */
-  std::string path;
-  /*! If the device is optical */
-  bool optical;
+  std::string path{};
+  /*! The storage type (e.g. OPTICAL) */
+  STORAGE::Type type{STORAGE::Type::UNKNOWN};
 };
+} // namespace STORAGE
 } // namespace MEDIA_DETECT
 
 class IStorageEventsCallback
@@ -38,17 +48,17 @@ public:
   /*! \brief Callback executed when a new storage device is added
     * @param device the storage device
     */
-  virtual void OnStorageAdded(const MEDIA_DETECT::StorageDevice& device) = 0;
+  virtual void OnStorageAdded(const MEDIA_DETECT::STORAGE::StorageDevice& device) = 0;
 
   /*! \brief Callback executed when a new storage device is safely removed
     * @param device the storage device
     */
-  virtual void OnStorageSafelyRemoved(const MEDIA_DETECT::StorageDevice& device) = 0;
+  virtual void OnStorageSafelyRemoved(const MEDIA_DETECT::STORAGE::StorageDevice& device) = 0;
 
   /*! \brief Callback executed when a new storage device is unsafely removed
     * @param device the storage device
     */
-  virtual void OnStorageUnsafelyRemoved(const MEDIA_DETECT::StorageDevice& device) = 0;
+  virtual void OnStorageUnsafelyRemoved(const MEDIA_DETECT::STORAGE::StorageDevice& device) = 0;
 };
 
 class IStorageProvider
diff --git a/xbmc/storage/MediaManager.cpp b/xbmc/storage/MediaManager.cpp
index ad070ed5a8..7556933334 100644
--- a/xbmc/storage/MediaManager.cpp
+++ b/xbmc/storage/MediaManager.cpp
@@ -689,7 +689,7 @@ std::vector<std::string> CMediaManager::GetDiskUsage()
   return m_platformStorage->GetDiskUsage();
 }
 
-void CMediaManager::OnStorageAdded(const MEDIA_DETECT::StorageDevice& device)
+void CMediaManager::OnStorageAdded(const MEDIA_DETECT::STORAGE::StorageDevice& device)
 {
 #ifdef HAS_DVD_DRIVE
   const std::shared_ptr<CSettings> settings = CServiceBroker::GetSettingsComponent()->GetSettings();
@@ -702,7 +702,7 @@ void CMediaManager::OnStorageAdded(const MEDIA_DETECT::StorageDevice& device)
     }
     else
     {
-      if (device.optical)
+      if (device.type == MEDIA_DETECT::STORAGE::Type::OPTICAL)
       {
         if (MEDIA_DETECT::CAutorun::ExecuteAutorun(device.path))
         {
@@ -723,13 +723,13 @@ void CMediaManager::OnStorageAdded(const MEDIA_DETECT::StorageDevice& device)
 #endif
 }
 
-void CMediaManager::OnStorageSafelyRemoved(const MEDIA_DETECT::StorageDevice& device)
+void CMediaManager::OnStorageSafelyRemoved(const MEDIA_DETECT::STORAGE::StorageDevice& device)
 {
   CGUIDialogKaiToast::QueueNotification(CGUIDialogKaiToast::Info, g_localizeStrings.Get(13023),
                                         device.label, TOAST_DISPLAY_TIME, false);
 }
 
-void CMediaManager::OnStorageUnsafelyRemoved(const MEDIA_DETECT::StorageDevice& device)
+void CMediaManager::OnStorageUnsafelyRemoved(const MEDIA_DETECT::STORAGE::StorageDevice& device)
 {
   CGUIDialogKaiToast::QueueNotification(CGUIDialogKaiToast::Warning, g_localizeStrings.Get(13022),
                                         device.label);
diff --git a/xbmc/storage/MediaManager.h b/xbmc/storage/MediaManager.h
index d88361ee09..c90e5c6c1c 100644
--- a/xbmc/storage/MediaManager.h
+++ b/xbmc/storage/MediaManager.h
@@ -88,19 +88,19 @@ public:
     * \sa IStorageEventsCallback
     * @param device the storage device
   */
-  void OnStorageAdded(const MEDIA_DETECT::StorageDevice& device) override;
+  void OnStorageAdded(const MEDIA_DETECT::STORAGE::StorageDevice& device) override;
 
   /*! \brief Callback executed when a new storage device is safely removed
     * \sa IStorageEventsCallback
     * @param device the storage device
   */
-  void OnStorageSafelyRemoved(const MEDIA_DETECT::StorageDevice& device) override;
+  void OnStorageSafelyRemoved(const MEDIA_DETECT::STORAGE::StorageDevice& device) override;
 
   /*! \brief Callback executed when a new storage device is unsafely removed
     * \sa IStorageEventsCallback
     * @param device the storage device
   */
-  void OnStorageUnsafelyRemoved(const MEDIA_DETECT::StorageDevice& device) override;
+  void OnStorageUnsafelyRemoved(const MEDIA_DETECT::STORAGE::StorageDevice& device) override;
 
   void OnJobComplete(unsigned int jobID, bool success, CJob *job) override { }