From: Simon McVittie Date: Fri, 21 Jan 2011 18:54:33 +0000 (+0000) Subject: DBusLoop: remove second layer of watch callbacks where possible X-Git-Tag: dbus-1.5.6~102 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4b43f5db7a3570978b49cb3adca3ca4afc95f8aa;p=thirdparty%2Fdbus.git 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 --- diff --git a/bus/activation.c b/bus/activation.c index 31dbaf546..7ce463c3b 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 2008d1de1..04b128665 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -102,19 +102,6 @@ server_get_context (DBusServer *server) return context; } -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 692948e0e..d74b3261f 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -294,31 +294,13 @@ bus_connection_disconnected (DBusConnection *connection) dbus_connection_unref (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 54704a4fc..2e9be9887 100644 --- a/bus/dir-watch-inotify.c +++ b/bus/dir-watch-inotify.c @@ -49,12 +49,6 @@ static int inotify_fd = -1; 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) { @@ -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 62f7b3dcb..ac6290ccf 100644 --- a/bus/dir-watch-kqueue.c +++ b/bus/dir-watch-kqueue.c @@ -49,12 +49,6 @@ static int num_fds = 0; 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) { @@ -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 72c9d74f5..74b8bd2a5 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 8d16340ed..049fae6fa 100644 --- a/bus/test.c +++ b/bus/test.c @@ -36,28 +36,13 @@ 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 087bb5bc3..436cfee28 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 7d78c82e9..5be955b4a 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 f8e366b71..4fd84fe86 100644 --- a/test/test-utils.c +++ b/test/test-utils.c @@ -8,24 +8,13 @@ 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 @@ -215,28 +203,13 @@ serverdata_new (DBusLoop *loop, return sd; } -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