use hash tables in ping.[ch] instead of a list. we're pinging every window, not just windows youre trying to close, so don't use datastructures that suck with lots of windows..

openbox/event.c

@@ -769,7 +769,7 @@ static void event_handle_root(XEvent *e)
             else if (e->xclient.data.l[0] == 3)
                 ob_exit(0);
         } else if (msgtype == prop_atoms.wm_protocols) {
-            if (e->xclient.data.l[0] == prop_atoms.net_wm_ping)
+            if ((Atom)e->xclient.data.l[0] == prop_atoms.net_wm_ping)
                 ping_got_pong(e->xclient.data.l[1]);
         }
         break;

openbox/openbox.c

@@ -43,6 +43,7 @@
 #include "grab.h"
 #include "group.h"
 #include "config.h"
+#include "ping.h"
 #include "mainloop.h"
 #include "gettext.h"
 #include "parser/parse.h"
@@ -301,6 +302,7 @@ gint main(gint argc, gchar **argv)
             screen_startup(reconfigure);
             grab_startup(reconfigure);
             group_startup(reconfigure);
+            ping_startup(reconfigure);
             client_startup(reconfigure);
             dock_startup(reconfigure);
             moveresize_startup(reconfigure);
@@ -360,6 +362,7 @@ gint main(gint argc, gchar **argv)
             moveresize_shutdown(reconfigure);
             dock_shutdown(reconfigure);
             client_shutdown(reconfigure);
+            ping_shutdown(reconfigure);
             group_shutdown(reconfigure);
             grab_shutdown(reconfigure);
             screen_shutdown(reconfigure);

openbox/ping.c

@@ -33,8 +33,8 @@ typedef struct _ObPingTarget
     gint waiting;
 } ObPingTarget;
 
-static GSList *ping_targets = NULL;
+static GHashTable *ping_targets = NULL;
-static gboolean active = FALSE;
+static GHashTable *ping_ids     = NULL;
 static guint32     ping_next_id = 1;
 
 #define PING_TIMEOUT (G_USEC_PER_SEC * 3)
@@ -45,34 +45,47 @@ static void ping_send(ObPingTarget *t);
 static void     ping_end(ObClient *client, gpointer data);
 static gboolean ping_timeout(gpointer data);
 
+void ping_startup(gboolean reconfigure)
+{
+    if (reconfigure) return;
+
+    ping_targets = g_hash_table_new(g_direct_hash, g_int_equal);
+    ping_ids = g_hash_table_new(g_direct_hash, g_int_equal);
+
+    /* listen for clients to disappear */
+    client_add_destroy_notify(ping_end, NULL);
+}
+
+void ping_shutdown(gboolean reconfigure)
+{
+    if (reconfigure) return;
+
+    g_hash_table_unref(ping_targets);
+    g_hash_table_unref(ping_ids);
+
+    client_remove_destroy_notify(ping_end);
+}
+
 void ping_start(struct _ObClient *client, ObPingEventHandler h)
 {
-    GSList *it;
     ObPingTarget *t;
 
     g_assert(client->ping == TRUE);
 
     /* make sure we're not already pinging it */
-    for (it = ping_targets; it != NULL; it = g_slist_next(it)) {
+    g_assert(g_hash_table_lookup(ping_targets, &client) == NULL);
-        t = it->data;
-        if (t->client == client) return;
-    }
 
-    t = g_new(ObPingTarget, 1);
+    t = g_new0(ObPingTarget, 1);
     t->client = client;
     t->h = h;
-    t->waiting = 1; /* first wait for a reply */
 
-    ping_send(t);
+    g_hash_table_insert(ping_targets, &t->client, t);
-    ping_targets = g_slist_prepend(ping_targets, t);
+
     ob_main_loop_timeout_add(ob_main_loop, PING_TIMEOUT, ping_timeout,
                              t, g_direct_equal, NULL);
-
+    /* act like we just timed out immediately, to start the pinging process
-    if (!active) {
+       now instead of after the first delay */
-        active = TRUE;
+    ping_timeout(t);
-        /* listen for the client to disappear */
-        client_add_destroy_notify(ping_end, NULL);
-    }
 }
 
 void ping_stop(struct _ObClient *c)
