2005-09-11 Mark McLoughlin <mark@skynet.ie>

	* test/data/auth/fallback.auth-script: we don't
	retry the EXTERNAL method when we know its going
	to fail anymore.

2005-09-11  Mark McLoughlin  <mark@skynet.ie>

	* dbus/dbus-connection-internal.h: rename
	(add|remove|toggle)_(watch|timeout) to unlocked()

	* dbus/dbus-connection.c: ditto.

	* dbus/dbus-timeout.c, dbus/dbus-transport-unix.c:
	Update some callers for the renaming.

2005-09-10  Mark McLoughlin  <mark@skynet.ie>

	* dbus/dbus-auth.c: (record_mechanisms): don't
	retry the first auth mechanism because we know
	we're just going to get rejected again.

	* dbus/dbus-keyring.c: (_dbus_keyring_reload):
	Fix thinko ... and what a nasty little bugger to
	track down you were ...

	* dbus/dbus-connection.c:
	(_dbus_connection_add_watch),
	(_dbus_connection_remove_watch): add note about
	these needing the connection to be locked.
	(_dbus_connection_get_dispatch_status_unlocked):
	set status to DATA_REMAINS when we queue the
	disconnected message.

	* bus/dispatch.c:
	(bus_dispatch): fix warning.
	(check_existent_service_no_auto_start):
	Expect ChildSignaled error too.
	(check_existent_hello_from_self): fix another
	couple of warnings.

9 files changed, 117 insertions, 72 deletions

diff --git a/ChangeLog b/ChangeLog
index 56065355..ebb200fc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,44 @@
+2005-09-11  Mark McLoughlin  <mark@skynet.ie>
+
+	* test/data/auth/fallback.auth-script: we don't
+	retry the EXTERNAL method when we know its going
+	to fail anymore.
+
+2005-09-11  Mark McLoughlin  <mark@skynet.ie>
+
+	* dbus/dbus-connection-internal.h: rename
+	(add|remove|toggle)_(watch|timeout) to unlocked()
+	
+	* dbus/dbus-connection.c: ditto.
+	
+	* dbus/dbus-timeout.c, dbus/dbus-transport-unix.c:
+	Update some callers for the renaming.
+
+2005-09-10  Mark McLoughlin  <mark@skynet.ie>
+
+	* dbus/dbus-auth.c: (record_mechanisms): don't
+	retry the first auth mechanism because we know
+	we're just going to get rejected again.
+	
+	* dbus/dbus-keyring.c: (_dbus_keyring_reload):
+	Fix thinko ... and what a nasty little bugger to
+	track down you were ...
+
+	* dbus/dbus-connection.c:
+	(_dbus_connection_add_watch),
+	(_dbus_connection_remove_watch): add note about
+	these needing the connection to be locked.
+	(_dbus_connection_get_dispatch_status_unlocked):
+	set status to DATA_REMAINS when we queue the
+	disconnected message.
+	
+	* bus/dispatch.c:
+	(bus_dispatch): fix warning.
+	(check_existent_service_no_auto_start):
+	Expect ChildSignaled error too.
+	(check_existent_hello_from_self): fix another
+	couple of warnings.
+	
 2005-09-08  Joe Shaw  <joeshaw@novell.com>
 
 	Patches from James Willcox <snorp@snorp.net>
diff --git a/bus/dispatch.c b/bus/dispatch.c
index 35c34283..9bb55bf1 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -271,8 +271,7 @@ bus_dispatch (DBusConnection *connection,
                                                 message, service_name, &error))
             {
               _DBUS_ASSERT_ERROR_IS_SET (&error);
-              _dbus_verbose ("bus_activation_activate_service() failed\n");
-               ("Failed: %s\n", error.name);
+              _dbus_verbose ("bus_activation_activate_service() failed: %s\n", error.name);
               goto out;
             }
           
