mtp: Fix crashes when `LIBMTP_devicestorage_t` `StorageDescription = NULL`.

The MTP spec section 5.2.2.7 allows `StorageDescription` to be
an empty string.

`libmtp` currently translates this to

    char * StorageDescription = NULL

instead of `""` in `ptp_unpack_SI()` via `ptp_unpack_string()`.
(I'm not sure if it's good that it does that, to be followed up on separately.)

`create_storage_name()` until now returned
`g_strdup(storage->StorageDescription)`, which returns `NULL` if `NULL` is
given, and thus `get_storage_info()` would eventually call

    char *storage_name = NULL = create_storage_name(storage);
    g_file_info_set_name (info, storage_name = NULL);
    g_file_info_set_display_name (info, storage_name = NULL);

resulting in assertion failures in `gvfsd`:

    g_file_info_set_name: assertion 'name != NULL' failed
    g_file_info_set_display_name: assertion 'display_name != NULL' failed

as well as crashes in file managers like Thunar:

    g_file_get_child: assertion 'name != NULL' failed

and warnings in Nautilus like:

    Got GFileInfo with NULL name in mtp://Ricoh_Company__Ltd._RICOH_THETA_V_00165759/, ignoring. This shouldn't happen unless the gvfs backend is broken.

This commit fixes it by adding a contract to `create_storage_name()`
that it will never represent empty strings as NULL.

1 files changed, 26 insertions, 1 deletions

diff --git a/daemon/gvfsbackendmtp.c b/daemon/gvfsbackendmtp.c
index d3c6daf2..176b3528 100644
--- a/daemon/gvfsbackendmtp.c
+++ b/daemon/gvfsbackendmtp.c
@@ -132,6 +132,18 @@ handle_event (EventData *data, GVfsBackendMtp *backend);
  * Storage name helper
  ************************************************/
 
+/**
+ * create_storage_name:
+ *
+ * Returns a unique, printable storage name for a LIBMTP_devicestorage_t
+ * based on its StorageDescription, appending the storage ID if necessary
+ * to make it unique.
+ *
+ * The caller takes ownership of the returned string.
+ * This function never returns NULL strings.
+ *
+ * The passed-in `storage->StorageDescription` may be NULL.
+ */
 static char *create_storage_name (const LIBMTP_devicestorage_t *storage)
 {
   /* The optional post-fixing of storage's name with ID requires us to
@@ -145,6 +157,9 @@ static char *create_storage_name (const LIBMTP_devicestorage_t *storage)
   gboolean is_unique = TRUE;
   const LIBMTP_devicestorage_t *tmp_storage;
 
+  /* `storage->StorageDescription` may be NULL, so we ensure to only use
+     functions that can handle this, like `g_strcmp0()`. */
+
   /* Forward search for duplicates */
   for (tmp_storage = storage->next; tmp_storage != 0; tmp_storage = tmp_storage->next) {
     if (!g_strcmp0 (storage->StorageDescription, tmp_storage->StorageDescription)) {
@@ -167,7 +182,17 @@ static char *create_storage_name (const LIBMTP_devicestorage_t *storage)
   /* If description is unique, we can use it as storage name; otherwise,
      we add storage ID to it */
   if (is_unique) {
-    return g_strdup (storage->StorageDescription);
+    /* Never return a NULL string (`g_strdup` returns NULL on NULL).
+       Use the storage ID on empty strings to avoid duplicate entries
+       for devices with multiple storages without description. */
+    if (storage->StorageDescription && strlen (storage->StorageDescription) > 0) {
+      return g_strdup (storage->StorageDescription);
+    } else {
+      /* Translators: This is shown as the name for MTP devices
+       *              without StorageDescription.
+       *              The %X is the formatted storage ID. */
+      return g_strdup_printf (_("Storage (%X)"), storage->id);
+    }
   } else {
     return g_strdup_printf ("%s (%X)", storage->StorageDescription, storage->id);
   }