Skip to content

Commit

Permalink
Do not export useless device attributes to the client
Browse files Browse the repository at this point in the history
That giant uint64_t isn't looking so big now, and we'll want to add even more
to it in the future. Split out some private flags that are never useful to the
client, although the #defines will have to remain until we break API again.
  • Loading branch information
hughsie committed Jan 6, 2021
1 parent e002f21 commit cf10029
Show file tree
Hide file tree
Showing 19 changed files with 237 additions and 82 deletions.
12 changes: 12 additions & 0 deletions contrib/ci/fedora.sh
Expand Up @@ -2,6 +2,18 @@
set -e
set -x

# these are deprecated in favor of INTERNAL flags
deprecated="FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS
FWUPD_DEVICE_FLAG_ONLY_SUPPORTED
FWUPD_DEVICE_FLAG_MD_SET_NAME
FWUPD_DEVICE_FLAG_MD_SET_VERFMT
FWUPD_DEVICE_FLAG_MD_SET_ICON"
for val in $deprecated; do
if grep -- $val plugins/*/*.c ; then
exit 1
fi
done

#generate a tarball
git config tar.tar.xz.command "xz -c"
mkdir -p build && pushd build
Expand Down
1 change: 1 addition & 0 deletions libfwupd/README.md
Expand Up @@ -4,6 +4,7 @@ Planned API/ABI changes for next release
* Typedef `FwupdFeatureFlags` to `guint64` so it's the same size on all platforms
* Remove the `soup-session` fallback property in `FwupdClient`.
* Remove fwupd_device_set_vendor_id() and fwupd_device_get_vendor_id()
* Remove the deprecated flags like `FWUPD_DEVICE_FLAG_MD_SET_ICON`

Migration from Version 0.9.x
============================
Expand Down
4 changes: 0 additions & 4 deletions libfwupd/fwupd-enums.c
Expand Up @@ -203,8 +203,6 @@ fwupd_device_flag_to_string (FwupdDeviceFlags device_flag)
return "has-multiple-branches";
if (device_flag == FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL)
return "backup-before-install";
if (device_flag == FWUPD_DEVICE_FLAG_RETRY_OPEN)
return "retry-open";
if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN)
return "unknown";
return NULL;
Expand Down Expand Up @@ -309,8 +307,6 @@ fwupd_device_flag_from_string (const gchar *device_flag)
return FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES;
if (g_strcmp0 (device_flag, "backup-before-install") == 0)
return FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL;
if (g_strcmp0 (device_flag, "retry-open") == 0)
return FWUPD_DEVICE_FLAG_RETRY_OPEN;
return FWUPD_DEVICE_FLAG_UNKNOWN;
}

Expand Down
30 changes: 14 additions & 16 deletions libfwupd/fwupd-enums.h
Expand Up @@ -106,11 +106,11 @@ typedef enum {
* @FWUPD_DEVICE_FLAG_TRUSTED: Extra metadata can be exposed about this device
* @FWUPD_DEVICE_FLAG_NEEDS_SHUTDOWN: Requires system shutdown to apply firmware
* @FWUPD_DEVICE_FLAG_ANOTHER_WRITE_REQUIRED: Requires the update to be retried with a new plugin
* @FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS: Do not add instance IDs from the device baseclass
* @FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS: Deprecated, no not use
* @FWUPD_DEVICE_FLAG_NEEDS_ACTIVATION: Device update needs to be separately activated
* @FWUPD_DEVICE_FLAG_HISTORICAL Device is for historical data only
* @FWUPD_DEVICE_FLAG_ENSURE_SEMVER: Ensure the version is a valid semantic version, e.g. numbers separated with dots
* @FWUPD_DEVICE_FLAG_ONLY_SUPPORTED: Only devices supported in the metadata will be opened
* @FWUPD_DEVICE_FLAG_ENSURE_SEMVER: Deprecated, no not use
* @FWUPD_DEVICE_FLAG_ONLY_SUPPORTED: Deprecated, no not use
* @FWUPD_DEVICE_FLAG_WILL_DISAPPEAR: Device will disappear after update and can't be verified
* @FWUPD_DEVICE_FLAG_CAN_VERIFY: Device checksums can be compared against metadata
* @FWUPD_DEVICE_FLAG_CAN_VERIFY_IMAGE: Image can be dumped from device for verification
Expand All @@ -119,17 +119,16 @@ typedef enum {
* @FWUPD_DEVICE_FLAG_USABLE_DURING_UPDATE: Device remains usable while fwupd flashes or schedules the update
* @FWUPD_DEVICE_FLAG_VERSION_CHECK_REQUIRED: All firmware updates for this device require a firmware version check
* @FWUPD_DEVICE_FLAG_INSTALL_ALL_RELEASES: Install each intermediate release rather than jumping direct to newest
* @FWUPD_DEVICE_FLAG_MD_SET_NAME: Set the device name from the metadata <name> if available
* @FWUPD_DEVICE_FLAG_MD_SET_NAME_CATEGORY: Set the device name from the metadata <category> if available
* @FWUPD_DEVICE_FLAG_MD_SET_VERFMT: Set the device version format from the metadata if available
* @FWUPD_DEVICE_FLAG_MD_SET_NAME: Deprecated, no not use
* @FWUPD_DEVICE_FLAG_MD_SET_NAME_CATEGORY: Deprecated, no not use
* @FWUPD_DEVICE_FLAG_MD_SET_VERFMT: Deprecated, no not use
* @FWUPD_DEVICE_FLAG_ADD_COUNTERPART_GUIDS: Add counterpart GUIDs from an alternate mode like bootloader
* @FWUPD_DEVICE_FLAG_NO_GUID_MATCHING: Force an explicit ID match when adding devices to the device list
* @FWUPD_DEVICE_FLAG_UPDATABLE_HIDDEN: Device is updatable but should not be called by the client
* @FWUPD_DEVICE_FLAG_SKIPS_RESTART: Device relies upon activation or power cycle to load firmware
* @FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES: Device supports switching to a different stream of firmware
* @FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL: Device firmware should be saved before installing firmware
* @FWUPD_DEVICE_FLAG_MD_SET_ICON: Set the device icon from the metadata if available
* @FWUPD_DEVICE_FLAG_RETRY_OPEN: Retry the device open up to 5 times if it fails
* @FWUPD_DEVICE_FLAG_MD_SET_ICON: Deprecated, no not use
*
* The device flags.
**/
Expand All @@ -153,11 +152,11 @@ typedef enum {
#define FWUPD_DEVICE_FLAG_TRUSTED (1u << 16) /* Since: 1.1.2 */
#define FWUPD_DEVICE_FLAG_NEEDS_SHUTDOWN (1u << 17) /* Since: 1.2.4 */
#define FWUPD_DEVICE_FLAG_ANOTHER_WRITE_REQUIRED (1u << 18) /* Since: 1.2.5 */
#define FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS (1u << 19) /* Since: 1.2.5 */
#define FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS (1u << 19) /* Since: 1.2.5; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_NEEDS_ACTIVATION (1u << 20) /* Since: 1.2.6 */
#define FWUPD_DEVICE_FLAG_ENSURE_SEMVER (1u << 21) /* Since: 1.2.9 */
#define FWUPD_DEVICE_FLAG_ENSURE_SEMVER (1u << 21) /* Since: 1.2.9; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_HISTORICAL (1u << 22) /* Since: 1.3.2 */
#define FWUPD_DEVICE_FLAG_ONLY_SUPPORTED (1u << 23) /* Since: 1.3.3 */
#define FWUPD_DEVICE_FLAG_ONLY_SUPPORTED (1u << 23) /* Since: 1.3.3; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_WILL_DISAPPEAR (1u << 24) /* Since: 1.3.3 */
#define FWUPD_DEVICE_FLAG_CAN_VERIFY (1u << 25) /* Since: 1.3.3 */
#define FWUPD_DEVICE_FLAG_CAN_VERIFY_IMAGE (1u << 26) /* Since: 1.3.3 */
Expand All @@ -166,17 +165,16 @@ typedef enum {
#define FWUPD_DEVICE_FLAG_USABLE_DURING_UPDATE (1u << 29) /* Since: 1.3.3 */
#define FWUPD_DEVICE_FLAG_VERSION_CHECK_REQUIRED (1u << 30) /* Since: 1.3.7 */
#define FWUPD_DEVICE_FLAG_INSTALL_ALL_RELEASES (1u << 31) /* Since: 1.3.7 */
#define FWUPD_DEVICE_FLAG_MD_SET_NAME (1llu << 32) /* Since: 1.4.0 */
#define FWUPD_DEVICE_FLAG_MD_SET_NAME_CATEGORY (1llu << 33) /* Since: 1.4.0 */
#define FWUPD_DEVICE_FLAG_MD_SET_VERFMT (1llu << 34) /* Since: 1.4.0 */
#define FWUPD_DEVICE_FLAG_MD_SET_NAME (1llu << 32) /* Since: 1.4.0; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_MD_SET_NAME_CATEGORY (1llu << 33) /* Since: 1.4.0; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_MD_SET_VERFMT (1llu << 34) /* Since: 1.4.0; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_ADD_COUNTERPART_GUIDS (1llu << 35) /* Since: 1.4.0 */
#define FWUPD_DEVICE_FLAG_NO_GUID_MATCHING (1llu << 36) /* Since: 1.4.1 */
#define FWUPD_DEVICE_FLAG_UPDATABLE_HIDDEN (1llu << 37) /* Since: 1.4.1 */
#define FWUPD_DEVICE_FLAG_SKIPS_RESTART (1llu << 38) /* Since: 1.5.0 */
#define FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES (1llu << 39) /* Since: 1.5.0 */
#define FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL (1llu << 40) /* Since: 1.5.0 */
#define FWUPD_DEVICE_FLAG_MD_SET_ICON (1llu << 41) /* Since: 1.5.2 */
#define FWUPD_DEVICE_FLAG_RETRY_OPEN (1llu << 42) /* Since: 1.5.5 */
#define FWUPD_DEVICE_FLAG_MD_SET_ICON (1llu << 41) /* Since: 1.5.2; Deprecated: 1.5.5 */
#define FWUPD_DEVICE_FLAG_UNKNOWN G_MAXUINT64 /* Since: 0.7.3 */
typedef guint64 FwupdDeviceFlags;

Expand Down
3 changes: 3 additions & 0 deletions libfwupdplugin/fu-device-private.h
Expand Up @@ -11,6 +11,9 @@

#define fu_device_set_plugin(d,v) fwupd_device_set_plugin(FWUPD_DEVICE(d),v)

const gchar *fu_device_internal_flag_to_string (FuDeviceInternalFlags flag);
FuDeviceInternalFlags fu_device_internal_flag_from_string (const gchar *flag);

GPtrArray *fu_device_get_parent_guids (FuDevice *self);
gboolean fu_device_has_parent_guid (FuDevice *self,
const gchar *guid);
Expand Down
151 changes: 142 additions & 9 deletions libfwupdplugin/fu-device.c
Expand Up @@ -62,6 +62,7 @@ typedef struct {
GPtrArray *possible_plugins;
GPtrArray *retry_recs; /* of FuDeviceRetryRecovery */
guint retry_delay;
FuDeviceInternalFlags internal_flags;
} FuDevicePrivate;

typedef struct {
Expand Down Expand Up @@ -138,6 +139,121 @@ fu_device_set_property (GObject *object, guint prop_id,
}
}

/**
* fu_device_internal_flag_to_string:
* @flag: A #FuDeviceInternalFlags, e.g. %FU_DEVICE_INTERNAL_FLAG_MD_SET_ICON
*
* Converts a #FuDeviceInternalFlags to a string.
*
* Return value: identifier string
*
* Since: 1.5.5
**/
const gchar *
fu_device_internal_flag_to_string (FuDeviceInternalFlags flag)
{
if (flag == FU_DEVICE_INTERNAL_FLAG_MD_SET_ICON)
return "md-set-icon";
if (flag == FU_DEVICE_INTERNAL_FLAG_MD_SET_NAME)
return "md-set-name";
if (flag == FU_DEVICE_INTERNAL_FLAG_MD_SET_NAME_CATEGORY)
return "md-set-name-category";
if (flag == FU_DEVICE_INTERNAL_FLAG_MD_SET_VERFMT)
return "md-set-verfmt";
if (flag == FU_DEVICE_INTERNAL_FLAG_ONLY_SUPPORTED)
return "only-supported";
if (flag == FU_DEVICE_INTERNAL_FLAG_NO_AUTO_INSTANCE_IDS)
return "no-auto-instance-ids";
if (flag == FU_DEVICE_INTERNAL_FLAG_ENSURE_SEMVER)
return "ensure-semver";
if (flag == FU_DEVICE_INTERNAL_FLAG_RETRY_OPEN)
return "retry-open";
return NULL;
}

/**
* fu_device_internal_flag_from_string:
* @flag: A string, e.g. `md-set-icon`
*
* Converts a string to a #FuDeviceInternalFlags.
*
* Return value: enumerated value
*
* Since: 1.5.5
**/
FuDeviceInternalFlags
fu_device_internal_flag_from_string (const gchar *flag)
{
if (g_strcmp0 (flag, "md-set-icon") == 0)
return FU_DEVICE_INTERNAL_FLAG_MD_SET_ICON;
if (g_strcmp0 (flag, "md-set-name") == 0)
return FU_DEVICE_INTERNAL_FLAG_MD_SET_NAME;
if (g_strcmp0 (flag, "md-set-name-category") == 0)
return FU_DEVICE_INTERNAL_FLAG_MD_SET_NAME_CATEGORY;
if (g_strcmp0 (flag, "md-set-verfmt") == 0)
return FU_DEVICE_INTERNAL_FLAG_MD_SET_VERFMT;
if (g_strcmp0 (flag, "only-supported") == 0)
return FU_DEVICE_INTERNAL_FLAG_ONLY_SUPPORTED;
if (g_strcmp0 (flag, "no-auto-instance-ids") == 0)
return FU_DEVICE_INTERNAL_FLAG_NO_AUTO_INSTANCE_IDS;
if (g_strcmp0 (flag, "ensure-semver") == 0)
return FU_DEVICE_INTERNAL_FLAG_ENSURE_SEMVER;
if (g_strcmp0 (flag, "retry-open") == 0)
return FU_DEVICE_INTERNAL_FLAG_RETRY_OPEN;
return FU_DEVICE_INTERNAL_FLAG_UNKNOWN;
}

/**
* fu_device_add_internal_flag:
* @self: A #FuDevice
* @flag: A #FuDeviceInternalFlags, e.g. %FU_DEVICE_INTERNAL_FLAG_MD_SET_ICON
*
* Adds a private flag that stays internal to the engine and is not leaked to the client.
*
* Since: 1.5.5
**/
void
fu_device_add_internal_flag (FuDevice *self, FuDeviceInternalFlags flag)
{
FuDevicePrivate *priv = GET_PRIVATE (self);
g_return_if_fail (FU_IS_DEVICE (self));
priv->internal_flags |= flag;
}

/**
* fu_device_remove_internal_flag:
* @self: A #FuDevice
* @flag: A #FuDeviceInternalFlags, e.g. %FU_DEVICE_INTERNAL_FLAG_MD_SET_ICON
*
* Removes a private flag that stays internal to the engine and is not leaked to the client.
*
* Since: 1.5.5
**/
void
fu_device_remove_internal_flag (FuDevice *self, FuDeviceInternalFlags flag)
{
FuDevicePrivate *priv = GET_PRIVATE (self);
g_return_if_fail (FU_IS_DEVICE (self));
priv->internal_flags &= ~flag;
}

/**
* fu_device_has_internal_flag:
* @self: A #FuDevice
* @flag: A #FuDeviceInternalFlags, e.g. %FU_DEVICE_INTERNAL_FLAG_MD_SET_ICON
*
* Tests for a private flag that stays internal to the engine and is not leaked to the client.
*
* Since: 1.5.5
**/
gboolean
fu_device_has_internal_flag (FuDevice *self, FuDeviceInternalFlags flag)
{
FuDevicePrivate *priv = GET_PRIVATE (self);
g_return_val_if_fail (FU_IS_DEVICE (self), FALSE);
return (priv->internal_flags & flag) > 0;
}

/**
* fu_device_get_possible_plugins:
* @self: A #FuDevice
Expand Down Expand Up @@ -1718,7 +1834,7 @@ fu_device_set_version (FuDevice *self, const gchar *version)
g_return_if_fail (FU_IS_DEVICE (self));

/* sanitize if required */
if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_ENSURE_SEMVER)) {
if (fu_device_has_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_ENSURE_SEMVER)) {
version_safe = fu_common_version_ensure_semver (version);
if (g_strcmp0 (version, version_safe) != 0)
g_debug ("converted '%s' to '%s'", version, version_safe);
Expand Down Expand Up @@ -1760,7 +1876,7 @@ fu_device_set_version_lowest (FuDevice *self, const gchar *version)
g_return_if_fail (FU_IS_DEVICE (self));

/* sanitize if required */
if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_ENSURE_SEMVER)) {
if (fu_device_has_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_ENSURE_SEMVER)) {
version_safe = fu_common_version_ensure_semver (version);
if (g_strcmp0 (version, version_safe) != 0)
g_debug ("converted '%s' to '%s'", version, version_safe);
Expand Down Expand Up @@ -1802,7 +1918,7 @@ fu_device_set_version_bootloader (FuDevice *self, const gchar *version)
g_return_if_fail (FU_IS_DEVICE (self));

/* sanitize if required */
if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_ENSURE_SEMVER)) {
if (fu_device_has_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_ENSURE_SEMVER)) {
version_safe = fu_common_version_ensure_semver (version);
if (g_strcmp0 (version, version_safe) != 0)
g_debug ("converted '%s' to '%s'", version, version_safe);
Expand Down Expand Up @@ -2063,21 +2179,26 @@ static void
fu_device_set_custom_flag (FuDevice *self, const gchar *hint)
{
FwupdDeviceFlags flag;
FuDeviceInternalFlags internal_flag;

/* is this a negated device flag */
if (g_str_has_prefix (hint, "~")) {
flag = fwupd_device_flag_from_string (hint + 1);
if (flag != FWUPD_DEVICE_FLAG_UNKNOWN)
fu_device_remove_flag (self, flag);
internal_flag = fu_device_internal_flag_from_string (hint + 1);
if (internal_flag != FU_DEVICE_INTERNAL_FLAG_UNKNOWN)
fu_device_remove_internal_flag (self, internal_flag);
return;
}

/* is this a known device flag */
flag = fwupd_device_flag_from_string (hint);
if (flag != FWUPD_DEVICE_FLAG_UNKNOWN) {
if (flag != FWUPD_DEVICE_FLAG_UNKNOWN)
fu_device_add_flag (self, flag);
return;
}
internal_flag = fu_device_internal_flag_from_string (hint);
if (internal_flag != FU_DEVICE_INTERNAL_FLAG_UNKNOWN)
fu_device_remove_internal_flag (self, internal_flag);
}

/**
Expand Down Expand Up @@ -2366,6 +2487,18 @@ fu_device_add_string (FuDevice *self, guint idt, GString *str)
const gchar *name = g_ptr_array_index (priv->possible_plugins, i);
fu_common_string_append_kv (str, idt + 1, "PossiblePlugin", name);
}
if (priv->internal_flags != FU_DEVICE_INTERNAL_FLAG_NONE) {
g_autoptr(GString) tmp2 = g_string_new ("");
for (guint i = 0; i < 64; i++) {
if ((priv->internal_flags & ((guint64) 1 << i)) == 0)
continue;
g_string_append_printf (tmp2, "%s|",
fu_device_internal_flag_to_string ((guint64) 1 << i));
}
if (tmp2->len > 0)
g_string_truncate (tmp2, tmp2->len - 1);
fu_common_string_append_kv (tmp2, idt + 1, "PrivateFlags", tmp2->str);
}

/* subclassed */
if (klass->to_string != NULL)
Expand Down Expand Up @@ -2845,7 +2978,7 @@ fu_device_open (FuDevice *self, GError **error)

/* subclassed */
if (klass->open != NULL) {
if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_RETRY_OPEN)) {
if (fu_device_has_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_RETRY_OPEN)) {
if (!fu_device_retry_full (self, fu_device_open_cb,
FU_DEVICE_RETRY_OPEN_COUNT,
FU_DEVICE_RETRY_OPEN_DELAY,
Expand Down Expand Up @@ -2994,7 +3127,7 @@ fu_device_rescan (FuDevice *self, GError **error)
* @self: A #FuDevice
*
* Converts all the Device Instance IDs added using fu_device_add_instance_id()
* into actual GUIDs, **unless** %FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS has
* into actual GUIDs, **unless** %FU_DEVICE_INTERNAL_FLAG_NO_AUTO_INSTANCE_IDS has
* been set.
*
* Plugins will only need to need to call this manually when adding child
Expand All @@ -3010,7 +3143,7 @@ fu_device_convert_instance_ids (FuDevice *self)
GPtrArray *instance_ids = fwupd_device_get_instance_ids (FWUPD_DEVICE (self));

/* OEM specific hardware */
if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_NO_AUTO_INSTANCE_IDS))
if (fu_device_has_internal_flag (self, FU_DEVICE_INTERNAL_FLAG_NO_AUTO_INSTANCE_IDS))
return;
for (guint i = 0; i < instance_ids->len; i++) {
const gchar *instance_id = g_ptr_array_index (instance_ids, i);
Expand Down

0 comments on commit cf10029

Please sign in to comment.