@@ -2475,6 +2474,8 @@ check_existent_service_no_auto_start (BusContext     *context,
       else if (dbus_message_is_error (message,
                                       DBUS_ERROR_SPAWN_CHILD_EXITED) ||
                dbus_message_is_error (message,
+                                      DBUS_ERROR_SPAWN_CHILD_SIGNALED) ||
+               dbus_message_is_error (message,
                                       DBUS_ERROR_SPAWN_EXEC_FAILED))
         {
           ; /* good, this is expected also */
@@ -2821,8 +2822,6 @@ check_existent_hello_from_self (BusContext     *context,
 {
   DBusMessage *message;
   dbus_uint32_t serial;
-  dbus_bool_t retval;
-  const char *base_service;
   const char *text;
 
   message = dbus_message_new_method_call (EXISTENT_SERVICE_NAME,
@@ -2862,25 +2861,20 @@ check_existent_hello_from_self (BusContext     *context,
   if (message == NULL)
     {
       _dbus_warn ("Failed to pop message! Should have been reply from RunHelloFromSelf message\n");
-      goto out;
+      return FALSE;
     }
 
   if (dbus_message_get_reply_serial (message) != serial)
     {
       _dbus_warn ("Wrong reply serial\n");
-      goto out;
+      dbus_message_unref (message);
+      return FALSE;
     }
 
   dbus_message_unref (message);
   message = NULL;
       
-  retval = TRUE;
-
- out:
-  if (message)
-    dbus_message_unref (message);
-
-  return retval;
+  return TRUE;
 }
 
 
diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c
index 660dc435..ffa20373 100644
--- a/dbus/dbus-auth.c
+++ b/dbus/dbus-auth.c
@@ -1668,14 +1668,22 @@ record_mechanisms (DBusAuth         *auth,
            * it lists things in that order anyhow.
            */
 
-          _dbus_verbose ("%s: Adding mechanism %s to list we will try\n",
-                         DBUS_AUTH_NAME (auth), mech->mechanism);
+          if (mech != &all_mechanisms[0])
+            {
+              _dbus_verbose ("%s: Adding mechanism %s to list we will try\n",
+                             DBUS_AUTH_NAME (auth), mech->mechanism);
           
-          if (!_dbus_list_append (& DBUS_AUTH_CLIENT (auth)->mechs_to_try,
-                                  (void*) mech))
+              if (!_dbus_list_append (& DBUS_AUTH_CLIENT (auth)->mechs_to_try,
+                                      (void*) mech))
+                {
+                  _dbus_string_free (&m);
+                  goto nomem;
+                }
+            }
+          else
             {
-              _dbus_string_free (&m);
-              goto nomem;
+              _dbus_verbose ("%s: Already tried mechanism %s; not adding to list we will try\n",
+                             DBUS_AUTH_NAME (auth), mech->mechanism);
             }
         }
       else
diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
index c8995706..644793a9 100644
--- a/dbus/dbus-connection-internal.h
+++ b/dbus/dbus-connection-internal.h
@@ -56,21 +56,21 @@ dbus_bool_t       _dbus_connection_has_messages_to_send_unlocked (DBusConnection
 DBusMessage*      _dbus_connection_get_message_to_send         (DBusConnection     *connection);
 void              _dbus_connection_message_sent                (DBusConnection     *connection,
                                                                 DBusMessage        *message);
-dbus_bool_t       _dbus_connection_add_watch                   (DBusConnection     *connection,
+dbus_bool_t       _dbus_connection_add_watch_unlocked          (DBusConnection     *connection,
                                                                 DBusWatch          *watch);
-void              _dbus_connection_remove_watch                (DBusConnection     *connection,
+void              _dbus_connection_remove_watch_unlocked       (DBusConnection     *connection,
                                                                 DBusWatch          *watch);
-void              _dbus_connection_toggle_watch                (DBusConnection     *connection,
+void              _dbus_connection_toggle_watch_unlocked       (DBusConnection     *connection,
                                                                 DBusWatch          *watch,
                                                                 dbus_bool_t         enabled);
 dbus_bool_t       _dbus_connection_handle_watch                (DBusWatch          *watch,
                                                                 unsigned int        condition,
                                                                 void               *data);
-dbus_bool_t       _dbus_connection_add_timeout                 (DBusConnection     *connection,
+dbus_bool_t       _dbus_connection_add_timeout_unlocked        (DBusConnection     *connection,
                                                                 DBusTimeout        *timeout);
-void              _dbus_connection_remove_timeout              (DBusConnection     *connection,
+void              _dbus_connection_remove_timeout_unlocked     (DBusConnection     *connection,
                                                                 DBusTimeout        *timeout);
-void              _dbus_connection_toggle_timeout              (DBusConnection     *connection,
+void              _dbus_connection_toggle_timeout_unlocked     (DBusConnection     *connection,
                                                                 DBusTimeout        *timeout,
                                                                 dbus_bool_t         enabled);
 DBusConnection*   _dbus_connection_new_for_transport           (DBusTransport      *transport);
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index f9196101..ff66f5f4 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -377,8 +377,8 @@ _dbus_connection_queue_received_message_link (DBusConnection  *connection,
       if (pending != NULL)
 	{
 	  if (pending->timeout_added)
-	    _dbus_connection_remove_timeout (connection,
-                                             pending->timeout);
+            _dbus_connection_remove_timeout_unlocked (connection,
+                                                      pending->timeout);
 
 	  pending->timeout_added = FALSE;
 	}
@@ -594,14 +594,15 @@ protected_change_watch (DBusConnection         *connection,
  * available. Otherwise records the watch to be added when said
  * function is available. Also re-adds the watch if the
  * DBusAddWatchFunction changes. May fail due to lack of memory.
+ * Connection lock should be held when calling this.
  *
  * @param connection the connection.
  * @param watch the watch to add.
  * @returns #TRUE on success.
  */
 dbus_bool_t
-_dbus_connection_add_watch (DBusConnection *connection,
-                            DBusWatch      *watch)
+_dbus_connection_add_watch_unlocked (DBusConnection *connection,
+                                     DBusWatch      *watch)
 {
   return protected_change_watch (connection, watch,
                                  _dbus_watch_list_add_watch,
@@ -612,13 +613,14 @@ _dbus_connection_add_watch (DBusConnection *connection,
  * Removes a watch using the connection's DBusRemoveWatchFunction
  * if available. It's an error to call this function on a watch
  * that was not previously added.
+ * Connection lock should be held when calling this.
  *
  * @param connection the connection.
  * @param watch the watch to remove.
  */
 void
-_dbus_connection_remove_watch (DBusConnection *connection,
-                               DBusWatch      *watch)
+_dbus_connection_remove_watch_unlocked (DBusConnection *connection,
+                                        DBusWatch      *watch)
 {
   protected_change_watch (connection, watch,
                           NULL,
@@ -637,9 +639,9 @@ _dbus_connection_remove_watch (DBusConnection *connection,
  * @param enabled whether to enable or disable
  */
 void
-_dbus_connection_toggle_watch (DBusConnection *connection,
-                               DBusWatch      *watch,
-                               dbus_bool_t     enabled)
+_dbus_connection_toggle_watch_unlocked (DBusConnection *connection,
+                                        DBusWatch      *watch,
+                                        dbus_bool_t     enabled)
 {
   _dbus_assert (watch != NULL);
 
@@ -710,14 +712,15 @@ protected_change_timeout (DBusConnection           *connection,
  * function is available. Also re-adds the timeout if the
  * DBusAddTimeoutFunction changes. May fail due to lack of memory.
  * The timeout will fire repeatedly until removed.
+ * Connection lock should be held when calling this.
  *
  * @param connection the connection.
  * @param timeout the timeout to add.
  * @returns #TRUE on success.
  */
 dbus_bool_t
-_dbus_connection_add_timeout (DBusConnection *connection,
-			      DBusTimeout    *timeout)
+_dbus_connection_add_timeout_unlocked (DBusConnection *connection,
+                                       DBusTimeout    *timeout)
 {
   return protected_change_timeout (connection, timeout,
                                    _dbus_timeout_list_add_timeout,
@@ -728,13 +731,14 @@ _dbus_connection_add_timeout (DBusConnection *connection,
  * Removes a timeout using the connection's DBusRemoveTimeoutFunction
  * if available. It's an error to call this function on a timeout
  * that was not previously added.
+ * Connection lock should be held when calling this.
  *
  * @param connection the connection.
  * @param timeout the timeout to remove.
  */
 void
-_dbus_connection_remove_timeout (DBusConnection *connection,
-				 DBusTimeout    *timeout)
+_dbus_connection_remove_timeout_unlocked (DBusConnection *connection,
+                                          DBusTimeout    *timeout)
 {
   protected_change_timeout (connection, timeout,
                             NULL,
@@ -746,15 +750,16 @@ _dbus_connection_remove_timeout (DBusConnection *connection,
  * Toggles a timeout and notifies app via connection's
  * DBusTimeoutToggledFunction if available. It's an error to call this
  * function on a timeout that was not previously added.
+ * Connection lock should be held when calling this.
  *
  * @param connection the connection.
  * @param timeout the timeout to toggle.
  * @param enabled whether to enable or disable
  */
 void
-_dbus_connection_toggle_timeout (DBusConnection   *connection,
-                                 DBusTimeout      *timeout,
-                                 dbus_bool_t       enabled)
+_dbus_connection_toggle_timeout_unlocked (DBusConnection   *connection,
+                                          DBusTimeout      *timeout,
+                                          dbus_bool_t       enabled)
 {
   protected_change_timeout (connection, timeout,
                             NULL, NULL,
@@ -770,14 +775,14 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection  *connection,
   
   _dbus_assert (pending->reply_serial != 0);
 
-  if (!_dbus_connection_add_timeout (connection, pending->timeout))
+  if (!_dbus_connection_add_timeout_unlocked (connection, pending->timeout))
     return FALSE;
   
   if (!_dbus_hash_table_insert_int (connection->pending_replies,
                                     pending->reply_serial,
                                     pending))
     {
-      _dbus_connection_remove_timeout (connection, pending->timeout);
+      _dbus_connection_remove_timeout_unlocked (connection, pending->timeout);
 
       HAVE_LOCK_CHECK (connection);
       return FALSE;
@@ -807,8 +812,8 @@ free_pending_call_on_hash_removal (void *data)
     {
       if (pending->timeout_added)
         {
-          _dbus_connection_remove_timeout (pending->connection,
-                                           pending->timeout);
+          _dbus_connection_remove_timeout_unlocked (pending->connection,
+                                                    pending->timeout);
           pending->timeout_added = FALSE;
         }
 
@@ -2353,8 +2358,8 @@ reply_handler_timeout (void *data)
       pending->timeout_link = NULL;
     }
 
-  _dbus_connection_remove_timeout (connection,
-				   pending->timeout);
+  _dbus_connection_remove_timeout_unlocked (connection,
+				            pending->timeout);
   pending->timeout_added = FALSE;
 
   _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
@@ -3264,6 +3269,8 @@ _dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection)
               _dbus_connection_queue_synthesized_message_link (connection,
                                                                connection->disconnect_message_link);
               connection->disconnect_message_link = NULL;
+
+              status = DBUS_DISPATCH_DATA_REMAINS;
             }
 
           /* Dump the outgoing queue, we aren't going to be able to
diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
index ede14d9d..77d90605 100644
--- a/dbus/dbus-keyring.c
+++ b/dbus/dbus-keyring.c
@@ -482,7 +482,7 @@ _dbus_keyring_reload (DBusKeyring *keyring,
       DBusKey *new;
 
       /* Don't load more than the max. */
-      if (n_keys >= (add_new ? MAX_KEYS_IN_FILE : MAX_KEYS_IN_FILE - 1))
+      if (n_keys >= (add_new ? MAX_KEYS_IN_FILE - 1 : MAX_KEYS_IN_FILE))
         break;
       
       next = 0;
diff --git a/dbus/dbus-timeout.c b/dbus/dbus-timeout.c
index bea86eb9..e684131e 100644
--- a/dbus/dbus-timeout.c
+++ b/dbus/dbus-timeout.c
@@ -123,8 +123,8 @@ _dbus_timeout_unref (DBusTimeout *timeout)
 /**
  * Changes the timeout interval. Note that you have to disable and
  * re-enable the timeout using the timeout toggle function
- * (_dbus_connection_toggle_timeout() etc.) to notify the application
- * of this change.
+ * (_dbus_connection_toggle_timeout_unlocked() etc.) to notify the
+ * application of this change.
  *
  * @param timeout the timeout
  * @param interval the new interval
@@ -140,7 +140,7 @@ _dbus_timeout_set_interval (DBusTimeout *timeout,
 
 /**
  * Changes the timeout's enabled-ness. Note that you should use
- * _dbus_connection_toggle_timeout() etc. instead, if
+ * _dbus_connection_toggle_timeout_unlocked() etc. instead, if
  * the timeout is passed out to an application main loop.
  * i.e. you can't use this function in the D-BUS library, it's
  * only used in the message bus daemon implementation.
diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c
index 4c07d5f3..f8af9c07 100644
--- a/dbus/dbus-transport-unix.c
+++ b/dbus/dbus-transport-unix.c
@@ -76,8 +76,8 @@ free_watches (DBusTransport *transport)
   if (unix_transport->read_watch)
     {
       if (transport->connection)
-        _dbus_connection_remove_watch (transport->connection,
-                                       unix_transport->read_watch);
+        _dbus_connection_remove_watch_unlocked (transport->connection,
+                                                unix_transport->read_watch);
       _dbus_watch_invalidate (unix_transport->read_watch);
       _dbus_watch_unref (unix_transport->read_watch);
       unix_transport->read_watch = NULL;
@@ -86,8 +86,8 @@ free_watches (DBusTransport *transport)
   if (unix_transport->write_watch)
     {
       if (transport->connection)
-        _dbus_connection_remove_watch (transport->connection,
-                                       unix_transport->write_watch);
+        _dbus_connection_remove_watch_unlocked (transport->connection,
+                                                unix_transport->write_watch);
       _dbus_watch_invalidate (unix_transport->write_watch);
       _dbus_watch_unref (unix_transport->write_watch);
       unix_transport->write_watch = NULL;
@@ -162,9 +162,9 @@ check_write_watch (DBusTransport *transport)
                  unix_transport->fd,
                  _dbus_connection_has_messages_to_send_unlocked (transport->connection));
 
-  _dbus_connection_toggle_watch (transport->connection,
-                                 unix_transport->write_watch,
-                                 needed);
+  _dbus_connection_toggle_watch_unlocked (transport->connection,
+                                          unix_transport->write_watch,
+                                          needed);
 
   _dbus_transport_unref (transport);
 }
@@ -222,9 +222,9 @@ check_read_watch (DBusTransport *transport)
     }
 
   _dbus_verbose ("  setting read watch enabled = %d\n", need_read_watch);
-  _dbus_connection_toggle_watch (transport->connection,
-                                 unix_transport->read_watch,
-                                 need_read_watch);
+  _dbus_connection_toggle_watch_unlocked (transport->connection,
+                                          unix_transport->read_watch,
+                                          need_read_watch);
 
   _dbus_transport_unref (transport);
 }
@@ -899,15 +899,15 @@ unix_connection_set (DBusTransport *transport)
                            _dbus_connection_handle_watch,
                            transport->connection, NULL);
   
-  if (!_dbus_connection_add_watch (transport->connection,
-                                   unix_transport->write_watch))
+  if (!_dbus_connection_add_watch_unlocked (transport->connection,
+                                            unix_transport->write_watch))
     return FALSE;
 
-  if (!_dbus_connection_add_watch (transport->connection,
-                                   unix_transport->read_watch))
+  if (!_dbus_connection_add_watch_unlocked (transport->connection,
+                                            unix_transport->read_watch))
     {
-      _dbus_connection_remove_watch (transport->connection,
-                                     unix_transport->write_watch);
+      _dbus_connection_remove_watch_unlocked (transport->connection,
+                                              unix_transport->write_watch);
       return FALSE;
     }
 
diff --git a/test/data/auth/fallback.auth-script b/test/data/auth/fallback.auth-script
index f598b77e..0d880c00 100644
--- a/test/data/auth/fallback.auth-script
+++ b/test/data/auth/fallback.auth-script
@@ -7,11 +7,6 @@ CLIENT
 EXPECT_COMMAND AUTH
 SEND 'REJECTED EXTERNAL DBUS_COOKIE_SHA1 DBUS_TEST_NONEXISTENT_MECH'
 
-## Will try EXTERNAL again.
-
-EXPECT_COMMAND AUTH
-SEND 'REJECTED EXTERNAL DBUS_COOKIE_SHA1 DBUS_TEST_NONEXISTENT_MECH'
-
 ## And this time we get DBUS_COOKIE_SHA1
 
 EXPECT_COMMAND AUTH