A working callback mechanism between C and Scheme code.

Callbacks can be fully setup via Scheme code; the C code assumes
callbacks have no args and return no values. Within Scheme we setup
callbacks via some lambda functions that capture args when needed (see
make-callback example).

Key press handler operates on the C side using a hash map to find the
callback to be called. Everything is set up on the Scheme side, which
is less verbose and more convenient to read.

* main.scm: Setup callbacks for keypresses
* schewm.c: Callbacks hash maps for keys and buttons, handlers for
keys
* wm.scm: Minor reorganization (key grab also now takes the callback
directly)

3 files changed, 330 insertions, 110 deletions

diff --git a/main.scm b/main.scm
index 827c911..77aedc3 100644
--- a/main.scm
+++ b/main.scm
@@ -19,6 +19,9 @@
    "#0d131a" ; outer-color
    ))
 
+(define (make-callback proc arg)
+  (lambda () (proc arg)))
+
 (define wm-keybindings
   `((,(make-shift-key "q") . ,wm-quit)
     (,(make-key "q") . ,wm-focus-close)
@@ -47,25 +50,25 @@
     (,(make-key "z")     . ,wm-toggle-bottom-left)
     (,(make-key "c")     . ,wm-toggle-bottom-right)
     ;; Set current workspace
-    (,(make-key "1") . ,wm-set-workspace)
-    (,(make-key "2") . ,wm-set-workspace)
-    (,(make-key "3") . ,wm-set-workspace)
-    (,(make-key "4") . ,wm-set-workspace)
-    (,(make-key "5") . ,wm-set-workspace)
-    (,(make-key "6") . ,wm-set-workspace)
-    (,(make-key "7") . ,wm-set-workspace)
-    (,(make-key "8") . ,wm-set-workspace)
-    (,(make-key "9") . ,wm-set-workspace)
+    (,(make-key "1") . ,(make-callback wm-set-workspace 1))
+    (,(make-key "2") . ,(make-callback wm-set-workspace 2))
+    (,(make-key "3") . ,(make-callback wm-set-workspace 3))
+    (,(make-key "4") . ,(make-callback wm-set-workspace 4))
+    (,(make-key "5") . ,(make-callback wm-set-workspace 5))
+    (,(make-key "6") . ,(make-callback wm-set-workspace 6))
+    (,(make-key "7") . ,(make-callback wm-set-workspace 7))
+    (,(make-key "8") . ,(make-callback wm-set-workspace 8))
+    (,(make-key "9") . ,(make-callback wm-set-workspace 9))
     ;; Send client to workspace
-    (,(make-shift-key "1") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "2") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "3") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "4") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "5") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "6") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "7") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "8") . ,wm-set-focused-client-workspace)
-    (,(make-shift-key "9") . ,wm-set-focused-client-workspace)
+    (,(make-shift-key "1") . ,(make-callback wm-set-focused-client-workspace 1))
+    (,(make-shift-key "2") . ,(make-callback wm-set-focused-client-workspace 2))
+    (,(make-shift-key "3") . ,(make-callback wm-set-focused-client-workspace 3))
+    (,(make-shift-key "4") . ,(make-callback wm-set-focused-client-workspace 4))
+    (,(make-shift-key "5") . ,(make-callback wm-set-focused-client-workspace 5))
+    (,(make-shift-key "6") . ,(make-callback wm-set-focused-client-workspace 6))
+    (,(make-shift-key "7") . ,(make-callback wm-set-focused-client-workspace 7))
+    (,(make-shift-key "8") . ,(make-callback wm-set-focused-client-workspace 8))
+    (,(make-shift-key "9") . ,(make-callback wm-set-focused-client-workspace 9))
     ;; Send client to monitor
     (,(make-shift-key "i") . ,wm-client-monitor-prev)
     (,(make-shift-key "o") . ,wm-client-monitor-next)))
@@ -76,5 +79,4 @@
    (lambda (mod keysym)
      (display (list mod keysym))
      (newline)))
-  (wm-run)
-  (wm-quit))
+  (wm-run))
diff --git a/schewm.c b/schewm.c
index 49f2e9d..44bb841 100644
--- a/schewm.c
+++ b/schewm.c
@@ -1,3 +1,4 @@
+#include <assert.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdint.h>
@@ -260,9 +261,18 @@ client_list_free(struct ClientList *list, bool deep) {
     }
 }
 
-// xcb_window_t is uint32_t
+/*
+ * The following hash functions come from [1].
+ *
+ * Apparently supposed to be overall decent, no independent tests were
+ * made here. We have 32-bit and 64-bit versions, as we need hash
+ * tables for both data types.
+ *
+ * [1] https://stackoverflow.com/a/12996028
+ */
+
 uint32_t
-wid_hash(uint32_t x) {
+u32_hash(uint32_t x) {
     x = ((x >> 16) ^ x) * 0x45d9f3b;
     x = ((x >> 16) ^ x) * 0x45d9f3b;
     x = (x >> 16) ^ x;
@@ -270,59 +280,76 @@ wid_hash(uint32_t x) {
 }
 
 uint32_t
-wid_unhash(uint32_t x) {
+u32_unhash(uint32_t x) {
     x = ((x >> 16) ^ x) * 0x119de1f3;
     x = ((x >> 16) ^ x) * 0x119de1f3;
     x = (x >> 16) ^ x;
     return x;
 }
 
+// We assume this works as well as for 32 bits, but who knows?
+uint64_t
+u64_hash(uint64_t x) {
+    x = (x ^ (x >> 30)) * UINT64_C(0xbf58476d1ce4e5b9);
+    x = (x ^ (x >> 27)) * UINT64_C(0x94d049bb133111eb);
+    x = x ^ (x >> 31);
+    return x;
+}
+
+uint64_t
+u64_unhash(uint64_t x) {
+    x = (x ^ (x >> 31) ^ (x >> 62)) * UINT64_C(0x319642b2d24d8ec3);
+    x = (x ^ (x >> 27) ^ (x >> 54)) * UINT64_C(0x96de1b173f119089);
+    x = x ^ (x >> 30) ^ (x >> 60);
+    return x;
+}
+
 struct ClientsMap {
-    struct ClientList **table;
-    size_t table_size;
+    struct ClientList **buckets;
+    size_t num_buckets;
     size_t size;
 };
 
 struct ClientsMap *
-clients_map_new(size_t size) {
+clients_map_new(size_t num_buckets) {
     struct ClientsMap *map = calloc(1, sizeof(struct ClientsMap));
     if (!map) {
         return NULL;
     }
 
-    map->table = calloc(size, sizeof(struct ClientList *));
-    if (!map->table) {
+    map->buckets = calloc(num_buckets, sizeof(struct ClientList *));
+    if (!map->buckets) {
         free(map);
         return NULL;
     }
 
-    map->table_size = size;
+    map->num_buckets = num_buckets;
     map->size = 0;
     return map;
 }
 
 void
-clients_map_add(struct ClientsMap *clients, struct Client *client) {
-    uint32_t hash = wid_hash(client->id);
-    uint32_t i = hash % clients->table_size;
-    // collision if (clients->table[i] != NULL)
-    clients->table[i] = client_list_add(clients->table[i], client);
-    clients->size++;
+clients_map_add(struct ClientsMap *map, struct Client *client) {
+    uint32_t hash = u32_hash(client->id);
+    uint32_t i = hash % map->num_buckets;
+    // collision if (map->buckets[i] != NULL)
+    map->buckets[i] = client_list_add(map->buckets[i], client);
+    map->size++;
 }
 
 struct Client *
-clients_map_erase(struct ClientsMap *clients, struct Client *client) {
-    uint32_t hash = wid_hash(client->id);
-    uint32_t i = hash % clients->table_size;
+clients_map_erase(struct ClientsMap *map, struct Client *client) {
+    uint32_t hash = u32_hash(client->id);
+    uint32_t i = hash % map->num_buckets;
     struct ClientList *prev = NULL;
-    for (struct ClientList *list = clients->table[i];
+    for (struct ClientList *list = map->buckets[i];
          list != NULL;
          list = list->next) {
         if (list->client == client) {
             if (prev == NULL) {
                 // It matched the first, update the starting node of
                 // the list
-                clients->table[i] = list->next;
+                map->buckets[i] = list->next;
             }
             if (prev != NULL) {
                 // We are not at the first, make prev skip the node we
@@ -330,7 +357,7 @@ clients_map_erase(struct ClientsMap *clients, struct Client *client) {
                 prev->next = list->next;
             }
             free(list);
-            clients->size--;
+            map->size--;
             return client;
         }
         prev = list;
@@ -340,10 +367,10 @@ clients_map_erase(struct ClientsMap *clients, struct Client *client) {
 }
 
 struct Client *
-clients_map_find(const struct ClientsMap *clients, xcb_window_t id) {
-    uint32_t hash = wid_hash(id);
-    uint32_t i = hash % clients->table_size;
-    for (struct ClientList *list = clients->table[i];
+clients_map_find(const struct ClientsMap *map, xcb_window_t id) {
+    uint32_t hash = u32_hash(id);
+    uint32_t i = hash % map->num_buckets;
+    for (struct ClientList *list = map->buckets[i];
          list != NULL;
          list = list->next) {
         if (list->client->id == id) {
@@ -358,12 +385,12 @@ clients_map_find(const struct ClientsMap *clients, xcb_window_t id) {
  * This function frees the memory of clients as well.
  */
 void
-clients_map_free(struct ClientsMap *clients) {
-    for (size_t i = 0; i < clients->table_size; i++) {
-        client_list_free(clients->table[i], true);
+clients_map_free(struct ClientsMap *map) {
+    for (size_t i = 0; i < map->num_buckets; i++) {
+        client_list_free(map->buckets[i], true);
     }
-    free(clients->table);
-    free(clients);
+    free(map->buckets);
+    free(map);
 }
 
 struct Monitor {
@@ -428,6 +455,151 @@ struct Workspace {
     struct Client *ring;
 };
 
+typedef void (*Callback)();
+
+struct CallbackList {
+    uint64_t key;
+    Callback callback;
+    struct CallbackList *next;
+};
+
+/*
+ * Prepends a new callback to list of callbacks.
+ */
+struct CallbackList *
+callback_list_add(struct CallbackList *list, uint64_t key, Callback callback) {
+    struct CallbackList *node = calloc(1, sizeof(struct CallbackList));
+    if (node == NULL) {
+        // Cannot alloc node, just abort. If list was NULL, stays so.
+        return list;
+    }
+    node->key = key;
+    node->callback = callback;
+    // Also works when list is NULL
+    node->next = list;
+    list = node;
+    return list;
+}
+
+void
+callback_list_free(struct CallbackList *list) {
+    struct CallbackList *next;
+    while (list != NULL) {
+        next = list->next;
+        free(list);
+        list = next;
+    }
+}
+
+struct CallbacksMap {
+    struct CallbackList **buckets;
+    size_t num_buckets;
+    size_t size;
+};
+
+struct CallbacksMap *
+callbacks_map_new(size_t num_buckets) {
+    struct CallbacksMap *map = calloc(1, sizeof(struct CallbacksMap));
+    if (!map) {
+        return NULL;
+    }
+
+    map->buckets = calloc(num_buckets, sizeof(struct CallbackList *));
+    if (!map->buckets) {
+        free(map);
+        return NULL;
+    }
+
+    map->num_buckets = num_buckets;
+    map->size = 0;
+    return map;
+}
+
+/*
+ * For internal use only. Call callbacks_map_get() for regular usage.
+ *
+ * Returns a pointer to the callback so we can modify stuff and reuse
+ * the search in multiple other functions.
+ */
+Callback *
+callbacks_map_find(const struct CallbacksMap *map, uint64_t key) {
+    uint64_t hash = u64_hash(key);
+    uint64_t i = hash % map->num_buckets;
+    for (struct CallbackList *list = map->buckets[i];
+         list != NULL;
+         list = list->next) {
+        if (list->key == key) {
+            return &list->callback;
+        }
+    }
+    // Not found
+    return NULL;
+}
+
+Callback
+callbacks_map_get(struct CallbacksMap *map, uint64_t key) {
+    Callback *callback_ptr = callbacks_map_find(map, key);
+    if (callback_ptr == NULL) {
+        return NULL;
+    }
+    return *callback_ptr;
+}
+
+/*
+ * The semantics with set/unset is that we simply replace whatever
+ * callback was there, if something already exists with that key.
+ *
+ * This function also increases the size of the map if a new callback
+ * is added and not simply replaced.
+ */
+void
+callbacks_map_set(struct CallbacksMap *map, uint64_t key, Callback callback) {
+    Callback *callback_ptr = callbacks_map_find(map, key);
+    if (callback_ptr != NULL) {
+        if (*callback_ptr == NULL) {
+            // Increase size since there was no callback set, even
+            // though the node exists
+            map->size++;
+        }
+        *callback_ptr = callback;
+    } else {
+        uint64_t hash = u64_hash(key);
+        uint64_t i = hash % map->num_buckets;
+        map->buckets[i] = callback_list_add(map->buckets[i], key, callback);
+        map->size++;
+    }
+}
+
+/*
+ * This function replaces the existing stored callback with NULL,
+ * returning the old value, if found. It also reduces the size of the
+ * map.
+ *
+ * That means that once the map grows, it is only truly using less
+ * memory when callbacks_map_free() is called.
+ */
+Callback
+callbacks_map_unset(struct CallbacksMap *map, uint64_t key) {
+    Callback *callback_ptr = callbacks_map_find(map, key);
+    if (callback_ptr != NULL) {
+        Callback callback = *callback_ptr;
+        *callback_ptr = NULL;
+        map->size--;
+        return callback;
+    }
+    // Not found
+    return NULL;
+}
+
+void
+callbacks_map_free(struct CallbacksMap *map) {
+    for (size_t i = 0; i < map->num_buckets; i++) {
+        callback_list_free(map->buckets[i]);
+    }
+    free(map->buckets);
+    free(map);
+}
+
 static struct {
     uint16_t inner_border_width, outer_border_width, magnet_border_width;
     int16_t offset_x, offset_y;
@@ -468,6 +640,8 @@ static struct {
     struct Client *focus;
     struct ClientsMap *clients;
     struct Workspace *workspaces;
+    struct CallbacksMap *keys;
+    struct CallbacksMap *buttons;
     generic_event_handler_t events[XCB_NO_OPERATION];
 } wm;
 
@@ -1054,6 +1228,7 @@ static void
 dpy_grab_key(uint16_t mod, xcb_keysym_t keysym) {
     const xcb_setup_t *setup = xcb_get_setup(dpy.conn);
     if (!setup) {
+        assert(setup);
         return;
     }
 
@@ -1489,8 +1664,8 @@ dpy_update_window_list(const struct ClientsMap *clients) {
     size_t num_windows = 0;
     if (clients->size != 0) {
         windows = malloc(clients->size * sizeof(xcb_window_t));
-        for (size_t i = 0; i < clients->table_size; i++) {
-            for (struct ClientList *list = clients->table[i];
+        for (size_t i = 0; i < clients->num_buckets; i++) {
+            for (struct ClientList *list = clients->buckets[i];
                  list != NULL;
                  list = list->next) {
                 windows[num_windows++] = list->client->id;
@@ -1566,8 +1741,8 @@ bool wm_update_outputs();
 bool wm_has_error();
 bool wm_init();
 void wm_quit();
-void wm_grab_key_with_mod(xcb_keysym_t keysym);
-void wm_grab_key_with_mod_shift(xcb_keysym_t keysym);
+void wm_grab_key_with_mod(xcb_keysym_t keysym, Callback callback);
+void wm_grab_key_with_mod_shift(xcb_keysym_t keysym, Callback callback);
 void wm_begin_move_client();
 void wm_begin_resize_client();
 void wm_focus_close();
@@ -1941,17 +2116,37 @@ wm_get_mod_key(bool with_shift) {
 }
 
 void
-wm_grab_key_with_mod(xcb_keysym_t keysym) {
-    dpy_grab_key(wm_get_mod_key(false), keysym);
+wm_grab_key(xcb_keysym_t keysym, bool with_shift, Callback callback) {
+    /*
+     * Key grabbing is done in two stages:
+     *
+     * 1. Register a function to be called when that key combo is
+     * pressed
+     */
+    uint16_t mod = wm_get_mod_key(with_shift);
+    uint64_t map_key = make_key_chord(mod, keysym);
+    callbacks_map_set(wm.keys, map_key, callback);
+
+    /*
+     * 2. Tell the display server we are interested in a certain
+     * mod/key combo
+     */
+    dpy_grab_key(mod, keysym);
+    dpy_flush();
+}
+
+void
+wm_grab_key_with_mod(xcb_keysym_t keysym, Callback callback) {
+    wm_grab_key(keysym, false, callback);
 }
 
 void
-wm_grab_key_with_mod_shift(xcb_keysym_t keysym) {
-    dpy_grab_key(wm_get_mod_key(true), keysym);
+wm_grab_key_with_mod_shift(xcb_keysym_t keysym, Callback callback) {
+    wm_grab_key(keysym, true, callback);
 }
 
 /*
- * event handlers which take a generic event and know what to cast it into.
+ * Event handlers that know what to cast the generic event into.
  */
 static void ev_configure_request(xcb_generic_event_t *);
 static void ev_destroy_notify(xcb_generic_event_t *);
@@ -2017,6 +2212,14 @@ wm_init() {
     if (wm.clients == NULL) {
         return false;
     }
+    wm.keys = callbacks_map_new(64);
+    if (wm.keys == NULL) {
+        return false;
+    }
+    wm.buttons = callbacks_map_new(16);
+    if (wm.buttons == NULL) {
+        return false;
+    }
 
     wm.cur_workspace = 0;
     wm.num_workspaces = 10;
@@ -2079,6 +2282,8 @@ void
 wm_destroy() {
     free(wm.workspaces);
     clients_map_free(wm.clients);
+    callbacks_map_free(wm.keys);
+    callbacks_map_free(wm.buttons);
     dpy_destroy();
 }
 
@@ -2502,12 +2707,19 @@ wm_set_key_press_handler(key_press_handler_t handler) {
 
 static void
 ev_key_press(xcb_generic_event_t *generic_ev) {
+    xcb_key_press_event_t *ev = (xcb_key_press_event_t *) generic_ev;
+    uint16_t mod = dpy_clean_mod(ev->state);
+    xcb_keysym_t keysym = dpy_keysym_from_keycode(ev->detail);
+
     if (key_press_handler) {
-        xcb_key_press_event_t *ev = (xcb_key_press_event_t *) generic_ev;
-        uint16_t mod = dpy_clean_mod(ev->state);
-        xcb_keysym_t keysym = dpy_keysym_from_keycode(ev->detail);
         key_press_handler(mod, keysym);
     }
+
+    uint64_t map_key = make_key_chord(mod, keysym);
+    Callback callback = callbacks_map_get(wm.keys, map_key);
+    if (callback != NULL) {
+        callback();
+    }
 }
 
 // XCB_MAP_REQUEST
diff --git a/wm.scm b/wm.scm
index 3dd4797..a778dae 100644
--- a/wm.scm
+++ b/wm.scm
@@ -1,4 +1,5 @@
 (define-module (wm)
+  #:use-module (ice-9 match)
   #:use-module (srfi srfi-9)
   #:use-module (system foreign)
   #:export (wm-init
@@ -87,54 +88,6 @@
 (define wm-run
   (schewm-func void "wm_run" '()))
 
-(define c/keysym-from-str
-  (schewm-func uint32
-               "keysym_from_str"
-               (list '*)))
-
-(define (string->key s)
-  (c/keysym-from-str (string->pointer s)))
-
-(define (make-key key)
-  (list #f (string->key key)))
-
-(define (make-shift-key key)
-  (list #t (string->key key)))
-
-(define wm-grab-key-with-mod
-  (schewm-func void "wm_grab_key_with_mod"
-               (list uint32)))
-
-(define wm-grab-key-with-mod-shift
-  (schewm-func void "wm_grab_key_with_mod_shift"
-               (list uint32)))
-
-(define (wm-grab-keys keybindings)
-  (unless (null? keybindings)
-    (let* ((keybinding (car keybindings))
-           (chord (car keybinding))
-           (func (cdr keybinding))
-           (with-shift (car chord))
-           (key (car (cdr chord))))
-      (if with-shift
-          (wm-grab-key-with-mod-shift key)
-          (wm-grab-key-with-mod key))
-      (wm-grab-keys (cdr keybindings)))))
-
-(define c/wm-set-key-press-handler
-  (schewm-func void
-               "wm_set_key_press_handler"
-               (list '*)))
-
-(define (wm-set-key-press-handler! handler)
-  ;; Event handler args are:
-  ;; - mod (`ev.state' clean from noise, uint16)
-  ;; - keysym (`ev.detail' converted to keysym, uint32)
-  (c/wm-set-key-press-handler
-   (procedure->pointer void
-                       handler
-                       (list uint16 uint32))))
-
 (define wm-focus-prev
   (schewm-func void "wm_focus_prev" '()))
 
@@ -198,3 +151,56 @@
 
 (define wm-client-monitor-next
   (schewm-func void "wm_client_monitor_next" '()))
+
+
+(define c/keysym-from-str
+  (schewm-func uint32
+               "keysym_from_str"
+               (list '*)))
+
+(define (string->key s)
+  (c/keysym-from-str (string->pointer s)))
+
+(define (make-key key)
+  (list #f (string->key key)))
+
+(define (make-shift-key key)
+  (list #t (string->key key)))
+
+(define c/wm-grab-key-with-mod
+  (schewm-func void "wm_grab_key_with_mod"
+               (list uint32 '*)))
+
+(define (wm-grab-key-with-mod key proc)
+  (c/wm-grab-key-with-mod key (procedure->pointer void proc '())))
+
+(define c/wm-grab-key-with-mod-shift
+  (schewm-func void "wm_grab_key_with_mod_shift"
+               (list uint32 '*)))
+
+(define (wm-grab-key-with-mod-shift key proc)
+  (c/wm-grab-key-with-mod-shift key (procedure->pointer void proc '())))
+
+(define (wm-grab-keys keybindings)
+  (unless (null? keybindings)
+    (let ((keybinding (car keybindings)))
+      (match keybinding
+             (((with-shift key) . func)
+              (if with-shift
+                  (wm-grab-key-with-mod-shift key func)
+                  (wm-grab-key-with-mod key func))))
+      (wm-grab-keys (cdr keybindings)))))
+
+(define c/wm-set-key-press-handler
+  (schewm-func void
+               "wm_set_key_press_handler"
+               (list '*)))
+
+(define (wm-set-key-press-handler! handler)
+  ;; Event handler args are:
+  ;; - mod (`ev.state' clean from noise, uint16)
+  ;; - keysym (`ev.detail' converted to keysym, uint32)
+  (c/wm-set-key-press-handler
+   (procedure->pointer void
+                       handler
+                       (list uint16 uint32))))