From 97133bd0285788f9b203607e3eaadfabb3f6582f Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 19 Dec 2017 20:33:14 +0000 Subject: [PATCH] daemon: avoid using floating GVariant refs It makes it harder to keep track of ownership except in the trivial `_new ()` case for which they were designed (i.e. shoving straight into another glib GVariant function that takes ownership of it right away). Closes: #1160 Approved by: cgwalters --- src/daemon/rpmostree-package-variants.c | 25 ++++++++++++------------ src/daemon/rpmostreed-deployment-utils.c | 2 +- src/daemon/rpmostreed-os.c | 16 +++++++-------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/daemon/rpmostree-package-variants.c b/src/daemon/rpmostree-package-variants.c index 05207bf4..0a3535ff 100644 --- a/src/daemon/rpmostree-package-variants.c +++ b/src/daemon/rpmostree-package-variants.c @@ -28,11 +28,11 @@ * package_to_variant * @package: RpmOstreePackage * - * Returns: A GVariant of (sss) where values are + * Returns: A floating GVariant of (sss) where values are * (package name, evr, arch) */ static GVariant * -package_to_variant (RpmOstreePackage *package) +package_variant_new (RpmOstreePackage *package) { return g_variant_new ("(sss)", rpm_ostree_package_get_name (package), @@ -54,20 +54,20 @@ build_diff_variant (const gchar *name, if (old_package) { g_variant_builder_add (&options_builder, "{sv}", "PreviousPackage", - package_to_variant (old_package)); + package_variant_new (old_package)); } if (new_package) { g_variant_builder_add (&options_builder, "{sv}", "NewPackage", - package_to_variant (new_package)); + package_variant_new (new_package)); } g_variant_builder_init (&builder, G_VARIANT_TYPE_TUPLE); g_variant_builder_add (&builder, "s", name); g_variant_builder_add (&builder, "u", type); g_variant_builder_add_value (&builder, g_variant_builder_end (&options_builder)); - return g_variant_builder_end (&builder); + return g_variant_ref_sink (g_variant_builder_end (&builder)); } @@ -117,8 +117,8 @@ rpm_ostree_db_diff_variant_compare_by_type (const void *v1, * @repo: A OstreeRepo * @old_ref: old ref to use * @new_ref: New ref to use - * @out_variant: floating GVariant that represents the differences - * between the rpm databases on the given refs. + * @out_variant: GVariant that represents the differences between the rpm + * databases on the given refs. * GCancellable: *cancellable * GError: **error * @@ -144,7 +144,8 @@ rpm_ostree_db_diff_variant (OstreeRepo *repo, g_assert_cmpuint (modified_old->len, ==, modified_new->len); - g_autoptr(GPtrArray) found = g_ptr_array_new (); + g_autoptr(GPtrArray) found = + g_ptr_array_new_with_free_func ((GDestroyNotify) g_variant_unref); for (guint i = 0; i < modified_old->len; i++) { @@ -182,12 +183,10 @@ rpm_ostree_db_diff_variant (OstreeRepo *repo, for (guint i = 0; i < found->len; i++) g_variant_builder_add_value (&builder, found->pdata[i]); - GVariant *variant = NULL; if (found->len > 0) - variant = g_variant_builder_end (&builder); + *out_variant = g_variant_builder_end (&builder); else - variant = g_variant_new ("a(sua{sv})", NULL); - - *out_variant = g_steal_pointer (&variant); + *out_variant = g_variant_new ("a(sua{sv})", NULL); + g_variant_ref_sink (*out_variant); return TRUE; } diff --git a/src/daemon/rpmostreed-deployment-utils.c b/src/daemon/rpmostreed-deployment-utils.c index 9b8d931a..2ce04b94 100644 --- a/src/daemon/rpmostreed-deployment-utils.c +++ b/src/daemon/rpmostreed-deployment-utils.c @@ -435,5 +435,5 @@ rpmostreed_commit_generate_cached_details_variant (OstreeDeployment *deployment, NULL, NULL, &dict, error)) return NULL; - return g_variant_dict_end (&dict); + return g_variant_ref_sink (g_variant_dict_end (&dict)); } diff --git a/src/daemon/rpmostreed-os.c b/src/daemon/rpmostreed-os.c index ca447de0..27be7bdd 100644 --- a/src/daemon/rpmostreed-os.c +++ b/src/daemon/rpmostreed-os.c @@ -314,7 +314,7 @@ os_handle_get_deployments_rpm_diff (RPMOSTreeOS *interface, glnx_unref_object OstreeDeployment *deployment1 = NULL; OstreeSysroot *ot_sysroot = NULL; OstreeRepo *ot_repo = NULL; - GVariant *value = NULL; /* freed when invoked */ + g_autoptr(GVariant) value = NULL; GError *local_error = NULL; const gchar *ref0; const gchar *ref1; @@ -376,8 +376,8 @@ os_handle_get_cached_update_rpm_diff (RPMOSTreeOS *interface, OstreeRepo *ot_repo = NULL; glnx_unref_object OstreeDeployment *base_deployment = NULL; GCancellable *cancellable = NULL; - GVariant *value = NULL; /* freed when invoked */ - GVariant *details = NULL; /* freed when invoked */ + g_autoptr(GVariant) value = NULL; + g_autoptr(GVariant) details = NULL; GError *local_error = NULL; global_sysroot = rpmostreed_sysroot_get (); @@ -1369,8 +1369,8 @@ os_handle_get_cached_rebase_rpm_diff (RPMOSTreeOS *interface, g_autoptr(RpmOstreeOrigin) origin = NULL; g_autofree gchar *comp_ref = NULL; GError *local_error = NULL; - GVariant *value = NULL; /* freed when invoked */ - GVariant *details = NULL; /* freed when invoked */ + g_autoptr(GVariant) value = NULL; + g_autoptr(GVariant) details = NULL; /* TODO: Totally ignoring packages for now */ @@ -1492,8 +1492,8 @@ os_handle_get_cached_deploy_rpm_diff (RPMOSTreeOS *interface, g_autofree char *checksum = NULL; g_autofree char *version = NULL; g_autoptr(GCancellable) cancellable = NULL; - GVariant *value = NULL; - GVariant *details = NULL; + g_autoptr(GVariant) value = NULL; + g_autoptr(GVariant) details = NULL; GError *local_error = NULL; GError **error = &local_error; @@ -1629,7 +1629,7 @@ rpmostreed_os_load_internals (RpmostreedOS *self, GError **error) GVariant *booted_variant = NULL; GVariant *default_variant = NULL; GVariant *rollback_variant = NULL; - GVariant *cached_update = NULL; + g_autoptr(GVariant) cached_update = NULL; gboolean has_cached_updates = FALSE; name = rpmostree_os_get_name (RPMOSTREE_OS (self));