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

diff --git a/src/lib/dbus/e_dbus.c b/src/lib/dbus/e_dbus.c
index 2e7a49a..fd896d2 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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);