From e48351e26030ab76db8c072efcf31413fb194a16 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Fri, 22 Jun 2018 12:32:39 +0100 Subject: [PATCH] Set the alternate device ID, not the object itself If the daemon either de-duplicates or replaces the object passed emitted from device-added then the object set as the alternate may not be the same instance as the daemon version. This causes weird things to happen. To make this less fragile, specify the *ID* of the object that should be the alternate device, which allows the daemon to do clever things, and then assign the object from the ID as the last step. Although fixing no bug, this makes implementing future functionality easier. --- plugins/dell/fu-plugin-dell.c | 2 +- plugins/dell/fu-self-test.c | 5 ++++ src/fu-device-private.h | 2 ++ src/fu-device.c | 48 +++++++++++++++++++++++++++++++++++ src/fu-device.h | 5 ++-- src/fu-engine.c | 10 ++++++++ 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/plugins/dell/fu-plugin-dell.c b/plugins/dell/fu-plugin-dell.c index c52b9f644..a57219cd6 100644 --- a/plugins/dell/fu-plugin-dell.c +++ b/plugins/dell/fu-plugin-dell.c @@ -737,7 +737,7 @@ fu_plugin_dell_detect_tpm (FuPlugin *plugin, GError **error) fu_device_add_flag (dev_alt, FWUPD_DEVICE_FLAG_REQUIRE_AC); fu_device_add_flag (dev_alt, FWUPD_DEVICE_FLAG_LOCKED); fu_device_add_icon (dev_alt, "computer"); - fu_device_set_alternate (dev_alt, dev); + fu_device_set_alternate_id (dev_alt, fu_device_get_id (dev)); fu_device_add_parent_guid (dev_alt, tpm_guid); /* If TPM is not owned and at least 1 flash left allow mode switching diff --git a/plugins/dell/fu-self-test.c b/plugins/dell/fu-self-test.c index b66de2a50..ddd96fc4e 100644 --- a/plugins/dell/fu-self-test.c +++ b/plugins/dell/fu-self-test.c @@ -42,6 +42,11 @@ static void _plugin_device_added_cb (FuPlugin *plugin, FuDevice *device, gpointer user_data) { GPtrArray *devices = (GPtrArray *) user_data; + if (fu_device_get_alternate_id (device) != NULL) { + FuDevice *device_alt = _find_device_by_id (devices, fu_device_get_alternate_id (device)); + if (device_alt != NULL) + fu_device_set_alternate (device, device_alt); + } g_ptr_array_add (devices, g_object_ref (device)); } diff --git a/src/fu-device-private.h b/src/fu-device-private.h index 8396fded1..7832b04b7 100644 --- a/src/fu-device-private.h +++ b/src/fu-device-private.h @@ -18,6 +18,8 @@ gboolean fu_device_has_parent_guid (FuDevice *device, guint fu_device_get_order (FuDevice *device); void fu_device_set_order (FuDevice *device, guint order); +void fu_device_set_alternate (FuDevice *device, + FuDevice *alternate); G_END_DECLS diff --git a/src/fu-device.c b/src/fu-device.c index b8683a0d7..ca9afb867 100644 --- a/src/fu-device.c +++ b/src/fu-device.c @@ -26,6 +26,7 @@ static void fu_device_finalize (GObject *object); typedef struct { + gchar *alternate_id; gchar *equivalent_id; FuDevice *alternate; FuDevice *parent; /* noref */ @@ -152,6 +153,44 @@ fu_device_set_equivalent_id (FuDevice *device, const gchar *equivalent_id) priv->equivalent_id = g_strdup (equivalent_id); } +/** + * fu_device_get_alternate: + * @device: A #FuDevice + * + * Gets any alternate device ID. An alternate device may be linked to the primary + * device in some way. + * + * Returns: (transfer none): a #FuDevice or %NULL + * + * Since: 1.0.9 + **/ +const gchar * +fu_device_get_alternate_id (FuDevice *device) +{ + FuDevicePrivate *priv = GET_PRIVATE (device); + g_return_val_if_fail (FU_IS_DEVICE (device), NULL); + return priv->alternate_id; +} + +/** + * fu_device_set_alternate: + * @device: A #FuDevice + * @alternate: Another #FuDevice + * + * Sets any alternate device ID. An alternate device may be linked to the primary + * device in some way. + * + * Since: 1.0.9 + **/ +void +fu_device_set_alternate_id (FuDevice *device, const gchar *alternate_id) +{ + FuDevicePrivate *priv = GET_PRIVATE (device); + g_return_if_fail (FU_IS_DEVICE (device)); + g_free (priv->alternate_id); + priv->alternate_id = g_strdup (alternate_id); +} + /** * fu_device_get_alternate: * @device: A #FuDevice @@ -159,6 +198,10 @@ fu_device_set_equivalent_id (FuDevice *device, const gchar *equivalent_id) * Gets any alternate device. An alternate device may be linked to the primary * device in some way. * + * The alternate object will be matched from the ID set in fu_device_set_alternate_id() + * and will be assigned by the daemon. This means if the ID is not found as an + * added device, then this function will return %NULL. + * * Returns: (transfer none): a #FuDevice or %NULL * * Since: 0.7.2 @@ -179,6 +222,8 @@ fu_device_get_alternate (FuDevice *device) * Sets any alternate device. An alternate device may be linked to the primary * device in some way. * + * This function is only usable by the daemon, not directly from plugins. + * * Since: 0.7.2 **/ void @@ -893,6 +938,8 @@ fu_device_to_string (FuDevice *device) tmp = fwupd_device_to_string (FWUPD_DEVICE (device)); if (tmp != NULL && tmp[0] != '\0') g_string_append (str, tmp); + if (priv->alternate_id != NULL) + fwupd_pad_kv_str (str, "AlternateId", priv->alternate_id); if (priv->equivalent_id != NULL) fwupd_pad_kv_str (str, "EquivalentId", priv->equivalent_id); keys = g_hash_table_get_keys (priv->metadata); @@ -1158,6 +1205,7 @@ fu_device_finalize (GObject *object) g_hash_table_unref (priv->metadata); g_ptr_array_unref (priv->children); g_ptr_array_unref (priv->parent_guids); + g_free (priv->alternate_id); g_free (priv->equivalent_id); G_OBJECT_CLASS (fu_device_parent_class)->finalize (object); diff --git a/src/fu-device.h b/src/fu-device.h index 183090be8..667446489 100644 --- a/src/fu-device.h +++ b/src/fu-device.h @@ -98,6 +98,9 @@ FuDevice *fu_device_new (void); /* accessors */ gchar *fu_device_to_string (FuDevice *device); +const gchar *fu_device_get_alternate_id (FuDevice *device); +void fu_device_set_alternate_id (FuDevice *device, + const gchar *alternate_id); const gchar *fu_device_get_equivalent_id (FuDevice *device); void fu_device_set_equivalent_id (FuDevice *device, const gchar *equivalent_id); @@ -105,8 +108,6 @@ void fu_device_add_guid (FuDevice *device, const gchar *guid); gchar *fu_device_get_guids_as_str (FuDevice *device); FuDevice *fu_device_get_alternate (FuDevice *device); -void fu_device_set_alternate (FuDevice *device, - FuDevice *alternate); FuDevice *fu_device_get_parent (FuDevice *device); GPtrArray *fu_device_get_children (FuDevice *device); void fu_device_add_child (FuDevice *device, diff --git a/src/fu-engine.c b/src/fu-engine.c index 0c2bdee37..805f58699 100644 --- a/src/fu-engine.c +++ b/src/fu-engine.c @@ -2891,6 +2891,16 @@ fu_engine_add_device (FuEngine *self, FuDevice *device) /* adopt any required children, which may or may not already exist */ fu_engine_adopt_children (self, device); + /* set any alternate objects on the device from the ID */ + if (fu_device_get_alternate_id (device) != NULL) { + FuDevice *device_alt; + device_alt = fu_device_list_get_by_id (self->device_list, + fu_device_get_alternate_id (device), + NULL); + if (device_alt != NULL) + fu_device_set_alternate (device, device_alt); + } + /* notify all plugins about this new device */ if (!fu_device_has_flag (device, FWUPD_DEVICE_FLAG_REGISTERED)) fu_engine_plugin_device_register (self, device);