Attachment 129448 Details for Bug 99741 – 0001-Simplify-GVariant-reference-counting.patch

[patch] 0001-Simplify-GVariant-reference-counting.patch

0001-Simplify-GVariant-reference-counting.patch (text/plain), 21.24 KB, created by Miloslav Trmac on 2017-02-09 19:34:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Miloslav Trmac

Created: 2017-02-09 19:34:01 UTC

Size: 21.24 KB

patch

obsolete

>From 73bebd9d1a6695ff01d8da75534e6bdafb416830 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= <mitr@redhat.com>
>Date: Wed, 8 Feb 2017 22:26:37 +0100
>Subject: [PATCH 1/9] Simplify GVariant reference counting
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>For (non-public-API) *_to_gvariant, explicitly document that they return
>a floating value, and rely on it in callers to avoid a
>variable/sink/unref combo.
>
>This should not change behavior.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=99741
>
>Signed-off-by: Miloslav TrmaÄ <mitr@redhat.com>
>---
> src/polkit/polkitactiondescription.c               | 26 ++++++------
> src/polkit/polkitauthority.c                       | 47 ++++------------------
> src/polkit/polkitauthorizationresult.c             | 18 ++++-----
> src/polkit/polkitdetails.c                         |  5 +--
> src/polkit/polkitidentity.c                        |  5 +--
> src/polkit/polkitsubject.c                         |  5 +--
> src/polkit/polkittemporaryauthorization.c          | 21 ++++------
> src/polkitbackend/polkitbackendauthority.c         | 22 ++++------
> .../polkitbackendinteractiveauthority.c            | 14 ++-----
> 9 files changed, 49 insertions(+), 114 deletions(-)
>
>diff --git a/src/polkit/polkitactiondescription.c b/src/polkit/polkitactiondescription.c
>index 4bd9604..9c820e7 100644
>--- a/src/polkit/polkitactiondescription.c
>+++ b/src/polkit/polkitactiondescription.c
>@@ -352,10 +352,10 @@ polkit_action_description_new_for_gvariant (GVariant *value)
>   return action_description;
> }
> 
>+/* Note that this returns a floating value. */
> GVariant *
> polkit_action_description_to_gvariant (PolkitActionDescription *action_description)
> {
>-  GVariant *value;
>   GVariantBuilder builder;
>   GHashTableIter iter;
>   const gchar *a_key;
>@@ -368,17 +368,15 @@ polkit_action_description_to_gvariant (PolkitActionDescription *action_descripti
>     g_variant_builder_add (&builder, "{ss}", a_key, a_value);
> 
>   /* TODO: note 'foo ? : ""' is a gcc specific extension (it's a short-hand for 'foo ? foo : ""') */
>-  value = g_variant_new ("(ssssssuuua{ss})",
>-                         action_description->action_id ? : "",
>-                         action_description->description ? : "",
>-                         action_description->message ? : "",
>-                         action_description->vendor_name ? : "",
>-                         action_description->vendor_url ? : "",
>-                         action_description->icon_name ? : "",
>-                         action_description->implicit_any,
>-                         action_description->implicit_inactive,
>-                         action_description->implicit_active,
>-                         &builder);
>-
>-  return value;
>+  return g_variant_new ("(ssssssuuua{ss})",
>+			action_description->action_id ? : "",
>+			action_description->description ? : "",
>+			action_description->message ? : "",
>+			action_description->vendor_name ? : "",
>+			action_description->vendor_url ? : "",
>+			action_description->icon_name ? : "",
>+			action_description->implicit_any,
>+			action_description->implicit_inactive,
>+			action_description->implicit_active,
>+			&builder);
> }
>diff --git a/src/polkit/polkitauthority.c b/src/polkit/polkitauthority.c
>index 7c4db7b..b783c0e 100644
>--- a/src/polkit/polkitauthority.c
>+++ b/src/polkit/polkitauthority.c
>@@ -886,8 +886,6 @@ polkit_authority_check_authorization (PolkitAuthority               *authority,
>                                       GAsyncReadyCallback            callback,
>                                       gpointer                       user_data)
> {
>-  GVariant *subject_value;
>-  GVariant *details_value;
>   CheckAuthData *data;
> 
>   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
>@@ -896,11 +894,6 @@ polkit_authority_check_authorization (PolkitAuthority               *authority,
>   g_return_if_fail (details == NULL || POLKIT_IS_DETAILS (details));
>   g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
> 
>-  subject_value = polkit_subject_to_gvariant (subject);
>-  details_value = polkit_details_to_gvariant (details);
>-  g_variant_ref_sink (subject_value);
>-  g_variant_ref_sink (details_value);
>-
>   data = g_new0 (CheckAuthData, 1);
>   data->authority = g_object_ref (authority);
>   data->simple = g_simple_async_result_new (G_OBJECT (authority),
>@@ -915,9 +908,9 @@ polkit_authority_check_authorization (PolkitAuthority               *authority,
>   g_dbus_proxy_call (authority->proxy,
>                      "CheckAuthorization",
>                      g_variant_new ("(@(sa{sv})s@a{ss}us)",
>-                                    subject_value,
>+				    polkit_subject_to_gvariant (subject), /* A floating value */
>                                     action_id,
>-                                    details_value,
>+				    polkit_details_to_gvariant (details), /* A floating value */
>                                     flags,
>                                     data->cancellation_id != NULL ? data->cancellation_id : ""),
>                      G_DBUS_CALL_FLAGS_NONE,
>@@ -925,8 +918,6 @@ polkit_authority_check_authorization (PolkitAuthority               *authority,
>                      cancellable,
>                      (GAsyncReadyCallback) check_authorization_cb,
>                      data);
>-  g_variant_unref (subject_value);
>-  g_variant_unref (details_value);
> }
> 
> /**
>@@ -1058,20 +1049,16 @@ polkit_authority_register_authentication_agent (PolkitAuthority      *authority,
>                                                 GAsyncReadyCallback   callback,
>                                                 gpointer              user_data)
> {
>-  GVariant *subject_value;
>-
>   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
>   g_return_if_fail (POLKIT_IS_SUBJECT (subject));
>   g_return_if_fail (locale != NULL);
>   g_return_if_fail (g_variant_is_object_path (object_path));
>   g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
> 
>-  subject_value = polkit_subject_to_gvariant (subject);
>-  g_variant_ref_sink (subject_value);
>   g_dbus_proxy_call (authority->proxy,
>                      "RegisterAuthenticationAgent",
>                      g_variant_new ("(@(sa{sv})ss)",
>-                                    subject_value,
>+				    polkit_subject_to_gvariant (subject), /* A floating value */
>                                     locale,
>                                     object_path),
>                      G_DBUS_CALL_FLAGS_NONE,
>@@ -1082,7 +1069,6 @@ polkit_authority_register_authentication_agent (PolkitAuthority      *authority,
>                                                 callback,
>                                                 user_data,
>                                                 polkit_authority_register_authentication_agent));
>-  g_variant_unref (subject_value);
> }
> 
> /**
>@@ -1375,19 +1361,15 @@ polkit_authority_unregister_authentication_agent (PolkitAuthority      *authorit
>                                                   GAsyncReadyCallback   callback,
>                                                   gpointer              user_data)
> {
>-  GVariant *subject_value;
>-
>   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
>   g_return_if_fail (POLKIT_IS_SUBJECT (subject));
>   g_return_if_fail (g_variant_is_object_path (object_path));
>   g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
> 
>-  subject_value = polkit_subject_to_gvariant (subject);
>-  g_variant_ref_sink (subject_value);
>   g_dbus_proxy_call (authority->proxy,
>                      "UnregisterAuthenticationAgent",
>                      g_variant_new ("(@(sa{sv})s)",
>-                                    subject_value,
>+				    polkit_subject_to_gvariant (subject), /* A floating value */
>                                     object_path),
>                      G_DBUS_CALL_FLAGS_NONE,
>                      -1,
>@@ -1397,7 +1379,6 @@ polkit_authority_unregister_authentication_agent (PolkitAuthority      *authorit
>                                                 callback,
>                                                 user_data,
>                                                 polkit_authority_unregister_authentication_agent));
>-  g_variant_unref (subject_value);
> }
> 
> /**
>@@ -1511,7 +1492,6 @@ polkit_authority_authentication_agent_response (PolkitAuthority      *authority,
>                                                 GAsyncReadyCallback   callback,
>                                                 gpointer              user_data)
> {
>-  GVariant *identity_value;
>   /* Note that in reality, this API is only accessible to root, and
>    * only called from the setuid helper `polkit-agent-helper-1`.
>    *
>@@ -1526,14 +1506,12 @@ polkit_authority_authentication_agent_response (PolkitAuthority      *authority,
>   g_return_if_fail (POLKIT_IS_IDENTITY (identity));
>   g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
> 
>-  identity_value = polkit_identity_to_gvariant (identity);
>-  g_variant_ref_sink (identity_value);
>   g_dbus_proxy_call (authority->proxy,
>                      "AuthenticationAgentResponse2",
>                      g_variant_new ("(us@(sa{sv}))",
>                                     (guint32)uid,
>                                     cookie,
>-                                    identity_value),
>+				    polkit_identity_to_gvariant (identity)), /* A floating value */
>                      G_DBUS_CALL_FLAGS_NONE,
>                      -1,
>                      cancellable,
>@@ -1542,7 +1520,6 @@ polkit_authority_authentication_agent_response (PolkitAuthority      *authority,
>                                                 callback,
>                                                 user_data,
>                                                 polkit_authority_authentication_agent_response));
>-  g_variant_unref (identity_value);
> }
> 
> /**
>@@ -1653,18 +1630,14 @@ polkit_authority_enumerate_temporary_authorizations (PolkitAuthority     *author
>                                                      GAsyncReadyCallback  callback,
>                                                      gpointer             user_data)
> {
>-  GVariant *subject_value;
>-
>   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
>   g_return_if_fail (POLKIT_IS_SUBJECT (subject));
>   g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
> 
>-  subject_value = polkit_subject_to_gvariant (subject);
>-  g_variant_ref_sink (subject_value);
>   g_dbus_proxy_call (authority->proxy,
>                      "EnumerateTemporaryAuthorizations",
>                      g_variant_new ("(@(sa{sv}))",
>-                                    subject_value),
>+				    polkit_subject_to_gvariant (subject)), /* A floating value */
>                      G_DBUS_CALL_FLAGS_NONE,
>                      -1,
>                      cancellable,
>@@ -1673,7 +1646,6 @@ polkit_authority_enumerate_temporary_authorizations (PolkitAuthority     *author
>                                                 callback,
>                                                 user_data,
>                                                 polkit_authority_enumerate_temporary_authorizations));
>-  g_variant_unref (subject_value);
> }
> 
> /**
>@@ -1805,18 +1777,14 @@ polkit_authority_revoke_temporary_authorizations (PolkitAuthority     *authority
>                                                   GAsyncReadyCallback  callback,
>                                                   gpointer             user_data)
> {
>-  GVariant *subject_value;
>-
>   g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
>   g_return_if_fail (POLKIT_IS_SUBJECT (subject));
>   g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
> 
>-  subject_value = polkit_subject_to_gvariant (subject);
>-  g_variant_ref_sink (subject_value);
>   g_dbus_proxy_call (authority->proxy,
>                      "RevokeTemporaryAuthorizations",
>                      g_variant_new ("(@(sa{sv}))",
>-                                    subject_value),
>+				    polkit_subject_to_gvariant (subject)), /* A floating value */
>                      G_DBUS_CALL_FLAGS_NONE,
>                      -1,
>                      cancellable,
>@@ -1825,7 +1793,6 @@ polkit_authority_revoke_temporary_authorizations (PolkitAuthority     *authority
>                                                 callback,
>                                                 user_data,
>                                                 polkit_authority_revoke_temporary_authorizations));
>-  g_variant_unref (subject_value);
> }
> 
> /**
>diff --git a/src/polkit/polkitauthorizationresult.c b/src/polkit/polkitauthorizationresult.c
>index dd3d761..95855d4 100644
>--- a/src/polkit/polkitauthorizationresult.c
>+++ b/src/polkit/polkitauthorizationresult.c
>@@ -290,19 +290,15 @@ polkit_authorization_result_new_for_gvariant (GVariant *value)
>   return ret;
> }
> 
>+/* Note that this returns a floating value. */
> GVariant *
> polkit_authorization_result_to_gvariant (PolkitAuthorizationResult *authorization_result)
> {
>-  GVariant *ret;
>-  GVariant *details_gvariant;
>-
>-  details_gvariant = polkit_details_to_gvariant (polkit_authorization_result_get_details (authorization_result));
>-  g_variant_ref_sink (details_gvariant);
>-  ret = g_variant_new ("(bb@a{ss})",
>-                       polkit_authorization_result_get_is_authorized (authorization_result),
>-                       polkit_authorization_result_get_is_challenge (authorization_result),
>-                       details_gvariant);
>-  g_variant_unref (details_gvariant);
>+  PolkitDetails *details;
> 
>-  return ret;
>+  details = polkit_authorization_result_get_details (authorization_result);
>+  return g_variant_new ("(bb@a{ss})",
>+			polkit_authorization_result_get_is_authorized (authorization_result),
>+			polkit_authorization_result_get_is_challenge (authorization_result),
>+			polkit_details_to_gvariant (details)); /* A floating value */
> }
>diff --git a/src/polkit/polkitdetails.c b/src/polkit/polkitdetails.c
>index 07a6f63..b16aadc 100644
>--- a/src/polkit/polkitdetails.c
>+++ b/src/polkit/polkitdetails.c
>@@ -195,10 +195,10 @@ polkit_details_get_keys (PolkitDetails *details)
>   return ret;
> }
> 
>+/* Note that this returns a floating value. */
> GVariant *
> polkit_details_to_gvariant (PolkitDetails *details)
> {
>-  GVariant *ret;
>   GVariantBuilder builder;
> 
>   g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{ss}"));
>@@ -212,8 +212,7 @@ polkit_details_to_gvariant (PolkitDetails *details)
>       while (g_hash_table_iter_next (&hash_iter, (gpointer) &key, (gpointer) &value))
>         g_variant_builder_add (&builder, "{ss}", key, value);
>     }
>-  ret = g_variant_builder_end (&builder);
>-  return ret;
>+  return g_variant_builder_end (&builder);
> }
> 
> PolkitDetails *
>diff --git a/src/polkit/polkitidentity.c b/src/polkit/polkitidentity.c
>index 7813c2c..b0b46bf 100644
>--- a/src/polkit/polkitidentity.c
>+++ b/src/polkit/polkitidentity.c
>@@ -198,12 +198,12 @@ polkit_identity_from_string  (const gchar   *str,
>   return identity;
> }
> 
>+/* Note that this returns a floating value. */
> GVariant *
> polkit_identity_to_gvariant (PolkitIdentity *identity)
> {
>   GVariantBuilder builder;
>   GVariant *dict;
>-  GVariant *ret;
>   const gchar *kind;
> 
>   kind = "";
>@@ -233,8 +233,7 @@ polkit_identity_to_gvariant (PolkitIdentity *identity)
>     }
> 
>   dict = g_variant_builder_end (&builder);
>-  ret = g_variant_new ("(s@a{sv})", kind, dict);
>-  return ret;
>+  return g_variant_new ("(s@a{sv})", kind, dict);
> }
> 
> static GVariant *
>diff --git a/src/polkit/polkitsubject.c b/src/polkit/polkitsubject.c
>index df8e1aa..4a36941 100644
>--- a/src/polkit/polkitsubject.c
>+++ b/src/polkit/polkitsubject.c
>@@ -290,12 +290,12 @@ polkit_subject_from_string  (const gchar   *str,
>   return subject;
> }
> 
>+/* Note that this returns a floating value. */
> GVariant *
> polkit_subject_to_gvariant (PolkitSubject *subject)
> {
>   GVariantBuilder builder;
>   GVariant *dict;
>-  GVariant *ret;
>   const gchar *kind;
> 
>   kind = "";
>@@ -329,8 +329,7 @@ polkit_subject_to_gvariant (PolkitSubject *subject)
>     }
> 
>   dict = g_variant_builder_end (&builder);
>-  ret = g_variant_new ("(s@a{sv})", kind, dict);
>-  return ret;
>+  return g_variant_new ("(s@a{sv})", kind, dict);
> }
> 
> static GVariant *
>diff --git a/src/polkit/polkittemporaryauthorization.c b/src/polkit/polkittemporaryauthorization.c
>index b2c6003..ae71259 100644
>--- a/src/polkit/polkittemporaryauthorization.c
>+++ b/src/polkit/polkittemporaryauthorization.c
>@@ -212,22 +212,15 @@ polkit_temporary_authorization_new_for_gvariant (GVariant  *value,
>   return authorization;
> }
> 
>+/* Note that this returns a floating value. */
> GVariant *
> polkit_temporary_authorization_to_gvariant (PolkitTemporaryAuthorization *authorization)
> {
>-  GVariant *ret;
>-  GVariant *subject_gvariant;
>-
>-  subject_gvariant = polkit_subject_to_gvariant (authorization->subject);
>-  g_variant_ref_sink (subject_gvariant);
>-  ret = g_variant_new ("(ss@(sa{sv})tt)",
>-                       authorization->id,
>-                       authorization->action_id,
>-                       subject_gvariant,
>-                       authorization->time_obtained,
>-                       authorization->time_expires);
>-  g_variant_unref (subject_gvariant);
>-
>-  return ret;
>+  return g_variant_new ("(ss@(sa{sv})tt)",
>+			authorization->id,
>+			authorization->action_id,
>+			polkit_subject_to_gvariant (authorization->subject), /* A floating value */
>+			authorization->time_obtained,
>+			authorization->time_expires);
> }
> 
>diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
>index 64560e1..86dad02 100644
>--- a/src/polkitbackend/polkitbackendauthority.c
>+++ b/src/polkitbackend/polkitbackendauthority.c
>@@ -645,11 +645,8 @@ server_handle_enumerate_actions (Server                 *server,
>   for (l = actions; l != NULL; l = l->next)
>     {
>       PolkitActionDescription *ad = POLKIT_ACTION_DESCRIPTION (l->data);
>-      GVariant *value;
>-      value = polkit_action_description_to_gvariant (ad);
>-      g_variant_ref_sink (value);
>-      g_variant_builder_add_value (&builder, value);
>-      g_variant_unref (value);
>+      g_variant_builder_add_value (&builder,
>+				   polkit_action_description_to_gvariant (ad)); /* A floating value */
>     }
>   g_dbus_method_invocation_return_value (invocation, g_variant_new ("(a(ssssssuuua{ss}))", &builder));
> 
>@@ -709,11 +706,9 @@ check_auth_cb (GObject      *source_object,
>     }
>   else
>     {
>-      GVariant *value;
>-      value = polkit_authorization_result_to_gvariant (result);
>-      g_variant_ref_sink (value);
>-      g_dbus_method_invocation_return_value (data->invocation, g_variant_new ("(@(bba{ss}))", value));
>-      g_variant_unref (value);
>+      g_dbus_method_invocation_return_value (data->invocation,
>+					     g_variant_new ("(@(bba{ss}))",
>+							    polkit_authorization_result_to_gvariant (result))); /* A floating value */
>       g_object_unref (result);
>     }
> 
>@@ -1158,11 +1153,8 @@ server_handle_enumerate_temporary_authorizations (Server                 *server
>   for (l = authorizations; l != NULL; l = l->next)
>     {
>       PolkitTemporaryAuthorization *a = POLKIT_TEMPORARY_AUTHORIZATION (l->data);
>-      GVariant *value;
>-      value = polkit_temporary_authorization_to_gvariant (a);
>-      g_variant_ref_sink (value);
>-      g_variant_builder_add_value (&builder, value);
>-      g_variant_unref (value);
>+      g_variant_builder_add_value (&builder,
>+				   polkit_temporary_authorization_to_gvariant (a)); /* A floating value */
>     }
>   g_list_foreach (authorizations, (GFunc) g_object_unref, NULL);
>   g_list_free (authorizations);
>diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
>index ccfd608..bf0ee48 100644
>--- a/src/polkitbackend/polkitbackendinteractiveauthority.c
>+++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
>@@ -2299,7 +2299,6 @@ authentication_agent_initiate_challenge (AuthenticationAgent         *agent,
>   gchar *localized_message;
>   gchar *localized_icon_name;
>   PolkitDetails *localized_details;
>-  GVariant *details_gvariant;
>   GList *user_identities = NULL;
>   GVariantBuilder identities_builder;
>   GVariant *parameters;
>@@ -2397,28 +2396,21 @@ authentication_agent_initiate_challenge (AuthenticationAgent         *agent,
>   add_pid (localized_details, caller, "polkit.caller-pid");
>   add_pid (localized_details, subject, "polkit.subject-pid");
> 
>-  details_gvariant = polkit_details_to_gvariant (localized_details);
>-  g_variant_ref_sink (details_gvariant);
>-
>   g_variant_builder_init (&identities_builder, G_VARIANT_TYPE ("a(sa{sv})"));
>   for (l = user_identities; l != NULL; l = l->next)
>     {
>       PolkitIdentity *identity = POLKIT_IDENTITY (l->data);
>-      GVariant *value;
>-      value = polkit_identity_to_gvariant (identity);
>-      g_variant_ref_sink (value);
>-      g_variant_builder_add_value (&identities_builder, value);
>-      g_variant_unref (value);
>+      g_variant_builder_add_value (&identities_builder,
>+				   polkit_identity_to_gvariant (identity)); /* A floating value */
>     }
> 
>   parameters = g_variant_new ("(sss@a{ss}sa(sa{sv}))",
>                               action_id,
>                               localized_message,
>                               localized_icon_name,
>-                              details_gvariant,
>+			      polkit_details_to_gvariant (localized_details), /* A floating value */
>                               session->cookie,
>                               &identities_builder);
>-  g_variant_unref (details_gvariant);
> 
>   g_dbus_proxy_call (agent->proxy,
>                      "BeginAuthentication",
>-- 
>2.7.4
>

Actions: View | Splinter Review

Attachments on bug 99741: 129448 | 129449 | 129450 | 129451 | 129452 | 129453 | 129454 | 129455 | 129456 | 130032 | 132124

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.