@@ -82,13 +95,9 @@ void ping_stop(struct _ObClient *c)
 
 void ping_got_pong(guint32 id)
 {
-    GSList *it;
     ObPingTarget *t;
 
-    /* make sure we're not already pinging it */
+    if ((t = g_hash_table_lookup(ping_ids, &id))) {
-    for (it = ping_targets; it != NULL; it = g_slist_next(it)) {
-        t = it->data;
-        if (t->id == id) {
         /*g_print("-PONG: '%s' (id %u)\n", t->client->title, t->id);*/
         if (t->waiting > PING_TIMEOUT_WARN) {
             /* we had notified that they weren't responding, so now we
@@ -96,17 +105,25 @@ void ping_got_pong(guint32 id)
             t->h(t->client, FALSE);
         }
         t->waiting = 0; /* not waiting for a reply anymore */
-            break;
     }
-    }
+    else
-
-    if (it == NULL)
         ob_debug("Got PONG with id %u but not waiting for one\n", id);
 }
 
 static void ping_send(ObPingTarget *t)
 {
-    t->id = ping_next_id++;
+    /* t->id is 0 when it hasn't been assigned an id ever yet.
+       we can reuse ids when t->waiting == 0, because we won't be getting a
+       pong for that id in the future again.  that way for apps that aren't
+       timing out we don't need to remove/add them from/to the hash table */
+    if (t->id == 0 || t->waiting > 0) {
+        /* pick an id, and reinsert in the hash table with the new id */
+        if (t->id) g_hash_table_remove(ping_ids, &t->id);
+        t->id = ping_next_id;
+        if (++ping_next_id == 0) ++ping_next_id; /* skip 0 on wraparound */
+        g_hash_table_insert(ping_ids, &t->id, t);
+    }
+
     /*g_print("+PING: '%s' (id %u)\n", t->client->title, t->id);*/
     PROP_MSG_TO(t->client->window, t->client->window, wm_protocols,
                 prop_atoms.net_wm_ping, t->id, t->client->window, 0, 0,
@@ -117,11 +134,9 @@ static gboolean ping_timeout(gpointer data)
 {
     ObPingTarget *t = data;
 
-    if (t->waiting == 0) { /* got a reply already */
-        /* send another ping to make sure it's still alive */
     ping_send(t);
-    }
 
+    /* if the client hasn't been responding then do something about it */
     if (t->waiting == PING_TIMEOUT_WARN)
         t->h(t->client, TRUE); /* notify that the client isn't responding */
 
@@ -132,23 +147,15 @@ static gboolean ping_timeout(gpointer data)
 
 static void ping_end(ObClient *client, gpointer data)
 {
-    GSList *it;
     ObPingTarget *t;
 
-    for (it = ping_targets; it != NULL; it = g_slist_next(it)) {
+    t = g_hash_table_lookup(ping_targets, &client);
-        t = it->data;
+    g_assert(t);
-        if (t->client == client) {
-            ping_targets = g_slist_remove_link(ping_targets, it);
-            ob_main_loop_timeout_remove_data(ob_main_loop, ping_timeout, t,
-                                             FALSE);
-            g_free(t);
-            break;
-        }
-    }
 
-    /* stop listening if we're not waiting for any more pings */
+    g_hash_table_remove(ping_targets, &t->client);
-    if (!ping_targets) {
+    g_hash_table_remove(ping_ids, &t->id);
-        active = FALSE;
+
-        client_remove_destroy_notify(ping_end);
+    ob_main_loop_timeout_remove_data(ob_main_loop, ping_timeout, t, FALSE);
-    }    
+
+    g_free(t);
 }

openbox/ping.h

@@ -33,6 +33,9 @@ struct _ObClient;
 */
 typedef void (*ObPingEventHandler) (struct _ObClient *c, gboolean dead);
 
+void ping_startup(gboolean reconfigure);
+void ping_shutdown(gboolean reconfigure);
+
 void ping_start(struct _ObClient *c, ObPingEventHandler h);
 void ping_stop(struct _ObClient *c);