WINGs/Makefile.am

@@ -76,7 +76,6 @@ libWUtil_la_SOURCES = 	\
 	misc.c \
 	notification.c \
 	proplist.c \
-	tree.c \
 	userdefaults.c \
 	userdefaults.h \
 	usleep.c \

WINGs/WINGs/WUtil.h

@@ -615,14 +615,12 @@ void WMSetDataFormat(WMData *aData, unsigned format);
 unsigned WMGetDataFormat(WMData *aData);
 /* Storing data */
 
-/* ---[ WINGs/tree.c ]---------------------------------------------------- */
+/* ---[ wutil-rs/src/tree.rs ]---------------------------------------------------- */
 
 /* Generic Tree and TreeNode */
 
 WMTreeNode* WMCreateTreeNode(void *data);
 
-WMTreeNode* WMCreateTreeNodeWithDestructor(void *data, WMFreeDataProc *destructor);
-
 WMTreeNode* WMInsertItemInTree(WMTreeNode *parent, int index, void *item);
 
 #define WMAddItemToTree(parent, item)  WMInsertItemInTree(parent, -1, item)
@@ -631,38 +629,19 @@ WMTreeNode* WMInsertNodeInTree(WMTreeNode *parent, int index, WMTreeNode *aNode)
 
 #define WMAddNodeToTree(parent, aNode)  WMInsertNodeInTree(parent, -1, aNode)
 
-void WMDestroyTreeNode(WMTreeNode *aNode);
-
-void WMDeleteLeafForTreeNode(WMTreeNode *aNode, int index);
-
-void WMRemoveLeafForTreeNode(WMTreeNode *aNode, void *leaf);
-
-void* WMReplaceDataForTreeNode(WMTreeNode *aNode, void *newData);
-
 void* WMGetDataForTreeNode(WMTreeNode *aNode);
 
 int WMGetTreeNodeDepth(WMTreeNode *aNode);
 
-WMTreeNode* WMGetParentForTreeNode(WMTreeNode *aNode);
-
-/* Sort only the leaves of the passed node */
-void WMSortLeavesForTreeNode(WMTreeNode *aNode, WMCompareDataProc *comparer);
-
 /* Sort all tree recursively starting from the passed node */
-void WMSortTree(WMTreeNode *aNode, WMCompareDataProc *comparer);
-
-/* Returns the first node which matches node's data with cdata by 'match' */
-WMTreeNode* WMFindInTree(WMTreeNode *aTree, WMMatchDataProc *match, void *cdata);
+void WMSortTree(WMTreeNode *aNode, int (*comparer)(const WMTreeNode *a, const WMTreeNode *b));
 
 /* Returns the first node where node's data matches cdata by 'match' and node is
  * at most `limit' depths down from `aTree'. */
-WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit);
-
-/* Returns first tree node that has data == cdata */
-#define WMGetFirstInTree(aTree, cdata) WMFindInTree(aTree, NULL, cdata)
+WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, int (*match)(const WMTreeNode *item, const void *cdata), void *cdata, int limit);
 
 /* Walk every node of aNode with `walk' */
-void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data, Bool DepthFirst);
+void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data);
 
 /* ---[ WINGs/data.c ]---------------------------------------------------- */
 

WINGs/tree.c

