diff options
author | Niklas Hambüchen <mail@nh2.me> | 2020-11-15 18:22:07 +0100 |
---|---|---|
committer | Ondrej Holy <oholy@redhat.com> | 2020-12-16 10:28:34 +0000 |
commit | 0cdd813fc2ea63518189e72e647fc492db382eab (patch) | |
tree | 0ca8c077999f7aa1301bc6f1f65050e06badbf1c | |
parent | d45953ee95e259d8cc2bb63930992b932dd47134 (diff) | |
download | gvfs-0cdd813fc2ea63518189e72e647fc492db382eab.tar.gz |
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.
-rw-r--r-- | daemon/gvfsbackendmtp.c | 27 |
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); } |