From fa661c595dbece9a67342e9de69d17d0f208ce21 Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Wed, 3 Aug 2022 22:13:28 -0500 Subject: [PATCH] trivial: map common positive or negative keys to possible enumeration values Dell and Lenovo use Enable or Enabled and Disable or Disabled which is confusing to an end user. Set up some heuristics to map positive values and negative values when passed into the client. --- libfwupd/fwupd-bios-attr.c | 94 ++++++++++++++++++++++++++++++++++++++ libfwupd/fwupd-bios-attr.h | 2 + libfwupd/fwupd.map | 1 + src/fu-engine.c | 32 ++++++------- src/fu-self-test.c | 6 ++- src/fu-tool.c | 8 ++-- src/fu-util.c | 8 ++-- 7 files changed, 128 insertions(+), 23 deletions(-) diff --git a/libfwupd/fwupd-bios-attr.c b/libfwupd/fwupd-bios-attr.c index 711c804a5..5a204e146 100644 --- a/libfwupd/fwupd-bios-attr.c +++ b/libfwupd/fwupd-bios-attr.c @@ -326,6 +326,100 @@ fwupd_bios_attr_set_description(FwupdBiosAttr *self, const gchar *description) priv->description = g_strdup(description); } +/* determine if key is supposed to be positive */ +static gboolean +fu_bios_attr_key_is_positive(const gchar *key) +{ + if (g_strrstr(key, "enable")) + return TRUE; + if (g_strcmp0(key, "true") == 0) + return TRUE; + if (g_strcmp0(key, "1") == 0) + return TRUE; + if (g_strcmp0(key, "on") == 0) + return TRUE; + return FALSE; +} + +/* determine if key is supposed to be negative */ +static gboolean +fu_bios_attr_key_is_negative(const gchar *key) +{ + if (g_strrstr(key, "disable")) + return TRUE; + if (g_strcmp0(key, "false") == 0) + return TRUE; + if (g_strcmp0(key, "0") == 0) + return TRUE; + if (g_strcmp0(key, "off") == 0) + return TRUE; + return FALSE; +} + +/** + * fwupd_bios_attr_map_possible_value: + * @self: a #FwupdBiosAttr + * @key: the string to try to map + * @error: (nullable): optional return location for an error + * + * Attempts to map a user provided string into strings that a #FwupdBiosAttr can + * support. The following heuristics are used: + * - Ignore case sensitivity + * - Map obviously "positive" phrases into a value that turns on the #FwupdBiosAttr + * - Map obviously "negative" phrases into a value that turns off the #FwupdBiosAttr + * + * Returns: (transfer none): the possible value that maps or NULL if none if found + * + * Since: 1.8.4 + **/ +const gchar * +fwupd_bios_attr_map_possible_value(FwupdBiosAttr *self, const gchar *key, GError **error) +{ + FwupdBiosAttrPrivate *priv = GET_PRIVATE(self); + gboolean positive_key = FALSE; + gboolean negative_key = FALSE; + g_autofree gchar *result = NULL; + g_autofree gchar *lower_key = NULL; + + g_return_val_if_fail(FWUPD_IS_BIOS_ATTR(self), NULL); + g_return_val_if_fail(priv->kind == FWUPD_BIOS_ATTR_KIND_ENUMERATION, NULL); + + if (priv->possible_values->len == 0) { + g_set_error(error, + FWUPD_ERROR, + FWUPD_ERROR_NOT_SUPPORTED, + "%s doesn't contain any possible values", + priv->name); + return NULL; + } + + lower_key = g_utf8_strdown(key, -1); + positive_key = fu_bios_attr_key_is_positive(lower_key); + negative_key = fu_bios_attr_key_is_negative(lower_key); + for (guint i = 0; i < priv->possible_values->len; i++) { + const gchar *possible = g_ptr_array_index(priv->possible_values, i); + g_autofree gchar *lower_possible = g_utf8_strdown(possible, -1); + gboolean positive_possible; + gboolean negative_possible; + + /* perfect match */ + if (g_strcmp0(lower_possible, lower_key) == 0) + return possible; + /* fuzzy match */ + positive_possible = fu_bios_attr_key_is_positive(lower_possible); + negative_possible = fu_bios_attr_key_is_negative(lower_possible); + if ((positive_possible && positive_key) || (negative_possible && negative_key)) + return possible; + } + g_set_error(error, + FWUPD_ERROR, + FWUPD_ERROR_NOT_SUPPORTED, + "%s doesn't map to any possible values for %s", + key, + priv->name); + return NULL; +} + /** * fwupd_bios_attr_has_possible_value: * @self: a #FwupdBiosAttr diff --git a/libfwupd/fwupd-bios-attr.h b/libfwupd/fwupd-bios-attr.h index 69d59d6bc..107d8762b 100644 --- a/libfwupd/fwupd-bios-attr.h +++ b/libfwupd/fwupd-bios-attr.h @@ -91,6 +91,8 @@ const gchar * fwupd_bios_attr_get_path(FwupdBiosAttr *self); const gchar * fwupd_bios_attr_get_description(FwupdBiosAttr *self); +const gchar * +fwupd_bios_attr_map_possible_value(FwupdBiosAttr *self, const gchar *key, GError **error); gboolean fwupd_bios_attr_has_possible_value(FwupdBiosAttr *self, const gchar *val); void diff --git a/libfwupd/fwupd.map b/libfwupd/fwupd.map index 313d2baf9..7b9f42120 100644 --- a/libfwupd/fwupd.map +++ b/libfwupd/fwupd.map @@ -820,6 +820,7 @@ LIBFWUPD_1.8.4 { fwupd_bios_attr_get_type; fwupd_bios_attr_get_upper_bound; fwupd_bios_attr_has_possible_value; + fwupd_bios_attr_map_possible_value; fwupd_bios_attr_new; fwupd_bios_attr_set_current_value; fwupd_bios_attr_set_description; diff --git a/src/fu-engine.c b/src/fu-engine.c index 697bd8d17..3599d9a6e 100644 --- a/src/fu-engine.c +++ b/src/fu-engine.c @@ -767,10 +767,13 @@ fu_engine_update_bios_attr(FwupdBiosAttr *attr, const gchar *value, GError **err * error messages */ static gboolean -fu_engine_validate_bios_attr_input(FwupdBiosAttr *attr, const gchar *value, GError **error) +fu_engine_validate_bios_attr_input(FwupdBiosAttr *attr, const gchar **value, GError **error) { guint64 tmp = 0; + g_return_val_if_fail(*value != NULL, FALSE); + g_return_val_if_fail(value != NULL, FALSE); + if (attr == NULL) { g_set_error_literal(error, FWUPD_ERROR, @@ -786,7 +789,7 @@ fu_engine_validate_bios_attr_input(FwupdBiosAttr *attr, const gchar *value, GErr fwupd_bios_attr_get_name(attr)); return FALSE; } else if (fwupd_bios_attr_get_kind(attr) == FWUPD_BIOS_ATTR_KIND_INTEGER) { - if (!fu_strtoull(value, &tmp, 0, G_MAXUINT64, error)) + if (!fu_strtoull(*value, &tmp, 0, G_MAXUINT64, error)) return FALSE; if (tmp < fwupd_bios_attr_get_lower_bound(attr)) { g_set_error(error, @@ -794,7 +797,7 @@ fu_engine_validate_bios_attr_input(FwupdBiosAttr *attr, const gchar *value, GErr FWUPD_ERROR_NOT_SUPPORTED, "%s is too small (%" G_GUINT64_FORMAT "); expected at least %" G_GUINT64_FORMAT, - value, + *value, tmp, fwupd_bios_attr_get_lower_bound(attr)); return FALSE; @@ -805,20 +808,20 @@ fu_engine_validate_bios_attr_input(FwupdBiosAttr *attr, const gchar *value, GErr FWUPD_ERROR_NOT_SUPPORTED, "%s is too big (%" G_GUINT64_FORMAT "); expected no more than %" G_GUINT64_FORMAT, - value, + *value, tmp, fwupd_bios_attr_get_upper_bound(attr)); return FALSE; } } else if (fwupd_bios_attr_get_kind(attr) == FWUPD_BIOS_ATTR_KIND_STRING) { - tmp = strlen(value); + tmp = strlen(*value); if (tmp < fwupd_bios_attr_get_lower_bound(attr)) { g_set_error(error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED, "%s is too short (%" G_GUINT64_FORMAT "); expected at least %" G_GUINT64_FORMAT, - value, + *value, tmp, fwupd_bios_attr_get_lower_bound(attr)); return FALSE; @@ -829,20 +832,16 @@ fu_engine_validate_bios_attr_input(FwupdBiosAttr *attr, const gchar *value, GErr FWUPD_ERROR_NOT_SUPPORTED, "%s is too long (%" G_GUINT64_FORMAT "); expected no more than %" G_GUINT64_FORMAT, - value, + *value, tmp, fwupd_bios_attr_get_upper_bound(attr)); return FALSE; } } else if (fwupd_bios_attr_get_kind(attr) == FWUPD_BIOS_ATTR_KIND_ENUMERATION) { - if (!fwupd_bios_attr_has_possible_value(attr, value)) { - g_set_error(error, - FWUPD_ERROR, - FWUPD_ERROR_NOT_SUPPORTED, - "%s is not a supported possible attribute value", - value); + const gchar *result = fwupd_bios_attr_map_possible_value(attr, *value, error); + if (result == NULL) return FALSE; - } + *value = result; } else { g_set_error_literal(error, FWUPD_ERROR, @@ -869,11 +868,12 @@ fu_engine_modify_bios_attr(FuEngine *self, const gchar *key, const gchar *value, { FwupdBiosAttr *attr = fu_context_get_bios_attr(self->ctx, key); FwupdBiosAttr *pending; + const gchar *tmp = value; - if (!fu_engine_validate_bios_attr_input(attr, value, error)) + if (!fu_engine_validate_bios_attr_input(attr, &tmp, error)) return FALSE; - if (!fu_engine_update_bios_attr(attr, value, error)) + if (!fu_engine_update_bios_attr(attr, tmp, error)) return FALSE; pending = fu_context_get_bios_attr(self->ctx, FWUPD_BIOS_ATTR_PENDING_REBOOT); diff --git a/src/fu-self-test.c b/src/fu-self-test.c index 7dfb10dac..8d4e452d1 100644 --- a/src/fu-self-test.c +++ b/src/fu-self-test.c @@ -4512,7 +4512,11 @@ fu_engine_modify_bios_attrs_func(void) g_assert_no_error(error); g_assert_true(ret); - ret = fu_engine_modify_bios_attr(engine, "Absolute", "Disable", &error); + ret = fu_engine_modify_bios_attr(engine, "Absolute", "off", &error); + g_assert_no_error(error); + g_assert_true(ret); + + ret = fu_engine_modify_bios_attr(engine, "Absolute", "FOO", &error); g_assert_false(ret); g_assert_error(error, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED); g_clear_error(&error); diff --git a/src/fu-tool.c b/src/fu-tool.c index b2811f794..6134afe94 100644 --- a/src/fu-tool.c +++ b/src/fu-tool.c @@ -3236,13 +3236,15 @@ fu_util_set_bios_attr(FuUtilPrivate *priv, gchar **values, GError **error) error)) return FALSE; - if (!fu_engine_modify_bios_attr(priv->engine, values[0], values[1], error)) + if (!fu_engine_modify_bios_attr(priv->engine, values[0], values[1], error)) { + g_prefix_error(error, "failed to set '%s' using '%s': ", values[0], values[1]); return FALSE; + } if (!priv->as_json) { - g_autofree gchar *msg = - g_strdup_printf(_("Set BIOS attribute '%s' to '%s'."), values[0], values[1]); /* TRANSLATORS: Configured a BIOS setting to a value */ + g_autofree gchar *msg = + g_strdup_printf(_("Set BIOS attribute '%s' using '%s'."), values[0], values[1]); g_print("\n%s\n", msg); } priv->completion_flags |= FWUPD_DEVICE_FLAG_NEEDS_REBOOT; diff --git a/src/fu-util.c b/src/fu-util.c index e0dc9596a..0aa1fd075 100644 --- a/src/fu-util.c +++ b/src/fu-util.c @@ -3809,13 +3809,15 @@ fu_util_set_bios_attr(FuUtilPrivate *priv, gchar **values, GError **error) values[0], values[1], priv->cancellable, - error)) + error)) { + g_prefix_error(error, "failed to set '%s' using '%s': ", values[0], values[1]); return FALSE; + } if (!priv->as_json) { - g_autofree gchar *msg = - g_strdup_printf(_("Set BIOS attribute '%s' to '%s'."), values[0], values[1]); /* TRANSLATORS: Configured a BIOS setting to a value */ + g_autofree gchar *msg = + g_strdup_printf(_("Set BIOS attribute '%s' using '%s'."), values[0], values[1]); g_print("\n%s\n", msg); } priv->completion_flags |= FWUPD_DEVICE_FLAG_NEEDS_REBOOT;