DBusLoop: remove second layer of watch callbacks where possible

Similar to the previous commit, almost every use of DBusWatch can just
have the main loop call dbus_watch_handle.

The one exception is the bus activation code; it's had a comment
explaining why it's wrong since 2003. We should fix that one day, but for
now, just migrate it to a new _dbus_loop_add_watch_full which preserves
the second-layer callback.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=33342
Reviewed-by: Thiago Macieira <thiago@kde.org>

10 files changed, 49 insertions, 145 deletions

diff --git a/bus/activation.c b/bus/activation.c
index 31dbaf54..7ce463c3 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -1427,9 +1427,9 @@ add_babysitter_watch (DBusWatch      *watch,
 {
   BusPendingActivation *pending_activation = data;
 
-  return _dbus_loop_add_watch (bus_context_get_loop (pending_activation->activation->context),
-                               watch, babysitter_watch_callback, pending_activation,
-                               NULL);
+  return _dbus_loop_add_watch_full (
+      bus_context_get_loop (pending_activation->activation->context),
+      watch, babysitter_watch_callback, pending_activation, NULL);
 }
 
 static void
@@ -1439,7 +1439,7 @@ remove_babysitter_watch (DBusWatch      *watch,
   BusPendingActivation *pending_activation = data;
 
   _dbus_loop_remove_watch (bus_context_get_loop (pending_activation->activation->context),
-                           watch, babysitter_watch_callback, pending_activation);
+                           watch);
 }
 
 static dbus_bool_t
diff --git a/bus/bus.c b/bus/bus.c
index 2008d1de..04b12866 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -103,19 +103,6 @@ server_get_context (DBusServer *server)
 }
 
 static dbus_bool_t