@@ -1,255 +0,0 @@
-
-#include <string.h>
-
-#include "WUtil.h"
-
-typedef struct W_TreeNode {
-	void *data;
-
-	/*unsigned int uflags:16; */
-
-	WMArray *leaves;
-
-	int depth;
-
-	struct W_TreeNode *parent;
-
-	WMFreeDataProc *destructor;
-} W_TreeNode;
-
-static void destroyNode(void *data)
-{
-	WMTreeNode *aNode = (WMTreeNode *) data;
-
-	if (aNode->destructor) {
-		(*aNode->destructor) (aNode->data);
-	}
-	if (aNode->leaves) {
-		WMFreeArray(aNode->leaves);
-	}
-	wfree(aNode);
-}
-
-WMTreeNode *WMCreateTreeNode(void *data)
-{
-	return WMCreateTreeNodeWithDestructor(data, NULL);
-}
-
-WMTreeNode *WMCreateTreeNodeWithDestructor(void *data, WMFreeDataProc * destructor)
-{
-	WMTreeNode *aNode;
-
-	aNode = (WMTreeNode *) wmalloc(sizeof(W_TreeNode));
-	aNode->destructor = destructor;
-	aNode->data = data;
-	aNode->parent = NULL;
-	aNode->depth = 0;
-	aNode->leaves = NULL;
-	/*aNode->leaves = WMCreateArrayWithDestructor(1, destroyNode); */
-
-	return aNode;
-}
-
-WMTreeNode *WMInsertItemInTree(WMTreeNode * parent, int index, void *item)
-{
-	WMTreeNode *aNode;
-
-	wassertrv(parent != NULL, NULL);
-
-	aNode = WMCreateTreeNodeWithDestructor(item, parent->destructor);
-	aNode->parent = parent;
-	aNode->depth = parent->depth + 1;
-	if (!parent->leaves) {
-		parent->leaves = WMCreateArrayWithDestructor(1, destroyNode);
-	}
-	if (index < 0) {
-		WMAddToArray(parent->leaves, aNode);
-	} else {
-		WMInsertInArray(parent->leaves, index, aNode);
-	}
-
-	return aNode;
-}
-
-static void updateNodeDepth(WMTreeNode * aNode, int depth)
-{
-	int i;
-
-	aNode->depth = depth;
-
-	if (aNode->leaves) {
-		for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) {
-			updateNodeDepth(WMGetFromArray(aNode->leaves, i), depth + 1);
-		}
-	}
-}
-
-WMTreeNode *WMInsertNodeInTree(WMTreeNode * parent, int index, WMTreeNode * aNode)
-{
-	wassertrv(parent != NULL, NULL);
-	wassertrv(aNode != NULL, NULL);
-
-	aNode->parent = parent;
-	updateNodeDepth(aNode, parent->depth + 1);
-	if (!parent->leaves) {
-		parent->leaves = WMCreateArrayWithDestructor(1, destroyNode);
-	}
-	if (index < 0) {
-		WMAddToArray(parent->leaves, aNode);
-	} else {
-		WMInsertInArray(parent->leaves, index, aNode);
-	}
-
-	return aNode;
-}
-
-void WMDestroyTreeNode(WMTreeNode * aNode)
-{
-	wassertr(aNode != NULL);
-
-	if (aNode->parent && aNode->parent->leaves) {
-		WMRemoveFromArray(aNode->parent->leaves, aNode);
-	} else {
-		destroyNode(aNode);
-	}
-}
-
-void WMDeleteLeafForTreeNode(WMTreeNode * aNode, int index)
-{
-	wassertr(aNode != NULL);
-	wassertr(aNode->leaves != NULL);
-
-	WMDeleteFromArray(aNode->leaves, index);
-}
-
-static int sameData(const void *item, const void *data)
-{
-	return (((WMTreeNode *) item)->data == data);
-}
-
-void WMRemoveLeafForTreeNode(WMTreeNode * aNode, void *leaf)
-{
-	int index;
-
-	wassertr(aNode != NULL);
-	wassertr(aNode->leaves != NULL);
-
-	index = WMFindInArray(aNode->leaves, sameData, leaf);
-	if (index != WANotFound) {
-		WMDeleteFromArray(aNode->leaves, index);
-	}
-}
-
-void *WMReplaceDataForTreeNode(WMTreeNode * aNode, void *newData)
-{
-	void *old;
-
-	wassertrv(aNode != NULL, NULL);
-
-	old = aNode->data;
-	aNode->data = newData;
-
-	return old;
-}
-
-void *WMGetDataForTreeNode(WMTreeNode * aNode)
-{
-	return aNode->data;
-}
-
-int WMGetTreeNodeDepth(WMTreeNode * aNode)
-{
-	return aNode->depth;
-}
-
-WMTreeNode *WMGetParentForTreeNode(WMTreeNode * aNode)
-{
-	return aNode->parent;
-}
-
-void WMSortLeavesForTreeNode(WMTreeNode * aNode, WMCompareDataProc * comparer)
-{
-	wassertr(aNode != NULL);
-
-	if (aNode->leaves) {
-		WMSortArray(aNode->leaves, comparer);
-	}
-}
-
-static void sortLeavesForNode(WMTreeNode * aNode, WMCompareDataProc * comparer)
-{
-	int i;
-
-	if (!aNode->leaves)
-		return;
-
-	WMSortArray(aNode->leaves, comparer);
-	for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) {
-		sortLeavesForNode(WMGetFromArray(aNode->leaves, i), comparer);
-	}
-}
-
-void WMSortTree(WMTreeNode * aNode, WMCompareDataProc * comparer)
-{
-	wassertr(aNode != NULL);
-
-	sortLeavesForNode(aNode, comparer);
-}
-
-static WMTreeNode *findNodeInTree(WMTreeNode * aNode, WMMatchDataProc * match, void *cdata, int limit)
-{
-	if (match == NULL && aNode->data == cdata)
-		return aNode;
-	else if (match && (*match) (aNode->data, cdata))
-		return aNode;
-
-	if (aNode->leaves && limit != 0) {
-		WMTreeNode *leaf;
-		int i;
-
-		for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) {
-			leaf = findNodeInTree(WMGetFromArray(aNode->leaves, i),
-					      match, cdata, limit > 0 ? limit - 1 : limit);
-			if (leaf)
-				return leaf;
-		}
-	}
-
-	return NULL;
-}
-
-WMTreeNode *WMFindInTree(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata)
-{
-	wassertrv(aTree != NULL, NULL);
-
-	return findNodeInTree(aTree, match, cdata, -1);
-}
-
-WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit)
-{
-	wassertrv(aTree != NULL, NULL);
-	wassertrv(limit >= 0, NULL);
-
-	return findNodeInTree(aTree, match, cdata, limit);
-}
-
-void WMTreeWalk(WMTreeNode * aNode, WMTreeWalkProc * walk, void *data, Bool DepthFirst)
-{
-	int i;
-	WMTreeNode *leaf;
-
-	wassertr(aNode != NULL);
-
-	if (DepthFirst)
-		(*walk)(aNode, data);
-
-	if (aNode->leaves) {
-		for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) {
-			leaf = (WMTreeNode *)WMGetFromArray(aNode->leaves, i);
-			WMTreeWalk(leaf, walk, data, DepthFirst);
-		}
-	}
-
-	if (!DepthFirst)
-		(*walk)(aNode, data);
-}

