Refactor linked-list operations into a header

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>

src/common.h

@@ -393,10 +393,7 @@ typedef struct session {
 	/// A hash table of all windows.
 	win *windows;
 	/// Windows in their stacking order
-	win *window_stack;
+	struct list_node window_stack;
-	/// Pointer to the `next` field of the bottom most window,
-	/// or a pointer to `window_stack` when there is no window
-	win **window_stack_bottom;
 	/// Pointer to <code>win</code> of current active window. Used by
 	/// EWMH <code>_NET_ACTIVE_WINDOW</code> focus detection. In theory,
 	/// it's more reliable to store the window ID directly here, just in

src/compton.c

@@ -52,6 +52,7 @@
 #include "event.h"
 #include "options.h"
 #include "uthash_extra.h"
+#include "list.h"
 
 /// Get session_t pointer from a pointer to a member of session_t
 #define session_ptr(ptr, member)                                                         \
@@ -437,7 +438,7 @@ static win *paint_preprocess(session_t *ps, bool *fade_running) {
 	ps->fade_time += steps * ps->o.fade_delta;
 
 	// First, let's process fading
-	WIN_STACK_ITER(ps, w) {
+	list_foreach_safe(win, w, &ps->window_stack, stack_neighbour) {
 		const winmode_t mode_old = w->mode;
 		const bool was_painted = w->to_paint;
 		const double opacity_old = w->opacity;
@@ -490,7 +491,7 @@ static win *paint_preprocess(session_t *ps, bool *fade_running) {
 	// Track whether it's the highest window to paint
 	bool is_highest = true;
 	bool reg_ignore_valid = true;
-	WIN_STACK_ITER(ps, w) {
+	list_foreach(win, w, &ps->window_stack, stack_neighbour) {
 		__label__ skip_window;
 		bool to_paint = true;
 		// w->to_paint remembers whether this window is painted last time
@@ -650,12 +651,10 @@ static void rebuild_shadow_exclude_reg(session_t *ps) {
 static void restack_win(session_t *ps, win *w, xcb_window_t new_above) {
 	xcb_window_t old_above;
 
-	if (w->next) {
+	if (!list_node_is_last(&ps->window_stack, &w->stack_neighbour)) {
-		old_above = w->next->id;
+		old_above = list_next_entry(w, stack_neighbour)->id;
-		assert(&w->next != ps->window_stack_bottom);
 	} else {
 		old_above = XCB_NONE;
-		assert(&w->next == ps->window_stack_bottom);
 	}
 	log_debug("Restack %#010x (%s), old_above: %#010x, new_above: %#010x", w->id,
 	          w->name, old_above, new_above);
@@ -663,44 +662,30 @@ static void restack_win(session_t *ps, win *w, xcb_window_t new_above) {
 	if (old_above != new_above) {
 		w->reg_ignore_valid = false;
 		rc_region_unref(&w->reg_ignore);
-		if (w->next) {
+		if (!list_node_is_last(&ps->window_stack, &w->stack_neighbour)) {
-			w->next->reg_ignore_valid = false;
+			auto next_w = list_next_entry(w, stack_neighbour);
-			rc_region_unref(&w->next->reg_ignore);
+			next_w->reg_ignore_valid = false;
+			rc_region_unref(&next_w->reg_ignore);
 		}
 
-		win **prev = NULL, *tmp_w;
+		struct list_node *new_next;
+		if (!new_above) {
+			new_next = &ps->window_stack;
+		} else {
+			win *tmp_w = NULL;
 			HASH_FIND_INT(ps->windows, &new_above, tmp_w);
 
-		if (new_above && !tmp_w) {
+			if (!tmp_w) {
-			log_error("(%#010x, %#010x): Failed to found new above window.",
+				log_error("(%#010x, %#010x): Failed to found new above "
+				          "window.",
 				          w->id, new_above);
 				return;
 			}
 
-		// Unlink from old position
+			new_next = &tmp_w->stack_neighbour;
-		*w->prev = w->next;
-		if (w->next) {
-			w->next->prev = w->prev;
-		}
-		if (ps->window_stack_bottom == &w->next) {
-			ps->window_stack_bottom = w->prev;
 		}
 
-		if (!new_above) {
+		list_move_before(&w->stack_neighbour, new_next);
-			*ps->window_stack_bottom = w;
-			w->prev = ps->window_stack_bottom;
-			ps->window_stack_bottom = &w->next;
-			w->next = NULL;
-		} else {
-			prev = tmp_w->prev;
-			// Link to new position
-			w->next = *prev;
-			if (w->next) {
-				w->next->prev = &w->next;
-			}
-			w->prev = prev;
-			*w->prev = w;
-		}
 
 		// add damage for this window
 		add_damage_from_win(ps, w);
@@ -720,7 +705,7 @@ static void restack_win(session_t *ps, win *w, xcb_window_t new_above) {
 
 /// Free up all the images and deinit the backend
 static void destroy_backend(session_t *ps) {
-	WIN_STACK_ITER(ps, w) {
+	list_foreach_safe(win, w, &ps->window_stack, stack_neighbour) {
 		// Wrapping up fading in progress
 		win_skip_fading(ps, &w);
 
@@ -808,8 +793,11 @@ void configure_root(session_t *ps, int width, int height) {
 	ps->damage = ps->damage_ring + ps->ndamage - 1;
 
 	// Invalidate reg_ignore from the top
-	rc_region_unref(&ps->window_stack->reg_ignore);
+	if (!list_is_empty(&ps->window_stack)) {
-	ps->window_stack->reg_ignore_valid = false;
+		auto top_w = list_entry(ps->window_stack.next, win, stack_neighbour);
+		rc_region_unref(&top_w->reg_ignore);
+		top_w->reg_ignore_valid = false;
+	}
 
 #ifdef CONFIG_OPENGL
 	// GLX root change callback
@@ -904,7 +892,7 @@ void circulate_win(session_t *ps, xcb_circulate_notify_event_t *ce) {
 		return;
 
 	if (ce->place == PlaceOnTop) {
-		new_above = ps->window_stack->id;
+		new_above = list_entry(ps->window_stack.next, win, stack_neighbour)->id;
 	} else {
 		new_above = XCB_NONE;
 	}
@@ -1752,7 +1740,6 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 #ifdef CONFIG_DBUS
 	    .dbus_data = NULL,
 #endif
-	    .window_stack = NULL,
 	};
 
 	auto stderr_logger = stderr_logger_new();
@@ -1765,7 +1752,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 	// Allocate a session and copy default values into it
 	session_t *ps = cmalloc(session_t);
 	*ps = s_def;
-	ps->window_stack_bottom = &ps->window_stack;
+	list_init_head(&ps->window_stack);
 	ps->loop = EV_DEFAULT;
 	pixman_region32_init(&ps->screen_reg);
 
@@ -2144,7 +2131,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 
 		free(reply);
 		log_trace("Initial stack:");
-		WIN_STACK_ITER(ps, w) {
+		list_foreach(win, w, &ps->window_stack, stack_neighbour) {
 			log_trace("%#010x \"%s\"", w->id, w->name);
 		}
 	}
@@ -2195,7 +2182,7 @@ static void session_destroy(session_t *ps) {
 
 	// Free window linked list
 
-	WIN_STACK_ITER(ps, w) {
+	list_foreach_safe(win, w, &ps->window_stack, stack_neighbour) {
 		if (w->state != WSTATE_DESTROYING) {
 			win_ev_stop(ps, w);
 			HASH_DEL(ps->windows, w);
@@ -2204,8 +2191,7 @@ static void session_destroy(session_t *ps) {
 		free_win_res(ps, w);
 		free(w);
 	}
-	ps->window_stack = NULL;
+	list_init_head(&ps->window_stack);
-	ps->window_stack_bottom = &ps->window_stack;
 
 	// Free blacklists
 	free_wincondlst(&ps->o.shadow_blacklist);

src/dbus.c

@@ -24,9 +24,10 @@
 #include "log.h"
 #include "string_utils.h"
 #include "types.h"
+#include "uthash_extra.h"
 #include "utils.h"
 #include "win.h"
-#include "uthash_extra.h"
+#include "list.h"
 
 #include "dbus.h"
 
@@ -760,7 +761,10 @@ static bool cdbus_process_win_get(session_t *ps, DBusMessage *msg) {
 
 	// next
 	if (!strcmp("next", target)) {
-		cdbus_reply_wid(ps, msg, (w->next ? w->next->id : 0));
+		cdbus_reply_wid(ps, msg,
+		                (list_node_is_last(&ps->window_stack, &w->stack_neighbour)
+		                     ? 0
+		                     : list_next_entry(w, stack_neighbour)->id));
 		return true;
 	}
 

src/list.h

@@ -0,0 +1,99 @@
+#pragma once
+#include <stdbool.h>
+#include <stddef.h>
+
+/**
+ * container_of - cast a member of a structure out to the containing structure
+ * @ptr:	the pointer to the member.
+ * @type:	the type of the container struct this is embedded in.
+ * @member:	the name of the member within the struct.
+ *
+ */
+#define container_of(ptr, type, member)                                                  \
+	({                                                                               \
+		const __typeof__(((type *)0)->member) *__mptr = (ptr);                   \
+		(type *)((char *)__mptr - offsetof(type, member));                       \
+	})
+
+struct list_node {
+	struct list_node *next, *prev;
+};
+
+#define list_entry(ptr, type, node) container_of(ptr, type, node)
+#define list_next_entry(ptr, node) list_entry((ptr)->node.next, __typeof__(*(ptr)), node)
+#define list_prev_entry(ptr, node) list_entry((ptr)->node.prev, __typeof__(*(ptr)), node)
+
+/// Insert a new node between two adjacent nodes in the list
+static inline void
+__list_insert_between(struct list_node *restrict prev, struct list_node *restrict next,
+                      struct list_node *restrict new_) {
+	new_->prev = prev;
+	new_->next = next;
+	next->prev = new_;
+	prev->next = new_;
+}
+
+/// Insert a new node after `curr`
+static inline void
+list_insert_after(struct list_node *restrict curr, struct list_node *restrict new_) {
+	__list_insert_between(curr, curr->next, new_);
+}
+
+/// Insert a new node before `curr`
+static inline void
+list_insert_before(struct list_node *restrict curr, struct list_node *restrict new_) {
+	__list_insert_between(curr->prev, curr, new_);
+}
+
+/// Link two nodes in the list, so `next` becomes the successor node of `prev`
+static inline void __list_link(struct list_node *prev, struct list_node *next) {
+	next->prev = prev;
+	prev->next = next;
+}
+
+/// Remove a node from the list
+static inline void list_remove(struct list_node *to_remove) {
+	__list_link(to_remove->prev, to_remove->next);
+	to_remove->prev = (void *)-1;
+	to_remove->next = (void *)-2;
+}
+
+/// Move `to_move` so that it's before `new_next`
+static inline void
+list_move_before(struct list_node *restrict to_move, struct list_node *restrict new_next) {
+	list_remove(to_move);
+	list_insert_before(new_next, to_move);
+}
+
+/// Initialize a list node that's intended to be the head node
+static inline void list_init_head(struct list_node *head) {
+	head->next = head->prev = head;
+}
+
+/// Return true if head is the only node in the list. Under usual circumstances this means
+/// the list is empty
+static inline bool list_is_empty(const struct list_node *head) {
+	return head->prev == head;
+}
+
+/// Return true if `to_check` is the first node in list headed by `head`
+static inline bool
+list_node_is_first(const struct list_node *head, const struct list_node *to_check) {
+	return head->next == to_check;
+}
+
+/// Return true if `to_check` is the last node in list headed by `head`
+static inline bool
+list_node_is_last(const struct list_node *head, const struct list_node *to_check) {
+	return head->prev == to_check;
+}
+
+#define list_foreach(type, i, head, member)                                              \
+	for (type *i = list_entry((head)->next, type, member); &i->member != (head);     \
+	     i = list_next_entry(i, member))
+
+/// Like list_for_each, but it's safe to remove the current list node from the list
+#define list_foreach_safe(type, i, head, member)                                         \
+	for (type *i = list_entry((head)->next, type, member),                           \
+	          *__tmp = list_next_entry(i, member);                                   \
+	     &i->member != (head); i = __tmp, __tmp = list_next_entry(i, member))

src/opengl.c

@@ -221,7 +221,7 @@ void glx_destroy(session_t *ps) {
 		return;
 
 	// Free all GLX resources of windows
-	WIN_STACK_ITER(ps, w) {
+	list_foreach(win, w, &ps->window_stack, stack_neighbour) {
 		free_win_res_glx(ps, w);
 	}
 

src/win.c

@@ -28,6 +28,7 @@
 #include "uthash_extra.h"
 #include "utils.h"
 #include "x.h"
+#include "list.h"
 
 #ifdef CONFIG_DBUS
 #include "dbus.h"
@@ -57,7 +58,7 @@
  * Clear leader cache of all windows.
  */
 static inline void clear_cache_win_leaders(session_t *ps) {
-	WIN_STACK_ITER(ps, w) {
+	list_foreach(win, w, &ps->window_stack, stack_neighbour) {
 		w->cache_leader = XCB_NONE;
 	}
 }
@@ -918,7 +919,6 @@ void add_win(session_t *ps, xcb_window_t id, xcb_window_t prev) {
 	    .invert_color_force = UNSET,
 
 	    // Initialized in this function
-	    .next = NULL,
 	    .id = XCB_NONE,
 	    .a = {0},
 	    .pictfmt = NULL,
@@ -1025,24 +1025,16 @@ void add_win(session_t *ps, xcb_window_t id, xcb_window_t prev) {
 	}
 
 	// Find window insertion point
-	win **p = NULL;
+	struct list_node *p = NULL;
 	if (prev) {
 		win *w = NULL;
 		HASH_FIND_INT(ps->windows, &prev, w);
 		assert(w);
-		p = w->prev;
+		p = w->stack_neighbour.prev;
 	} else {
 		p = &ps->window_stack;
 	}
-	new->next = *p;
+	list_insert_after(p, &new->stack_neighbour);
-	if (new->next) {
-		new->next->prev = &new->next;
-	}
-	new->prev = p;
-	*p = new;
-	if (p == ps->window_stack_bottom) {
-		ps->window_stack_bottom = &new->next;
-	}
 	HASH_ADD_INT(ps->windows, id, new);
 
 #ifdef CONFIG_DBUS
@@ -1427,7 +1419,7 @@ void win_update_frame_extents(session_t *ps, win *w, xcb_window_t client) {
 }
 
 bool win_is_region_ignore_valid(session_t *ps, const win *w) {
-	WIN_STACK_ITER(ps, i) {
+	list_foreach(win, i, &ps->window_stack, stack_neighbour) {
 		if (i == w)
 			break;
 		if (!i->reg_ignore_valid)
@@ -1487,19 +1479,14 @@ static void finish_destroy_win(session_t *ps, win **_w) {
 	//      paint happened at least once, w->reg_ignore_valid would
 	//      be true, and there is no need to invalid w->next->reg_ignore
 	//      when w is destroyed.
-	if (w->next) {
+	if (!list_node_is_last(&ps->window_stack, &w->stack_neighbour)) {
-		rc_region_unref(&w->next->reg_ignore);
+		auto next_w = list_next_entry(w, stack_neighbour);
-		w->next->reg_ignore_valid = false;
+		rc_region_unref(&next_w->reg_ignore);
+		next_w->reg_ignore_valid = false;
 	}
 
 	log_trace("Trying to destroy (%#010x)", w->id);
-	*w->prev = w->next;
+	list_remove(&w->stack_neighbour);
-	if (w->next) {
-		w->next->prev = w->prev;
-	}
-	if (&w->next == ps->window_stack_bottom) {
-		ps->window_stack_bottom = w->prev;
-	}
 
 	if (w == ps->active_win) {
 		ps->active_win = NULL;
@@ -1510,7 +1497,7 @@ static void finish_destroy_win(session_t *ps, win **_w) {
 	// Drop w from all prev_trans to avoid accessing freed memory in
 	// repair_win()
 	// TODO there can only be one prev_trans pointing to w
-	WIN_STACK_ITER(ps, w2) {
+	list_foreach(win, w2, &ps->window_stack, stack_neighbour) {
 		if (w == w2->prev_trans) {
 			w2->prev_trans = NULL;
 		}

src/win.h

@@ -22,9 +22,7 @@
 #include "types.h"
 #include "utils.h"
 #include "x.h"
-
+#include "list.h"
-#define WIN_STACK_ITER(ps, w)                                                            \
-	for (win *w = ps->window_stack, *next; w ? (next = w->next, true) : false; w = next)
 
 typedef struct session session_t;
 typedef struct _glx_texture glx_texture_t;
@@ -131,14 +129,11 @@ typedef enum win_flags {
 /// Structure representing a top-level window compton manages.
 typedef struct win win;
 struct win {
+	struct list_node stack_neighbour;
 	/// backend data attached to this window. Only available when
 	/// `state` is not UNMAPPED
 	void *win_image;
 	void *shadow_image;
-	/// Pointer to the next lower window in window stack.
-	win *next;
-	/// Pointer to a linked-list pointer that points to this window.
-	win **prev;
 	/// Pointer to the next higher window to paint.
 	win *prev_trans;
 	// TODO rethink reg_ignore