From 55a337fa62b03730ed049a70e4237ffb08b6fbfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= 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č --- 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..ed0655e 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..15a81ac 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..877a9a6 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..5e07678 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..27d8e51 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..7511381 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.5