util/Makefile.am

@@ -85,8 +85,8 @@ wmgenmenu_LDADD = \
 wmgenmenu_SOURCES = wmgenmenu.c wmgenmenu.h
 
 wmmenugen_LDADD = \
-	$(top_builddir)/WINGs/libWUtil.la \
 	$(top_builddir)/wutil-rs/target/debug/libwutil_rs.a \
+	$(top_builddir)/WINGs/libWUtil.la \
 	@INTLIBS@
 
 wmmenugen_SOURCES = wmmenugen.c wmmenugen.h wmmenugen_misc.c \

util/wmmenugen.c

@@ -35,8 +35,8 @@
 static void addWMMenuEntryCallback(WMMenuEntry *aEntry);
 static void assemblePLMenuFunc(WMTreeNode *aNode, void *data);
 static int dirParseFunc(const char *filename, const struct stat *st, int tflags, struct FTW *ftw);
-static int menuSortFunc(const void *left, const void *right);
-static int nodeFindSubMenuByNameFunc(const void *item, const void *cdata);
+static int menuSortFunc(const WMTreeNode *left, const WMTreeNode *right);
+static int nodeFindSubMenuByNameFunc(const WMTreeNode *tree, const void *cdata);
 static WMTreeNode *findPositionInMenu(const char *submenu);
 
 
@@ -178,7 +178,7 @@ int main(int argc, char **argv)
 	}
 
 	WMSortTree(menu, menuSortFunc);
