From: Simon McVittie Date: Fri, 23 Aug 2013 10:09:30 +0000 (+0100) Subject: Revert "Remove refcounting from DBusAuth and DBusAuthorization" X-Git-Tag: dbus-1.8.2~202 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3006b952dbd939d01507d1397e8c4a0e03f2d7a6;p=platform%2Fupstream%2Fdbus.git Revert "Remove refcounting from DBusAuth and DBusAuthorization" This reverts commit 7f6d7229d8812d985d544cf5dd3636865c5abc81. --- diff --git a/dbus/dbus-auth-script.c b/dbus/dbus-auth-script.c index d195dde..107c92b 100644 --- a/dbus/dbus-auth-script.c +++ b/dbus/dbus-auth-script.c @@ -250,7 +250,6 @@ _dbus_auth_script_run (const DBusString *filename) dbus_bool_t retval; int line_no; DBusAuth *auth; - DBusAuthorization *authorization; DBusString from_auth; DBusAuthState state; DBusString context; @@ -258,7 +257,6 @@ _dbus_auth_script_run (const DBusString *filename) retval = FALSE; auth = NULL; - authorization = NULL; _dbus_string_init_const (&guid, "5fa01f4202cd837709a3274ca0df9d00"); _dbus_string_init_const (&context, "org_freedesktop_test"); @@ -376,16 +374,24 @@ _dbus_auth_script_run (const DBusString *filename) goto out; } + /* test ref/unref */ + _dbus_auth_ref (auth); + _dbus_auth_unref (auth); + creds = _dbus_credentials_new_from_current_process (); if (creds == NULL) { _dbus_warn ("no memory for credentials\n"); + _dbus_auth_unref (auth); + auth = NULL; goto out; } if (!_dbus_auth_set_credentials (auth, creds)) { _dbus_warn ("no memory for setting credentials\n"); + _dbus_auth_unref (auth); + auth = NULL; _dbus_credentials_unref (creds); goto out; } @@ -396,6 +402,7 @@ _dbus_auth_script_run (const DBusString *filename) _dbus_string_starts_with_c_str (&line, "SERVER_ANONYMOUS")) { DBusCredentials *creds; + DBusAuthorization *authorization; if (auth != NULL) { @@ -416,22 +423,32 @@ _dbus_auth_script_run (const DBusString *filename) _dbus_authorization_set_allow_anonymous (authorization, TRUE); auth = _dbus_auth_server_new (&guid, authorization); + /* DBusAuth owns it, or finalized on OOM */ + _dbus_authorization_unref (authorization); if (auth == NULL) { _dbus_warn ("no memory to create DBusAuth\n"); goto out; } + /* test ref/unref */ + _dbus_auth_ref (auth); + _dbus_auth_unref (auth); + creds = _dbus_credentials_new_from_current_process (); if (creds == NULL) { _dbus_warn ("no memory for credentials\n"); + _dbus_auth_unref (auth); + auth = NULL; goto out; } if (!_dbus_auth_set_credentials (auth, creds)) { _dbus_warn ("no memory for setting credentials\n"); + _dbus_auth_unref (auth); + auth = NULL; _dbus_credentials_unref (creds); goto out; } @@ -789,9 +806,7 @@ _dbus_auth_script_run (const DBusString *filename) out: if (auth) - _dbus_auth_free (auth); - if (authorization) - _dbus_authorization_free (authorization); + _dbus_auth_unref (auth); _dbus_string_free (&file); _dbus_string_free (&line); diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index 3da25ec..a218701 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -153,6 +153,7 @@ typedef struct */ struct DBusAuth { + int refcount; /**< reference count */ const char *side; /**< Client or server */ DBusString incoming; /**< Incoming data buffer */ @@ -345,6 +346,8 @@ _dbus_auth_new (int size) if (auth == NULL) return NULL; + auth->refcount = 1; + auth->keyring = NULL; auth->cookie_id = -1; @@ -2259,7 +2262,7 @@ process_command (DBusAuth *auth) */ DBusAuth* _dbus_auth_server_new (const DBusString *guid, - DBusAuthorization *authorization) + DBusAuthorization *authorization) { DBusAuth *auth; DBusAuthServer *server_auth; @@ -2287,7 +2290,7 @@ _dbus_auth_server_new (const DBusString *guid, server_auth = DBUS_AUTH_SERVER (auth); server_auth->guid = guid_copy; - server_auth->authorization = authorization; + server_auth->authorization = _dbus_authorization_ref (authorization); /* perhaps this should be per-mechanism with a lower * max @@ -2330,7 +2333,7 @@ _dbus_auth_client_new (void) * mechanism */ if (!send_auth (auth, &all_mechanisms[0])) { - _dbus_auth_free (auth); + _dbus_auth_unref (auth); return NULL; } @@ -2338,45 +2341,67 @@ _dbus_auth_client_new (void) } /** - * Free memory allocated for an auth object. + * Increments the refcount of an auth object. * * @param auth the auth conversation + * @returns the auth conversation */ -void -_dbus_auth_free (DBusAuth *auth) +DBusAuth * +_dbus_auth_ref (DBusAuth *auth) { _dbus_assert (auth != NULL); + + auth->refcount += 1; + + return auth; +} - shutdown_mech (auth); +/** + * Decrements the refcount of an auth object. + * + * @param auth the auth conversation + */ +void +_dbus_auth_unref (DBusAuth *auth) +{ + _dbus_assert (auth != NULL); + _dbus_assert (auth->refcount > 0); - if (DBUS_AUTH_IS_CLIENT (auth)) + auth->refcount -= 1; + if (auth->refcount == 0) { - _dbus_string_free (& DBUS_AUTH_CLIENT (auth)->guid_from_server); - _dbus_list_clear (& DBUS_AUTH_CLIENT (auth)->mechs_to_try); - } - else - { - _dbus_assert (DBUS_AUTH_IS_SERVER (auth)); + shutdown_mech (auth); - _dbus_string_free (& DBUS_AUTH_SERVER (auth)->guid); - } + if (DBUS_AUTH_IS_CLIENT (auth)) + { + _dbus_string_free (& DBUS_AUTH_CLIENT (auth)->guid_from_server); + _dbus_list_clear (& DBUS_AUTH_CLIENT (auth)->mechs_to_try); + } + else + { + _dbus_assert (DBUS_AUTH_IS_SERVER (auth)); - if (auth->keyring) - _dbus_keyring_unref (auth->keyring); + _dbus_string_free (& DBUS_AUTH_SERVER (auth)->guid); + _dbus_authorization_unref (DBUS_AUTH_SERVER (auth)->authorization); + } - _dbus_string_free (&auth->context); - _dbus_string_free (&auth->challenge); - _dbus_string_free (&auth->identity); - _dbus_string_free (&auth->incoming); - _dbus_string_free (&auth->outgoing); + if (auth->keyring) + _dbus_keyring_unref (auth->keyring); - dbus_free_string_array (auth->allowed_mechs); + _dbus_string_free (&auth->context); + _dbus_string_free (&auth->challenge); + _dbus_string_free (&auth->identity); + _dbus_string_free (&auth->incoming); + _dbus_string_free (&auth->outgoing); - _dbus_credentials_unref (auth->credentials); - _dbus_credentials_unref (auth->authenticated_identity); - _dbus_credentials_unref (auth->desired_identity); + dbus_free_string_array (auth->allowed_mechs); - dbus_free (auth); + _dbus_credentials_unref (auth->credentials); + _dbus_credentials_unref (auth->authenticated_identity); + _dbus_credentials_unref (auth->desired_identity); + + dbus_free (auth); + } } /** diff --git a/dbus/dbus-auth.h b/dbus/dbus-auth.h index 1cf0570..3f178a2 100644 --- a/dbus/dbus-auth.h +++ b/dbus/dbus-auth.h @@ -45,7 +45,8 @@ typedef enum DBusAuth* _dbus_auth_server_new (const DBusString *guid, DBusAuthorization *authorization); DBusAuth* _dbus_auth_client_new (void); -void _dbus_auth_free (DBusAuth *auth); +DBusAuth* _dbus_auth_ref (DBusAuth *auth); +void _dbus_auth_unref (DBusAuth *auth); dbus_bool_t _dbus_auth_set_mechanisms (DBusAuth *auth, const char **mechanisms); DBusAuthState _dbus_auth_do_work (DBusAuth *auth); diff --git a/dbus/dbus-authorization.c b/dbus/dbus-authorization.c index 3f0b966..05a3aa8 100644 --- a/dbus/dbus-authorization.c +++ b/dbus/dbus-authorization.c @@ -5,6 +5,8 @@ #include "dbus-connection-internal.h" struct DBusAuthorization { + int refcount; + DBusConnection *connection; /* Authorization functions, used as callback by SASL (implemented by @@ -24,31 +26,58 @@ struct DBusAuthorization { DBusAuthorization * _dbus_authorization_new (void) { - /* it returns the allocated memory or NULL in case of OOM */ - return dbus_malloc0 (sizeof (DBusAuthorization)); + DBusAuthorization *ret; + + ret = dbus_malloc0 (sizeof (DBusAuthorization)); + if (ret == NULL) + { + _dbus_verbose ("OOM\n"); + return NULL; /* OOM */ + } + + ret->refcount = 1; + + return ret; +} + +DBusAuthorization * +_dbus_authorization_ref (DBusAuthorization *self) +{ + _dbus_assert (self != NULL); + + self->refcount += 1; + + return self; } void -_dbus_authorization_free (DBusAuthorization *self) +_dbus_authorization_unref (DBusAuthorization *self) { _dbus_assert (self != NULL); + _dbus_assert (self->refcount > 0); - if (self->unix_data && self->unix_data_free) - { - _dbus_verbose ("freeing unix authorization callback data\n"); - (*self->unix_data_free) (self->unix_data); - self->unix_data = NULL; - } + self->refcount -= 1; - if (self->windows_data && self->windows_data_free) + if (self->refcount == 0) { - _dbus_verbose ("freeing windows authorization callback data\n"); - (*self->windows_data_free) (self->windows_data); - self->windows_data = NULL; + _dbus_verbose ("last reference, finalizing\n"); + + if (self->unix_data && self->unix_data_free) + { + _dbus_verbose ("freeing unix authorization callback data\n"); + (*self->unix_data_free) (self->unix_data); + self->unix_data = NULL; + } + + if (self->windows_data && self->windows_data_free) + { + _dbus_verbose ("freeing windows authorization callback data\n"); + (*self->windows_data_free) (self->windows_data); + self->windows_data = NULL; + } + + dbus_free (self); } - - _dbus_verbose ("freeing memory for %p\n", self); - dbus_free (self); } /* Called by transport's set_connection with the connection locked */ @@ -56,7 +85,6 @@ void _dbus_authorization_set_connection (DBusAuthorization *self, DBusConnection *connection) { - _dbus_assert (self != NULL); _dbus_assert (connection != NULL); _dbus_assert (self->connection == NULL); @@ -87,8 +115,6 @@ _dbus_authorization_set_unix_authorization_callback (DBusAuthorization void **old_data, DBusFreeFunction *old_free_data_function) { - _dbus_assert (self != NULL); - *old_data = self->unix_data; *old_free_data_function = self->unix_data_free; @@ -122,8 +148,6 @@ _dbus_authorization_set_windows_authorization_callback (DBusAuthorization void **old_data, DBusFreeFunction *old_free_data_function) { - _dbus_assert (self != NULL); - *old_data = self->windows_data; *old_free_data_function = self->windows_data_free; @@ -142,7 +166,6 @@ auth_via_unix_authorization_callback (DBusAuthorization *self, /* Dropping the lock here probably isn't that safe. */ - _dbus_assert (self != NULL); _dbus_assert (auth_identity != NULL); uid = _dbus_credentials_get_unix_uid (auth_identity); @@ -180,7 +203,6 @@ auth_via_windows_authorization_callback (DBusAuthorization *self, /* Dropping the lock here probably isn't that safe. */ - _dbus_assert (self != NULL); _dbus_assert (auth_identity != NULL); windows_sid = _dbus_strdup (_dbus_credentials_get_windows_sid (auth_identity)); @@ -221,7 +243,6 @@ auth_via_default_rules (DBusAuthorization *self, DBusCredentials *our_identity; dbus_bool_t allow; - _dbus_assert (self != NULL); _dbus_assert (auth_identity != NULL); /* By default, connection is allowed if the client is 1) root or 2) @@ -278,8 +299,6 @@ _dbus_authorization_do_authorization (DBusAuthorization *self, { dbus_bool_t allow; - _dbus_assert (self != NULL); - /* maybe-FIXME: at this point we *should* have a connection set unless we * are in some test case, but we assert its presence only in some if's * branches since default_rules does not need one and is used in a test case @@ -321,7 +340,5 @@ void _dbus_authorization_set_allow_anonymous (DBusAuthorization *self, dbus_bool_t value) { - _dbus_assert (self != NULL); - self->allow_anonymous = value != FALSE; } diff --git a/dbus/dbus-authorization.h b/dbus/dbus-authorization.h index eca81d8..8f7f1d4 100644 --- a/dbus/dbus-authorization.h +++ b/dbus/dbus-authorization.h @@ -9,7 +9,8 @@ typedef struct DBusAuthorization DBusAuthorization; DBusAuthorization *_dbus_authorization_new (void); void _dbus_authorization_set_connection (DBusAuthorization *self, DBusConnection *connection); -void _dbus_authorization_free (DBusAuthorization *self); +DBusAuthorization * _dbus_authorization_ref (DBusAuthorization *self); +void _dbus_authorization_unref (DBusAuthorization *self); void _dbus_authorization_set_unix_authorization_callback (DBusAuthorization *self, DBusAllowUnixUserFunction function, void *data, DBusFreeFunction free_data_function, void **old_data, diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index db16574..3a9cf84 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -134,7 +134,7 @@ _dbus_transport_init_base (DBusTransport *transport, if (auth == NULL) { if (authorization != NULL) - _dbus_authorization_free (authorization); + _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -142,9 +142,9 @@ _dbus_transport_init_base (DBusTransport *transport, counter = _dbus_counter_new (); if (counter == NULL) { - _dbus_auth_free (auth); + _dbus_auth_unref (auth); if (authorization != NULL) - _dbus_authorization_free (authorization); + _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -153,9 +153,9 @@ _dbus_transport_init_base (DBusTransport *transport, if (creds == NULL) { _dbus_counter_unref (counter); - _dbus_auth_free (auth); + _dbus_auth_unref (auth); if (authorization != NULL) - _dbus_authorization_free (authorization); + _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -173,9 +173,9 @@ _dbus_transport_init_base (DBusTransport *transport, { _dbus_credentials_unref (creds); _dbus_counter_unref (counter); - _dbus_auth_free (auth); + _dbus_auth_unref (auth); if (authorization != NULL) - _dbus_authorization_free (authorization); + _dbus_authorization_unref (authorization); _dbus_message_loader_unref (loader); return FALSE; } @@ -237,9 +237,9 @@ _dbus_transport_finalize_base (DBusTransport *transport) _dbus_transport_disconnect (transport); _dbus_message_loader_unref (transport->loader); - _dbus_auth_free (transport->auth); + _dbus_auth_unref (transport->auth); if (transport->authorization) - _dbus_authorization_free (transport->authorization); + _dbus_authorization_unref (transport->authorization); _dbus_counter_set_notify (transport->live_messages, 0, 0, NULL, NULL); _dbus_counter_unref (transport->live_messages);