Revert "Remove refcounting from DBusAuth and DBusAuthorization"
authorSimon McVittie <simon.mcvittie@collabora.co.uk>
Fri, 23 Aug 2013 10:09:30 +0000 (11:09 +0100)
committerSimon McVittie <simon.mcvittie@collabora.co.uk>
Fri, 23 Aug 2013 10:10:40 +0000 (11:10 +0100)
This reverts commit 7f6d7229d8812d985d544cf5dd3636865c5abc81.

dbus/dbus-auth-script.c
dbus/dbus-auth.c
dbus/dbus-auth.h
dbus/dbus-authorization.c
dbus/dbus-authorization.h
dbus/dbus-transport.c

index d195dde..107c92b 100644 (file)
@@ -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);
index 3da25ec..a218701 100644 (file)
@@ -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);
+    }
 }
 
 /**
index 1cf0570..3f178a2 100644 (file)
@@ -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);
index 3f0b966..05a3aa8 100644 (file)
@@ -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;
 }
index eca81d8..8f7f1d4 100644 (file)
@@ -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,
index db16574..3a9cf84 100644 (file)
@@ -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);