-	WMTreeWalk(menu, assemblePLMenuFunc, previousDepth, True);
+	WMTreeWalk(menu, assemblePLMenuFunc, previousDepth);
 
 	i = WMGetArrayItemCount(plMenuNodes);
 	if (i > 2) { /* more than one submenu unprocessed is almost certainly an error */
@@ -324,13 +324,13 @@ static void assemblePLMenuFunc(WMTreeNode *aNode, void *data)
 
 /* sort the menu tree; callback for WMSortTree()
  */
-static int menuSortFunc(const void *left, const void *right)
+static int menuSortFunc(const WMTreeNode *left, const WMTreeNode *right)
 {
 	WMMenuEntry *leftwm;
 	WMMenuEntry *rightwm;
 
-	leftwm = (WMMenuEntry *)WMGetDataForTreeNode(*(WMTreeNode **)left);
-	rightwm = (WMMenuEntry *)WMGetDataForTreeNode(*(WMTreeNode **)right);
+	leftwm = (WMMenuEntry *)WMGetDataForTreeNode(left);
+	rightwm = (WMMenuEntry *)WMGetDataForTreeNode(right);
 
 	/* submenus first */
 	if (!leftwm->CmdLine && rightwm->CmdLine)
@@ -380,11 +380,11 @@ static WMTreeNode *findPositionInMenu(const char *submenu)
 
 /* find node where Name = cdata and node is a submenu
  */
-static int nodeFindSubMenuByNameFunc(const void *item, const void *cdata)
+static int nodeFindSubMenuByNameFunc(const WMTreeNode *tree, const void *cdata)
 {
 	WMMenuEntry *wm;
 
-	wm = (WMMenuEntry *)item;
+	wm = (WMMenuEntry *)WMGetDataForTreeNode(tree);
 
 	if (wm->CmdLine) /* if it has a cmdline, it can't be a submenu */
 		return 0;

wutil-rs/Makefile.am

@@ -11,6 +11,7 @@ RUST_SOURCES = \
 	src/memory.rs \
 	src/prop_list.rs \
 	src/string.rs
+	src/tree.rs
 
 RUST_EXTRA = \
 	Cargo.lock \

wutil-rs/src/lib.rs

@@ -6,3 +6,4 @@ pub mod hash_table;
 pub mod memory;
 pub mod prop_list;
 pub mod string;
+pub mod tree;

wutil-rs/src/tree.rs

@@ -0,0 +1,252 @@
+//! N-ary tree structure.
+//!
+//! ## Rust rewrite notes
+//!
+//! This is only used in one place (`util/wmmenugen.c`), and it should probably
+//! be moved out of wutil-rs (if not deleted entirely) once we migrate that
+//! utility.
+//!
+//! The FFI functions provided here assume that they will be used as they are in
+//! `wmmenugen.c`. (For example, the C interface may allow for some callback
+//! parameters to be null, but they aren't in practice, so the Rust layer
+//! doesn't check for that.)
+//!
+//! The original C library had a larger surface area and tracked parent
+//! pointers, but the Rust version only has the API used by wmmenugen. Case in
+//! point: `WMTreeNode` originally had an optional destructor function pointer
+//! for freeing its `data` field, but wmmenugen never actually deleted a tree
+//! node, so the destructor was never called. This Rust rewrite doesn't track a
+//! `data` destructor and doesn't even provide a way to delete tree nodes.
+//!
+//! If a generic tree really becomes necessary, [`Tree`] may be adapted, but a
+//! different design is probably warranted. (At the very least, `Tree` should be
+//! generic over the type of `data` so that we're not using `c_void`. It is also
+//! probably better to index into an owned `Vec` or use an external
+//! arena. `GhostCell` or one of its ilk may even make sense.)
+
+use std::ffi::c_void;
+
+pub struct Tree {
+    depth: u32,
+    children: Vec<Box<Tree>>,
+    data: *mut c_void,
+}
+
+pub mod ffi {
+    use super::Tree;
+
+    use std::{
+        ffi::{c_int, c_void},
+        ptr,
+    };
+
+    /// Creates the root of a new tree with the data `data`.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMCreateTreeNode(data: *mut c_void) -> *mut Tree {
+        Box::leak(Box::new(Tree {
+            depth: 0,
+            children: Vec::new(),
+            data,
+        }))
+    }
+
+    /// Creates a tree node as the `index`th child of `parent`, with the data
+    /// `item`. If `index` is negative, the node is added as the last child of
+    /// `parent`.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMInsertItemInTree(
+        parent: *mut Tree,
+        index: c_int,
+        item: *mut c_void,
+    ) -> *mut Tree {
+        if parent.is_null() {
+            return ptr::null_mut();
+        }
+        let parent = unsafe { &mut *parent };
+
+        let child = Tree {
+            depth: parent.depth + 1,
+            children: Vec::new(),
+            data: item,
+        };
+
+        if index < 0 {
+            parent.children.push(Box::new(child));
+            parent.children.last_mut().unwrap().as_mut() as *mut _
+        } else {
+            let index = index as usize;
+            parent.children.insert(index, Box::new(child));
+            parent.children[index].as_mut() as *mut _
+        }
+    }
+
+    /// Inserts `tree` as the `index`th child of `parent`. If `index` is
+    /// negative, `tree` is added as the last child of `parent`.
+    ///
+    /// This eagerly updates the depth of the entire subtree rooted at `tree`,
+    /// so it has a O(N) cost rather than O(1).
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMInsertNodeInTree(
+        parent: *mut Tree,
+        index: c_int,
+        tree: *mut Tree,
+    ) -> *mut Tree {
+        if parent.is_null() {
+            return ptr::null_mut();
+        }
+        let parent = unsafe { &mut *parent };
+
+        if tree.is_null() {
+            return ptr::null_mut();
+        }
+
+        let mut stack = vec![(unsafe { &mut *tree }, parent.depth + 1)];
+        while let Some((tree, depth)) = stack.pop() {
+            tree.depth = depth;
+            for child in &mut tree.children {
+                stack.push((child, depth + 1));
+            }
+        }
+
+        if index < 0 {
+            parent.children.push(unsafe { Box::from_raw(tree) });
+            tree
+        } else {
+            let index = index as usize;
+            parent
+                .children
+                .insert(index, unsafe { Box::from_raw(tree) });
+            tree
+        }
+    }
+
+    /// Returns the data field of `tree`.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMGetDataForTreeNode(tree: *mut Tree) -> *mut c_void {
+        if tree.is_null() {
+            return ptr::null_mut();
+        }
+        unsafe { (*tree).data }
+    }
+
+    /// Returns the depth of `tree` (0 if `tree` is a root, 1 if it is an
+    /// immediate child of the root, etc.). This is an `O(1)` operation.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMGetTreeNodeDepth(tree: *mut Tree) -> c_int {
+        if tree.is_null() {
+            return 0;
+        }
+        unsafe { (*tree).depth as c_int }
+    }
+
+    /// Recursively sorts the children of each node of the subtree rooted at
+    /// `tree`, according to `comparer`.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMSortTree(
+        tree: *mut Tree,
+        comparer: unsafe extern "C" fn(*const Tree, *const Tree) -> c_int,
+    ) {
+        use std::cmp::Ordering;
+
+        if tree.is_null() {
+            return;
+        }
+
+        let comparer = |a: &Box<Tree>, b: &Box<Tree>| {
+            let a = a.as_ref() as *const Tree as *const _;
+            let b = b.as_ref() as *const Tree as *const _;
+            match unsafe { comparer(a, b) }.signum() {
+                -1 => Ordering::Less,
+                0 => Ordering::Equal,
+                1 => Ordering::Greater,
+                _ => unreachable!(),
+            }
+        };
+
+        let mut stack = vec![unsafe { &mut *tree }];
+
+        while let Some(tree) = stack.pop() {
+            tree.children.sort_by(comparer);
+            stack.extend(tree.children.iter_mut().map(|c| c.as_mut()));
+        }
+    }
+
+    /// Returns the first tree node in the subtree rooted at `tree` that matches
+    /// `match_p`, or null if none is found.  If `match_p` is `None`, returns
+    /// the first node whose `data` field is equal to `cdata`. Search is
+    /// recursive, up to `limit` depth (which must be non-negative).
+    ///
+    /// ## Rust rewrite notes
+    ///
+    /// This was originally a DFS with a `depthFirst` parameter which actually
+    /// controlled whether tree traversal was pre- or post-order (and not
+    /// whether the search was depth-first or breadth-first, which the name
+    /// might seem to suggest). Since this was only ever called with a non-zero
+    /// `depthFirst`, the Rust version is hard-coded as a pre-order DFS.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMFindInTreeWithDepthLimit(
+        tree: *mut Tree,
+        match_p: Option<unsafe extern "C" fn(*const Tree, *const c_void) -> c_int>,
+        cdata: *mut c_void,
+        limit: c_int,
+    ) -> *mut Tree {
+        if tree.is_null() {
+            return ptr::null_mut();
+        }
+        let safe_tree = unsafe { &mut *tree };
+
+        let match_p = |t: &Tree| -> bool {
+            if let Some(p) = match_p {
+                (unsafe { (p)(t, cdata) }) != 0
+            } else {
+                t.data == cdata
+            }
+        };
+
+        let mut stack = vec![(safe_tree, 0)];
+
+        while let Some((tree, depth)) = stack.pop() {
+            if depth > limit {
+                continue;
+            }
+
+            if match_p(tree) {
+                return tree as *mut Tree;
+            }
+
+            for child in tree.children.iter_mut() {
+                stack.push((child.as_mut(), depth + 1));
+            }
+        }
+
+        ptr::null_mut()
+    }
+
+    /// Traverses `tree` recursively, invoking `walk` on each tree node with
+    /// `cdata` passed in to provide a side channel.
+    #[unsafe(no_mangle)]
+    pub unsafe extern "C" fn WMTreeWalk(
+        tree: *mut Tree,
+        walk: unsafe extern "C" fn(*mut Tree, *mut c_void),
+        cdata: *mut c_void,
+    ) {
+        if tree.is_null() {
+            return;
+        }
+        let tree = unsafe { &mut *tree };
+
+        let walk_fn = |t: &mut Tree| unsafe {
+            walk(t as *mut Tree, cdata);
+        };
+
+        let mut stack = vec![tree];
+
+        while let Some(tree) = stack.pop() {
+            walk_fn(tree);
+
+            for child in &mut tree.children {
+                stack.push(child);
+            }
+        }
+    }
+}