From 36a91bfdb8f20a7602d2286c80aaa1870752bc95 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 21 Jan 2011 18:54:33 +0000 Subject: [PATCH] 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 --- bus/activation.c | 8 ++++---- bus/bus.c | 20 ++------------------ bus/connection.c | 23 ++--------------------- bus/dir-watch-inotify.c | 11 ++--------- bus/dir-watch-kqueue.c | 11 ++--------- bus/main.c | 14 ++------------ bus/test.c | 20 ++------------------ dbus/dbus-mainloop.c | 45 ++++++++++++++++++++++++++------------------- dbus/dbus-mainloop.h | 6 +++--- test/test-utils.c | 36 ++++-------------------------------- 10 files changed, 49 insertions(+), 145 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 348aca8..2349612 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1403,9 +1403,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 @@ -1415,7 +1415,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 b512ab5..cebdb7c 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -99,19 +99,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) { @@ -120,9 +107,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 @@ -134,8 +119,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 692948e..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 54704a4..2e9be98 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 62f7b3d..ac6290c 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 1af588e..2a03266 100644 --- a/bus/main.c +++ b/bus/main.c @@ -201,14 +201,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) { @@ -238,8 +230,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); @@ -252,8 +243,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 8d16340..049fae6 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 087bb5b..436cfee 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 7d78c82..5be955b 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 0811735..81aac9e 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 -- 1.7.2.3