From 192db467fff42f9ff299ea07bfc4cfa3cc1f7885 Mon Sep 17 00:00:00 2001 From: barbieri Date: Mon, 13 Oct 2008 18:23:00 +0000 Subject: [PATCH] e_dbus: signals are per-connection, fix signal match removal. Each connection must have its own signal handler lists. This patch also fixes match removal, it was using the sh->PIECE instead of PIECE to give it names, it's also better split the buffer manipulation so it checks for overflows as well. NOTE: signal handling is still unoptimal, it should be handled per connection instead of a global ecore signal. git-svn-id: svn+ssh://svn.enlightenment.org/var/svn/e/trunk/e_dbus@36639 7cbeb6ba-43b4-40fd-8cce-4c39aea84d33 --- src/lib/dbus/e_dbus.c | 7 +- src/lib/dbus/e_dbus_private.h | 4 +- src/lib/dbus/e_dbus_signal.c | 267 ++++++++++++++++++++++++------------------ 3 files changed, 161 insertions(+), 117 deletions(-) diff --git a/src/lib/dbus/e_dbus.c b/src/lib/dbus/e_dbus.c index 2e7a49a..fd896d2 100644 --- a/src/lib/dbus/e_dbus.c +++ b/src/lib/dbus/e_dbus.c @@ -185,6 +185,11 @@ e_dbus_connection_free(void *data) if (cd->shared_type != -1) shared_connections[cd->shared_type] = NULL; + if (cd->signal_dispatcher) + ecore_event_handler_del(cd->signal_dispatcher); + if (cd->signal_handlers) + ecore_list_destroy(cd->signal_handlers); + if (cd->conn_name) free(cd->conn_name); if (cd->idler) ecore_idler_del(cd->idler); @@ -579,7 +584,6 @@ e_dbus_init(void) if (++init != 1) return init; E_DBUS_EVENT_SIGNAL = ecore_event_type_new(); - e_dbus_signal_init(); e_dbus_object_init(); return init; } @@ -592,6 +596,5 @@ e_dbus_shutdown(void) { if (--init) return init; e_dbus_object_shutdown(); - e_dbus_signal_shutdown(); return init; } diff --git a/src/lib/dbus/e_dbus_private.h b/src/lib/dbus/e_dbus_private.h index 24248f0..8c5a2a4 100644 --- a/src/lib/dbus/e_dbus_private.h +++ b/src/lib/dbus/e_dbus_private.h @@ -15,6 +15,8 @@ struct E_DBus_Connection Ecore_List *fd_handlers; Ecore_List *timeouts; + Ecore_List *signal_handlers; + Ecore_Event_Handler *signal_dispatcher; Ecore_Idler *idler; @@ -29,8 +31,6 @@ struct E_DBus_Callback void *user_data; }; -int e_dbus_signal_init(void); -void e_dbus_signal_shutdown(void); int e_dbus_object_init(void); void e_dbus_object_shutdown(void); diff --git a/src/lib/dbus/e_dbus_signal.c b/src/lib/dbus/e_dbus_signal.c index eb32562..8cb0ee9 100644 --- a/src/lib/dbus/e_dbus_signal.c +++ b/src/lib/dbus/e_dbus_signal.c @@ -7,12 +7,8 @@ #include "e_dbus_private.h" #include "dbus/dbus.h" -static Ecore_List *signal_handlers = NULL; - -static Ecore_Event_Handler *event_handler = NULL; static int init = 0; - struct E_DBus_Signal_Handler { char *sender; @@ -27,86 +23,95 @@ struct E_DBus_Signal_Handler static int cb_signal_event(void *data, int type, void *event); -void e_dbus_signal_handler_free(E_DBus_Signal_Handler *sh); - - -/** - * Initialize the signal subsystem - * @internal - */ -int -e_dbus_signal_init(void) -{ - if (++init != 1) return init; - signal_handlers = ecore_list_new(); - if (!signal_handlers) {--init; return 0;}; - ecore_list_free_cb_set(signal_handlers, ECORE_FREE_CB(e_dbus_signal_handler_free)); - - event_handler = ecore_event_handler_add(E_DBUS_EVENT_SIGNAL, cb_signal_event, NULL); - return init; -} - -/** - * Shutdown the signal subsystem - * @internal - */ -void -e_dbus_signal_shutdown(void) -{ - if (--init) return; - ecore_list_destroy(signal_handlers); - - if (event_handler) ecore_event_handler_del(event_handler); - event_handler = NULL; -} - /** * Free a signal handler * @param sh the signal handler to free */ -void +static void e_dbus_signal_handler_free(E_DBus_Signal_Handler *sh) { - if (sh->sender) free(sh->sender); - if (sh->path) free(sh->path); - if (sh->interface) free(sh->interface); - if (sh->member) free(sh->member); - + free(sh->sender); free(sh); } +struct cb_name_owner_data +{ + E_DBus_Connection *conn; + E_DBus_Signal_Handler *sh; +}; + static void cb_name_owner(void *data, DBusMessage *msg, DBusError *err) { const char *unique_name = NULL; + struct cb_name_owner_data *d = data; + E_DBus_Connection *conn; E_DBus_Signal_Handler *sh; - sh = data; + conn = d->conn; + sh = d->sh; + free(d); if (dbus_error_is_set(err)) - { - if (ecore_list_goto(signal_handlers, sh)) - ecore_list_remove(signal_handlers); - dbus_error_free(err); - return; - } + goto error; dbus_message_get_args(msg, err, DBUS_TYPE_STRING, &unique_name, DBUS_TYPE_INVALID); if (dbus_error_is_set(err)) - { - DEBUG(1, "Invalid signature in reply to name owner call\n"); - dbus_error_free(err); - return; - } + goto error; - if (unique_name) - { - if (sh->sender) free(sh->sender); - sh->sender = strdup(unique_name); - } - else DEBUG(1, "Error, no unique name?\n"); + if (!unique_name) + goto error; + + free(sh->sender); + sh->sender = strdup(unique_name); + + return; + + error: + if (err) + DEBUG(1, "ERROR: %s %s\n", err->name, err->message); + + if (ecore_list_goto(conn->signal_handlers, sh)) + ecore_list_remove(conn->signal_handlers); + e_dbus_signal_handler_free(sh); + dbus_error_free(err); +} + +static int +_match_append(char *buf, int size, int *used, const char *keyword, int keyword_size, const char *value, int value_size) +{ + if (*used + keyword_size + value_size + sizeof(",=''") >= size) + { + DEBUG(1, "ERROR: cannot add match %s='%s': too long!\n", keyword, value); + return 0; + } + + buf += *used; + + *buf = ','; + buf++; + + memcpy(buf, keyword, keyword_size); + buf += keyword_size; + + *buf = '='; + buf++; + *buf = '\''; + buf++; + + memcpy(buf, value, value_size); + buf += value_size; + + *buf = '\''; + buf++; + + *buf = '\0'; + + *used += keyword_size + value_size + sizeof(",=''") - 1; + + return 1; } /** @@ -125,44 +130,84 @@ e_dbus_signal_handler_add(E_DBus_Connection *conn, const char *sender, const cha { E_DBus_Signal_Handler *sh; char match[DBUS_MAXIMUM_MATCH_RULE_LENGTH]; - int started = 0; - int len = 0; + int len, sender_len, path_len, interface_len, member_len; DBusError err; - sh = calloc(1, sizeof(E_DBus_Signal_Handler)); - strcpy(match, "type='signal'"); - len = 13; - + len = sizeof("type='signal'") - 1; + +#define ADD_MATCH_PIECE(PIECE) \ + do { \ + PIECE ## _len = PIECE ? strlen(PIECE) : 0; \ + if (!_match_append(match, sizeof(match), &len, #PIECE, sizeof(#PIECE) - 1, PIECE, PIECE ## _len)) \ + return NULL; \ + } while (0) + + ADD_MATCH_PIECE(sender); + ADD_MATCH_PIECE(path); + ADD_MATCH_PIECE(interface); + ADD_MATCH_PIECE(member); #undef ADD_MATCH_PIECE -#define ADD_MATCH_PIECE(PIECE) \ - if (PIECE) \ - {\ - len += strlen("," #PIECE "=''") + strlen(PIECE);\ - if (len >= sizeof(match)) return NULL;\ - strcat(match, "," #PIECE "='");\ - strcat(match, PIECE);\ - strcat(match, "'");\ - sh->PIECE = strdup(PIECE);\ - started = 1;\ - } - ADD_MATCH_PIECE(sender) - ADD_MATCH_PIECE(path) - ADD_MATCH_PIECE(interface) - ADD_MATCH_PIECE(member) + len = path_len + interface_len + member_len + 4; + sh = malloc(sizeof(*sh) + len); + if (!sh) + { + DEBUG(1, "ERROR: could not allocate signal handler.\n"); + return NULL; + } + + len = sizeof(*sh); + +#define SET_STRING(PIECE) \ + do { \ + sh->PIECE = (char *)sh + len; \ + if (PIECE) \ + memcpy(sh->PIECE, PIECE, PIECE ## _len + 1); \ + else \ + sh->PIECE = NULL; \ + len += PIECE ## _len + 1; \ + } while (0) + + SET_STRING(path); + SET_STRING(interface); + SET_STRING(member); +#undef SET_STRING + + sh->sender = strdup(sender); sh->cb_signal = cb_signal; sh->data = data; + sh->delete_me = 0; dbus_error_init(&err); dbus_bus_add_match(conn->conn, match, NULL); - ecore_list_append(signal_handlers, sh); + + if (!conn->signal_handlers) + { + conn->signal_handlers = ecore_list_new(); + ecore_list_free_cb_set + (conn->signal_handlers, ECORE_FREE_CB(e_dbus_signal_handler_free)); + conn->signal_dispatcher = ecore_event_handler_add + (E_DBUS_EVENT_SIGNAL, cb_signal_event, conn); + } /* if we have a sender, and it is not a unique name, we need to know the unique name to match since signals will have the name owner as ther sender. */ if (sender && sender[0] != ':') - e_dbus_get_name_owner(conn, sender, cb_name_owner, sh); - + { + struct cb_name_owner_data *data; + data = malloc(sizeof(*data)); + if (!data) + { + e_dbus_signal_handler_free(sh); + return NULL; + } + data->conn = conn; + data->sh = sh; + e_dbus_get_name_owner(conn, sender, cb_name_owner, data); + } + + ecore_list_append(conn->signal_handlers, sh); return sh; } @@ -178,7 +223,7 @@ EAPI void e_dbus_signal_handler_del(E_DBus_Connection *conn, E_DBus_Signal_Handler *sh) { char match[DBUS_MAXIMUM_MATCH_RULE_LENGTH]; - int len = 0; + int len, sender_len, path_len, interface_len, member_len; sh->delete_me = 1; if (e_dbus_idler_active) @@ -188,43 +233,38 @@ e_dbus_signal_handler_del(E_DBus_Connection *conn, E_DBus_Signal_Handler *sh) } strcpy(match, "type='signal'"); - len = 13; - + len = sizeof("type='signal'") - 1; + +#define ADD_MATCH_PIECE(PIECE) \ + do { \ + PIECE ## _len = sh->PIECE ? strlen(sh->PIECE) : 0; \ + if (!_match_append(match, sizeof(match), &len, #PIECE, sizeof(#PIECE) - 1, sh->PIECE, PIECE ## _len)) \ + return; \ + } while (0) + + ADD_MATCH_PIECE(sender); + ADD_MATCH_PIECE(path); + ADD_MATCH_PIECE(interface); + ADD_MATCH_PIECE(member); #undef ADD_MATCH_PIECE -#define ADD_MATCH_PIECE(PIECE) \ - if (PIECE) \ - {\ - len += strlen("," #PIECE "=''") + strlen(PIECE);\ - if (len >= sizeof(match)) return;\ - strcat(match, "," #PIECE "='");\ - strcat(match, PIECE);\ - strcat(match, "'");\ - } - - ADD_MATCH_PIECE(sh->sender) - ADD_MATCH_PIECE(sh->path) - ADD_MATCH_PIECE(sh->interface) - ADD_MATCH_PIECE(sh->member) dbus_bus_remove_match(conn->conn, match, NULL); - if (!ecore_list_goto(signal_handlers, sh)) return; - ecore_list_remove(signal_handlers); - free(sh->sender); - free(sh->path); - free(sh->interface); - free(sh->member); - free(sh); + if (!conn->signal_handlers) return; + if (!ecore_list_goto(conn->signal_handlers, sh)) return; + ecore_list_remove(conn->signal_handlers); + e_dbus_signal_handler_free(sh); } static int cb_signal_event(void *data, int type, void *event) { + E_DBus_Connection *conn = data; DBusMessage *msg = event; E_DBus_Signal_Handler *sh; - ecore_list_first_goto(signal_handlers); - while ((sh = ecore_list_next(signal_handlers))) + ecore_list_first_goto(conn->signal_handlers); + while ((sh = ecore_list_next(conn->signal_handlers))) { if ((!sh->cb_signal) || (sh->delete_me)) continue; @@ -245,8 +285,9 @@ e_dbus_signal_handlers_clean(E_DBus_Connection *conn) E_DBus_Signal_Handler *sh; if (!e_dbus_handler_deletions) return; - ecore_list_first_goto(signal_handlers); - while ((sh = ecore_list_next(signal_handlers))) + if (!conn->signal_handlers) return; + ecore_list_first_goto(conn->signal_handlers); + while ((sh = ecore_list_next(conn->signal_handlers))) { if (sh->delete_me) e_dbus_signal_handler_del(conn, sh); -- 2.7.4