-server_watch_callback (DBusWatch     *watch,
-                       unsigned int   condition,
-                       void          *data)
-{
-  /* FIXME this can be done in dbus-mainloop.c
-   * if the code in activation.c for the babysitter
-   * watch handler is fixed.
-   */
-
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 add_server_watch (DBusWatch  *watch,
                   void       *data)
 {
@@ -124,9 +111,7 @@ add_server_watch (DBusWatch  *watch,
 
   context = server_get_context (server);
 
-  return _dbus_loop_add_watch (context->loop,
-                               watch, server_watch_callback, server,
-                               NULL);
+  return _dbus_loop_add_watch (context->loop, watch);
 }
 
 static void
@@ -138,8 +123,7 @@ remove_server_watch (DBusWatch  *watch,
 
   context = server_get_context (server);
 
-  _dbus_loop_remove_watch (context->loop,
-                           watch, server_watch_callback, server);
+  _dbus_loop_remove_watch (context->loop, watch);
 }
 
 static dbus_bool_t
diff --git a/bus/connection.c b/bus/connection.c
index 692948e0..d74b3261 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -295,30 +295,12 @@ bus_connection_disconnected (DBusConnection *connection)
 }
 
 static dbus_bool_t
-connection_watch_callback (DBusWatch     *watch,
-                           unsigned int   condition,
-                           void          *data)
-{
- /* FIXME this can be done in dbus-mainloop.c
-  * if the code in activation.c for the babysitter
-  * watch handler is fixed.
-  */
-  
-#if 0
-  _dbus_verbose ("Calling handle_watch\n");
-#endif
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 add_connection_watch (DBusWatch      *watch,
                       void           *data)
 {
   DBusConnection *connection = data;
 
-  return _dbus_loop_add_watch (connection_get_loop (connection),
-                               watch, connection_watch_callback, connection,
-                               NULL);
+  return _dbus_loop_add_watch (connection_get_loop (connection), watch);
 }
 
 static void
@@ -327,8 +309,7 @@ remove_connection_watch (DBusWatch      *watch,
 {
   DBusConnection *connection = data;
   
-  _dbus_loop_remove_watch (connection_get_loop (connection),
-                           watch, connection_watch_callback, connection);
+  _dbus_loop_remove_watch (connection_get_loop (connection), watch);
 }
 
 static dbus_bool_t
diff --git a/bus/dir-watch-inotify.c b/bus/dir-watch-inotify.c
index 54704a4f..2e9be988 100644
--- a/bus/dir-watch-inotify.c
+++ b/bus/dir-watch-inotify.c
@@ -50,12 +50,6 @@ static DBusWatch *watch = NULL;
 static DBusLoop *loop = NULL;
 
 static dbus_bool_t
-_inotify_watch_callback (DBusWatch *watch, unsigned int condition, void *data)
-{
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 _handle_inotify_watch (DBusWatch *passed_watch, unsigned int flags, void *data)
 {
   char buffer[INOTIFY_BUF_LEN];
@@ -206,7 +200,7 @@ _shutdown_inotify (void *data)
 
   if (watch != NULL)
     {
-      _dbus_loop_remove_watch (loop, watch, _inotify_watch_callback, NULL);
+      _dbus_loop_remove_watch (loop, watch);
       _dbus_watch_invalidate (watch);
       _dbus_watch_unref (watch);
       _dbus_loop_unref (loop);
@@ -252,8 +246,7 @@ _init_inotify (BusContext *context)
           goto out;
         }
 
-      if (!_dbus_loop_add_watch (loop, watch, _inotify_watch_callback,
-                                 NULL, NULL))
+      if (!_dbus_loop_add_watch (loop, watch))
         {
           _dbus_warn ("Unable to add reload watch to main loop");
           _dbus_watch_unref (watch);
diff --git a/bus/dir-watch-kqueue.c b/bus/dir-watch-kqueue.c
index 62f7b3dc..ac6290cc 100644
--- a/bus/dir-watch-kqueue.c
+++ b/bus/dir-watch-kqueue.c
@@ -50,12 +50,6 @@ static DBusWatch *watch = NULL;
 static DBusLoop *loop = NULL;
 
 static dbus_bool_t
-_kqueue_watch_callback (DBusWatch *watch, unsigned int condition, void *data)
-{
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 _handle_kqueue_watch (DBusWatch *watch, unsigned int flags, void *data)
 {
   struct kevent ev;
@@ -80,7 +74,7 @@ _handle_kqueue_watch (DBusWatch *watch, unsigned int flags, void *data)
       kq = -1;
       if (watch != NULL)
 	{
-	  _dbus_loop_remove_watch (loop, watch, _kqueue_watch_callback, NULL);
+	  _dbus_loop_remove_watch (loop, watch);
           _dbus_watch_invalidate (watch);
           _dbus_watch_unref (watch);
 	  watch = NULL;
@@ -121,8 +115,7 @@ _init_kqueue (BusContext *context)
 	    goto out;
 	  }
 
-	if (!_dbus_loop_add_watch (loop, watch, _kqueue_watch_callback,
-                                   NULL, NULL))
+	if (!_dbus_loop_add_watch (loop, watch))
           {
             _dbus_warn ("Unable to add reload watch to main loop");
 	    _dbus_watch_invalidate (watch);
diff --git a/bus/main.c b/bus/main.c
index 72c9d74f..74b8bd2a 100644
--- a/bus/main.c
+++ b/bus/main.c
@@ -220,14 +220,6 @@ handle_reload_watch (DBusWatch    *watch,
   return TRUE;
 }
 
-static dbus_bool_t
-reload_watch_callback (DBusWatch    *watch,
-		       unsigned int  condition,
-		       void         *data)
-{
-  return dbus_watch_handle (watch, condition);
-}
-
 static void
 setup_reload_pipe (DBusLoop *loop)
 {
@@ -257,8 +249,7 @@ setup_reload_pipe (DBusLoop *loop)
       exit (1);
     }
 
-  if (!_dbus_loop_add_watch (loop, watch, reload_watch_callback,
-			     NULL, NULL))
+  if (!_dbus_loop_add_watch (loop, watch))
     {
       _dbus_warn ("Unable to add reload watch to main loop: %s\n",
 		  error.message);
@@ -271,8 +262,7 @@ setup_reload_pipe (DBusLoop *loop)
 static void
 close_reload_pipe (DBusWatch **watch)
 {
-    _dbus_loop_remove_watch (bus_context_get_loop (context),
-        *watch, reload_watch_callback, NULL);
+    _dbus_loop_remove_watch (bus_context_get_loop (context), *watch);
     _dbus_watch_invalidate (*watch);
     _dbus_watch_unref (*watch);
     *watch = NULL;
diff --git a/bus/test.c b/bus/test.c
index 8d16340e..049fae6f 100644
--- a/bus/test.c
+++ b/bus/test.c
@@ -37,27 +37,12 @@ static DBusList *clients = NULL;
 static DBusLoop *client_loop = NULL;
 
 static dbus_bool_t
-client_watch_callback (DBusWatch     *watch,
-                       unsigned int   condition,
-                       void          *data)
-{
-  /* FIXME this can be done in dbus-mainloop.c
-   * if the code in activation.c for the babysitter
-   * watch handler is fixed.
-   */
-
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 add_client_watch (DBusWatch      *watch,
                   void           *data)
 {
   DBusConnection *connection = data;
 
-  return _dbus_loop_add_watch (client_loop,
-                               watch, client_watch_callback, connection,
-                               NULL);
+  return _dbus_loop_add_watch (client_loop, watch);
 }
 
 static void
@@ -66,8 +51,7 @@ remove_client_watch (DBusWatch      *watch,
 {
   DBusConnection *connection = data;
 
-  _dbus_loop_remove_watch (client_loop,
-                           watch, client_watch_callback, connection);
+  _dbus_loop_remove_watch (client_loop, watch);
 }
 
 static dbus_bool_t
diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c
index 087bb5bc..436cfee2 100644
--- a/dbus/dbus-mainloop.c
+++ b/dbus/dbus-mainloop.c
@@ -254,11 +254,18 @@ _dbus_loop_unref (DBusLoop *loop)
 }
 
 dbus_bool_t
-_dbus_loop_add_watch (DBusLoop          *loop,
-                      DBusWatch        *watch,
-                      DBusWatchFunction  function,
-                      void             *data,
-                      DBusFreeFunction  free_data_func)
+_dbus_loop_add_watch (DBusLoop  *loop,
+                      DBusWatch *watch)
+{
+  return _dbus_loop_add_watch_full (loop, watch, NULL, NULL, NULL);
+}
+
+dbus_bool_t
+_dbus_loop_add_watch_full (DBusLoop          *loop,
+                           DBusWatch         *watch,
+                           DBusWatchFunction  function,
+                           void              *data,
+                           DBusFreeFunction   free_data_func)
 {
   WatchCallback *wcb;
 
@@ -277,10 +284,8 @@ _dbus_loop_add_watch (DBusLoop          *loop,
 }
 
 void
-_dbus_loop_remove_watch (DBusLoop          *loop,
-                         DBusWatch        *watch,
-                         DBusWatchFunction  function,
-                         void             *data)
+_dbus_loop_remove_watch (DBusLoop         *loop,
+                         DBusWatch        *watch)
 {
   DBusList *link;
 
@@ -295,9 +300,7 @@ _dbus_loop_remove_watch (DBusLoop          *loop,
       Callback *this = link->data;
 
       if (this->type == CALLBACK_WATCH &&
-          WATCH_CALLBACK (this)->watch == watch &&
-          WATCH_CALLBACK (this)->data == data &&
-          WATCH_CALLBACK (this)->function == function)
+          WATCH_CALLBACK (this)->watch == watch)
         {
           remove_callback (loop, link);
           
@@ -307,8 +310,7 @@ _dbus_loop_remove_watch (DBusLoop          *loop,
       link = next;
     }
 
-  _dbus_warn ("could not find watch %p function %p data %p to remove\n",
-              watch, (void *)function, data);
+  _dbus_warn ("could not find watch %p to remove\n", watch);
 }
 
 dbus_bool_t
@@ -596,8 +598,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
             {
               _dbus_warn ("watch %p was invalidated but not removed; "
                   "removing it now\n", wcb->watch);
-              _dbus_loop_remove_watch (loop, wcb->watch, wcb->function,
-                  wcb->data);
+              _dbus_loop_remove_watch (loop, wcb->watch);
             }
           else if (dbus_watch_get_enabled (wcb->watch))
             {
@@ -807,7 +808,14 @@ _dbus_loop_iterate (DBusLoop     *loop,
               if (condition != 0 &&
                   dbus_watch_get_enabled (wcb->watch))
                 {
-                  if (!(* wcb->function) (wcb->watch, condition, wcb->data))
+                  dbus_bool_t oom;
+
+                  if (wcb->function)
+                    oom = !(* wcb->function) (wcb->watch, condition, wcb->data);
+                  else
+                    oom = !dbus_watch_handle (wcb->watch, condition);
+
+                  if (oom)
                     wcb->last_iteration_oom = TRUE;
 
 #if MAINLOOP_SPEW
@@ -824,8 +832,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
 
                   _dbus_warn ("invalid request, socket fd %d not open\n",
                       fds[i].fd);
-                  _dbus_loop_remove_watch (loop, watch, wcb->function,
-                      wcb->data);
+                  _dbus_loop_remove_watch (loop, watch);
                   _dbus_watch_invalidate (watch);
                   _dbus_watch_unref (watch);
                 }
diff --git a/dbus/dbus-mainloop.h b/dbus/dbus-mainloop.h
index 7d78c82e..5be955b4 100644
--- a/dbus/dbus-mainloop.h
+++ b/dbus/dbus-mainloop.h
@@ -38,14 +38,14 @@ DBusLoop*   _dbus_loop_new            (void);
 DBusLoop*   _dbus_loop_ref            (DBusLoop            *loop);
 void        _dbus_loop_unref          (DBusLoop            *loop);
 dbus_bool_t _dbus_loop_add_watch      (DBusLoop            *loop,
+                                       DBusWatch           *watch);
+dbus_bool_t _dbus_loop_add_watch_full (DBusLoop            *loop,
                                        DBusWatch           *watch,
                                        DBusWatchFunction    function,
                                        void                *data,
                                        DBusFreeFunction     free_data_func);
 void        _dbus_loop_remove_watch   (DBusLoop            *loop,
-                                       DBusWatch           *watch,
-                                       DBusWatchFunction    function,
-                                       void                *data);
+                                       DBusWatch           *watch);
 dbus_bool_t _dbus_loop_add_timeout    (DBusLoop            *loop,
                                        DBusTimeout         *timeout);
 void        _dbus_loop_remove_timeout (DBusLoop            *loop,
diff --git a/test/test-utils.c b/test/test-utils.c
index f8e366b7..4fd84fe8 100644
--- a/test/test-utils.c
+++ b/test/test-utils.c
@@ -9,23 +9,12 @@ typedef struct
 } CData;
 
 static dbus_bool_t
-connection_watch_callback (DBusWatch     *watch,
-                           unsigned int   condition,
-                           void          *data)
-{
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 add_watch (DBusWatch *watch,
 	   void      *data)
 {
   CData *cd = data;
 
-  return _dbus_loop_add_watch (cd->loop,
-                               watch,
-                               connection_watch_callback,
-                               cd, NULL);
+  return _dbus_loop_add_watch (cd->loop, watch);
 }
 
 static void
@@ -34,8 +23,7 @@ remove_watch (DBusWatch *watch,
 {
   CData *cd = data;
   
-  _dbus_loop_remove_watch (cd->loop,
-                           watch, connection_watch_callback, cd);  
+  _dbus_loop_remove_watch (cd->loop, watch);
 }
 
 static dbus_bool_t
@@ -216,27 +204,12 @@ serverdata_new (DBusLoop       *loop,
 }
 
 static dbus_bool_t
-server_watch_callback (DBusWatch     *watch,
-                       unsigned int   condition,
-                       void          *data)
-{
-  /* FIXME this can be done in dbus-mainloop.c
-   * if the code in activation.c for the babysitter
-   * watch handler is fixed.
-   */
-
-  return dbus_watch_handle (watch, condition);
-}
-
-static dbus_bool_t
 add_server_watch (DBusWatch  *watch,
                   void       *data)
 {
   ServerData *context = data;
 
-  return _dbus_loop_add_watch (context->loop,
-                               watch, server_watch_callback, context,
-                               NULL);
+  return _dbus_loop_add_watch (context->loop, watch);
 }
 
 static void
@@ -245,8 +218,7 @@ remove_server_watch (DBusWatch  *watch,
 {
   ServerData *context = data;
   
-  _dbus_loop_remove_watch (context->loop,
-                           watch, server_watch_callback, context);
+  _dbus_loop_remove_watch (context->loop, watch);
 }
 
 static dbus_bool_t