In HURD_IHASH_ITERATE and HURD_IHASH_ITERATE_ITEMS, the hash table
argument (ht) was evaluated multiple times during loop initialization
and condition checking. If a caller passed an expression with side
effects (e.g., a function call returning a table pointer), it would
be executed repeatedly per loop, leading to performance degradation
or unintended behavior.
Fix this by wrapping the iterators in an outer `for` loop that
evaluates and caches `(ht)` exactly once using `__typeof__`. Token
pasting (##) is used to generate a unique cache variable name based
on the caller's item identifier, safely allowing these macros to be
nested without variable shadowing or naming collisions.
---
libihash/ihash.h | 46 +++++++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/libihash/ihash.h b/libihash/ihash.h
index 80679f14..5f73f3e1 100644
--- a/libihash/ihash.h
+++ b/libihash/ihash.h
@@ -322,17 +322,22 @@ hurd_ihash_value_t hurd_ihash_locp_find (hurd_ihash_t ht,
pointer pointed to must not have an influence on the condition
result, so the comma operator is used to make sure this
subexpression is always true). */
-#define HURD_IHASH_ITERATE(ht, val) \
- for (hurd_ihash_value_t val, \
- *_hurd_ihash_valuep = (ht)->size ? &(ht)->items[0].value : 0; \
- (ht)->size \
- && (size_t) ((_hurd_ihash_item_t) _hurd_ihash_valuep \
- - &(ht)->items[0]) \
- < (ht)->size \
- && (val = *_hurd_ihash_valuep, 1); \
- _hurd_ihash_valuep = (hurd_ihash_value_t *) \
- (((_hurd_ihash_item_t) _hurd_ihash_valuep) + 1)) \
- if (val != _HURD_IHASH_EMPTY && val != _HURD_IHASH_DELETED)
+#define HURD_IHASH_ITERATE(ht, val)
\
+ /* Cache 'ht' to prevent multiple evaluation of functions. */
\
+ for (__typeof__(ht) _ihash_ht_##val = (ht);
\
+ _ihash_ht_##val != NULL;
\
+ _ihash_ht_##val = NULL)
\
+ for (hurd_ihash_value_t val,
\
+ *_ihash_p_##val = _ihash_ht_##val->size
\
+ ? &_ihash_ht_##val->items[0].value : 0;
\
+ _ihash_ht_##val->size
\
+ && (size_t) ((_hurd_ihash_item_t) _ihash_p_##val
\
+ - &_ihash_ht_##val->items[0])
\
+ < _ihash_ht_##val->size
\
+ && (((val) = *_ihash_p_##val), 1);
\
+ _ihash_p_##val = (hurd_ihash_value_t *)
\
+ (((_hurd_ihash_item_t) _ihash_p_##val) + 1))
\
+ if ((val) != _HURD_IHASH_EMPTY && (val) != _HURD_IHASH_DELETED)
/* Iterate over all elements in the hash table making both the key and
the value available. You use this macro with a block, for example
@@ -344,12 +349,19 @@ hurd_ihash_value_t hurd_ihash_locp_find (hurd_ihash_t ht,
The block will be run for every element in the hash table HT. The
key and value of the current element is available as ITEM->key and
ITEM->value. */
-#define HURD_IHASH_ITERATE_ITEMS(ht, item) \
- for (_hurd_ihash_item_t item = (ht)->size? &(ht)->items[0]: 0; \
- (ht)->size && item - &(ht)->items[0] < (ht)->size; \
- item++) \
- if (item->value != _HURD_IHASH_EMPTY && \
- item->value != _HURD_IHASH_DELETED)
+#define HURD_IHASH_ITERATE_ITEMS(ht, item)
\
+ /* Cache 'ht' to prevent multiple evaluation of functions. */
\
+ for (__typeof__(ht) _ihash_items_ht_##item = (ht);
\
+ _ihash_items_ht_##item != NULL;
\
+ _ihash_items_ht_##item = NULL)
\
+ for (_hurd_ihash_item_t item = _ihash_items_ht_##item->size
\
+ ? &_ihash_items_ht_##item->items[0] : 0;
\
+ _ihash_items_ht_##item->size
\
+ && (size_t) ((item) - &_ihash_items_ht_##item->items[0])
\
+ < _ihash_items_ht_##item->size;
\
+ (item)++)
\
+ if ((item)->value != _HURD_IHASH_EMPTY &&
\
+ (item)->value != _HURD_IHASH_DELETED)
/* Remove the entry with the key KEY from the hash table HT. If such
an entry was found and removed, 1 is returned, otherwise 0. */
--
2.53.0
