add wlf_map and wlf_hash

[zzxyb]

No description provided.

[Copilot]

Pull request overview

This pull request adds two new data structure utilities to the wlframe project: wlf_map (an ordered map implemented as a red-black tree) and wlf_hash (an unordered hash table using open addressing with linear probing). Both structures provide generic key-value storage with custom comparison and hash functions, along with comprehensive APIs for insertion, deletion, lookup, and iteration.

Changes:

• Added red-black tree based ordered map (wlf_map) with O(log n) operations
• Added hash table based unordered map (wlf_hash) with O(1) average-case operations
• Added comprehensive test and example programs demonstrating various usage patterns

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
utils/wlf_map.c	Red-black tree implementation with insert, remove, find, and iterator operations
utils/wlf_hash.c	Hash table implementation with open addressing, auto-resizing, and helper hash functions
include/wlf/utils/wlf_map.h	Public API and documentation for ordered map
include/wlf/utils/wlf_hash.h	Public API and documentation for hash table
utils/meson.build	Build configuration updated to include new source files
examples/utils/utils_map_test.c	Example program demonstrating wlf_map usage patterns
examples/utils/utils_hash_test.c	Example program demonstrating wlf_hash usage patterns
examples/utils/meson.build	Build configuration for example programs

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The delete_fixup function dereferences w (parent->right or parent->left) without checking if it's NULL. In line 182, w->color is accessed, but w could be NULL if parent has no right child. This can cause a segmentation fault. The code should check if w is NULL before accessing its members.

[Copilot]

On 32-bit systems, the right shift by 32 bits in line 101 will be undefined behavior or produce incorrect results since uintptr_t would be 32 bits. The shift amount equals the bit width of the type. This hash function should check the size of uintptr_t and use an appropriate algorithm. Consider using: if (sizeof(uintptr_t) > 4) { /* 64-bit version / } else { / 32-bit version */ }

[Copilot]

The subtraction-based comparisons can overflow when the coordinates have different signs or extreme values. Lines 176 and 178 both use subtraction which can overflow. A safer implementation would be: if (pa->x < pb->x) return -1; if (pa->x > pb->x) return 1; if (pa->y < pb->y) return -1; if (pa->y > pb->y) return 1; return 0;

[Copilot]

The delete_fixup function dereferences w (parent->left) without checking if it's NULL. In line 215, w->color is accessed, but w could be NULL if parent has no left child. This can cause a segmentation fault. The code should check if w is NULL before accessing its members.

[Copilot]

The wlf_hash_contains function will incorrectly return false when a key exists but its associated value is NULL. Since wlf_hash_find returns NULL both when a key is not found and when the value is NULL, this function cannot distinguish between the two cases. The function should directly search for the key rather than relying on wlf_hash_find's return value, or it should accept that NULL values indicate non-existence.

Suggested change

	return wlf_hash_find(hash, key) != NULL;
	if (!hash || !key) {
	return false;
	}
	uint32_t hash_value = hash->hash_func(key);
	size_t index = hash_value % hash->capacity;
	size_t start_index = index;
	while (true) {
	struct wlf_hash_bucket *bucket = &hash->buckets[index];
	if (bucket->state == WLF_HASH_EMPTY) {
	return false; // Key not found
	} else if (bucket->state == WLF_HASH_OCCUPIED &&
	bucket->hash == hash_value &&
	hash->compare(bucket->key, key) == 0) {
	return true;
	}
	index = (index + 1) % hash->capacity;
	if (index == start_index) {
	return false; // Wrapped around, key not found
	}
	}

[Copilot]

The subtraction-based comparison can overflow when the two integers have different signs or extreme values. For example, if *a = INT_MAX and *b = -1, then *a - *b would overflow. A safer implementation would be: int ia = *(const int *)a; int ib = *(const int *)b; if (ia < ib) return -1; if (ia > ib) return 1; return 0;

Suggested change

	return *(const int *)a - *(const int *)b;
	int ia = *(const int *)a;
	int ib = *(const int *)b;
	if (ia < ib)
	return -1;
	if (ia > ib)
	return 1;
	return 0;

[Copilot]

The delete_fixup function is only called when child is not NULL, but it should also be called when child is NULL and original_color is BLACK. In red-black tree deletion, when a black node is removed and replaced with NULL, the tree balance needs to be fixed. The condition should be: if (original_color == WLF_MAP_BLACK) without checking if child is non-NULL.

Suggested change

	if (original_color == WLF_MAP_BLACK && child) {
	if (original_color == WLF_MAP_BLACK) {

[Copilot]

The subtraction-based comparison can overflow when ia and ib have different signs or extreme values. For example, if ia = INT_MAX and ib = -1, then ia - ib would overflow. A safer implementation would be: if (ia < ib) return -1; if (ia > ib) return 1; return 0;

Suggested change

	return ia - ib;
	if (ia < ib) {
	return -1;
	} else if (ia > ib) {
	return 1;
	}
	return 0;

[Copilot]

The subtraction-based comparisons can overflow when the coordinates have different signs or extreme values. Lines 394 and 395 both use subtraction which can overflow. A safer implementation would be: if (ca->x < cb->x) return -1; if (ca->x > cb->x) return 1; if (ca->y < cb->y) return -1; if (ca->y > cb->y) return 1; return 0;

Suggested change

	if (ca->x != cb->x) return ca->x - cb->x;
	return ca->y - cb->y;
	if (ca->x < cb->x) {
	return -1;
	}
	if (ca->x > cb->x) {
	return 1;
	}
	if (ca->y < cb->y) {
	return -1;
	}
	if (ca->y > cb->y) {
	return 1;
	}
	return 0;

[Copilot]

After replace_node is called and the node structure is modified, the fixup_parent should be captured before the replace_node operations happen in the two-children case. The current code captures fixup_parent after all the node replacements, but when to_delete is not the direct child of node, the parent relationship has already been changed by replace_node operations. This should be: struct wlf_map_node *fixup_parent = (child) ? child->parent : ((to_delete->parent == node) ? to_delete : to_delete->parent); or the parent should be saved before any modifications.

Suggested change

	struct wlf_map_node *fixup_parent = to_delete->parent;
	struct wlf_map_node *fixup_parent =
	(child) ? child->parent
	: ((to_delete->parent == node) ? to_delete : to_delete->parent);