From: Alan T. DeKok Date: Sat, 17 Jan 2026 13:10:28 +0000 (-0500) Subject: replace foreach_safe() with a safe foreach() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=83bf923ea57b6b80c91bb12d7b258391bfa2805d;p=thirdparty%2Ffreeradius-server.git replace foreach_safe() with a safe foreach() --- diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index df0653680e0..4add457e043 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -1297,9 +1297,9 @@ int tmpl_eval_cast_in_place(fr_value_box_list_t *list, request_t *request, tmpl_ * we now need to handle potentially * multivalued lists. */ - fr_value_box_list_foreach_safe(list, vb) { + fr_value_box_list_foreach(list, vb) { if (fr_value_box_cast_in_place(vb, vb, cast, NULL) < 0) goto error; - }} + } /* * ...and finally, apply the escape function diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index afbd9dea3e7..5f71e274872 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -257,9 +257,9 @@ xlat_action_t xlat_transparent(TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED xlat_ctx_t const *xctx, UNUSED request_t *request, fr_value_box_list_t *args) { - fr_value_box_list_foreach_safe(args, vb) { + fr_value_box_list_foreach(args, vb) { xlat_arg_copy_out(ctx, out, args, vb); - }} + } return XLAT_ACTION_DONE; } @@ -1361,7 +1361,7 @@ static xlat_action_t xlat_func_map(TALLOC_CTX *ctx, fr_dcursor_t *out, vb->vb_bool = false; /* Default fail value - changed to true on success */ fr_dcursor_append(out, vb); - fr_value_box_list_foreach_safe(&fmt_vb->vb_group, fmt) { + fr_value_box_list_foreach(&fmt_vb->vb_group, fmt) { if (map_afrom_attr_str(request, &map, fmt->vb_strvalue, &attr_rules, &attr_rules) < 0) { RPEDEBUG("Failed parsing \"%s\" as map", fmt_vb->vb_strvalue); return XLAT_ACTION_FAIL; @@ -1398,7 +1398,7 @@ static xlat_action_t xlat_func_map(TALLOC_CTX *ctx, fr_dcursor_t *out, REXDENT(); talloc_free(map); if (ret < 0) return XLAT_ACTION_FAIL; - }} + } vb->vb_bool = true; return XLAT_ACTION_DONE; @@ -2291,9 +2291,9 @@ static xlat_action_t xlat_func_join(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_value_box_list_foreach(in, arg) { fr_assert(arg->type == FR_TYPE_GROUP); - fr_value_box_list_foreach_safe(&arg->vb_group, vb) { + fr_value_box_list_foreach(&arg->vb_group, vb) { xlat_arg_copy_out(ctx, out, &arg->vb_group, vb); - }} + } } return XLAT_ACTION_DONE; } diff --git a/src/lib/util/dlist.h b/src/lib/util/dlist.h index a500692c3d1..2615d2ef3f7 100644 --- a/src/lib/util/dlist.h +++ b/src/lib/util/dlist.h @@ -86,35 +86,17 @@ static_assert(sizeof(unsigned int) >= 4, "Unsigned integer too small on this pla /** Iterate over the contents of a list * - * @param[in] _list_head to iterate over. - * @param[in] _type of item the list contains. - * @param[in] _iter Name of iteration variable. - * Will be declared in the scope of the loop. - */ -#define fr_dlist_foreach(_list_head, _type, _iter) \ - for (_type *_iter = fr_dlist_head(_list_head); _iter; _iter = fr_dlist_next(_list_head, _iter)) - -/** Iterate over the contents of a list allowing for removals + * The macro is "safe", in that the current iterator variable can be deleted. * - * @note foreach block must end with double curlybrace `}}`. - * We need to use another scoping section as we can't declare variables of multiple - * types within the initialiser of a for loop. + * The iterators can be nested, so long as the _iter variable names are different. * * @param[in] _list_head to iterate over. * @param[in] _type of item the list contains. * @param[in] _iter Name of iteration variable. * Will be declared in the scope of the loop. */ -#define fr_dlist_foreach_safe(_list_head, _type, _iter) \ -{ \ - _type *_iter; \ - fr_dlist_t _tmp ## _iter; \ - for (_iter = fr_dlist_head(_list_head), \ - _tmp ## _iter = fr_dlist_head(_list_head) ? *fr_dlist_item_to_entry((_list_head)->offset, fr_dlist_head(_list_head)) : (fr_dlist_t){ .prev = NULL, .next = NULL }; \ - _iter; \ - _iter = _tmp ## _iter.next && (_tmp ## _iter.next != &(_list_head)->entry) ? fr_dlist_entry_to_item((_list_head)->offset, _tmp ## _iter.next) : NULL, \ - _tmp ## _iter = _tmp ## _iter.next && (_tmp ## _iter.next != &(_list_head)->entry) ? *_tmp ## _iter.next : (fr_dlist_t){ .prev = NULL, .next = NULL }) - +#define fr_dlist_foreach(_list_head, _type, _iter) \ + for (_type *JOIN(_next,_iter), *_iter = fr_dlist_head(_list_head); JOIN(_next,_iter) = fr_dlist_next(_list_head, _iter), _iter != NULL; _iter = JOIN(_next,_iter)) /** Find the dlist pointers within a list item * @@ -1260,7 +1242,6 @@ DIAG_ON(unused-function) * Unfortunately as there's no way to dynamically define macro names these need to be done manually. * * #define _foreach(_list_head, _iter) fr_dlist_foreach(_dlist_head(_list_head), , _iter) - * #define _foreach_safe(_list_head, _iter) fr_dlist_foreach_safe(_dlist_head(_list_head), , _iter) * #define _verify(_list_head) __verify(__FILE__, __LINE__, _list_head) */ diff --git a/src/lib/util/dlist_tests.c b/src/lib/util/dlist_tests.c index 38617a62d13..ea3058a3007 100644 --- a/src/lib/util/dlist_tests.c +++ b/src/lib/util/dlist_tests.c @@ -177,10 +177,10 @@ static void test_dlist_foreach_safe(void) fr_dlist_insert_tail(&head, &a2); fr_dlist_insert_tail(&head, &a3); - fr_dlist_foreach_safe(&head, dlist_test_item_t, i) { + fr_dlist_foreach(&head, dlist_test_item_t, i) { fr_dlist_remove(&head, i); count++; - }} + } TEST_CHECK_RET((int)count, (int)3); } diff --git a/src/lib/util/event.c b/src/lib/util/event.c index 4f49c150637..5e2e827aa4c 100644 --- a/src/lib/util/event.c +++ b/src/lib/util/event.c @@ -1715,7 +1715,7 @@ unsigned int fr_event_list_reap_signal(fr_event_list_t *el, fr_time_delta_t time if (unlikely(kq < 0)) goto force; - fr_dlist_foreach_safe(&el->pid_to_reap, fr_event_pid_reap_t, i) { + fr_dlist_foreach(&el->pid_to_reap, fr_event_pid_reap_t, i) { if (!i->pid_ev) { EVENT_DEBUG("%p - %s - Reaper already called (logic error)... - %p", el, __FUNCTION__, i); @@ -1748,7 +1748,7 @@ unsigned int fr_event_list_reap_signal(fr_event_list_t *el, fr_time_delta_t time continue; } waiting++; - }} + } /* * Keep draining process exits as they come in... diff --git a/src/lib/util/rb_expire.c b/src/lib/util/rb_expire.c index f04eced2bbb..e5a0cca343f 100644 --- a/src/lib/util/rb_expire.c +++ b/src/lib/util/rb_expire.c @@ -66,7 +66,7 @@ void fr_rb_expire_update(fr_rb_expire_t *expire, void *data, fr_time_t now) /* * Expire old entries. */ - fr_dlist_foreach_safe(&expire->head, fr_rb_expire_node_t, old) {{ + fr_dlist_foreach(&expire->head, fr_rb_expire_node_t, old) {{ re = (fr_rb_expire_node_t *) (((uintptr_t) old) - offsetof(fr_rb_expire_node_t, entry)); if (old->when > now) break; @@ -74,6 +74,6 @@ void fr_rb_expire_update(fr_rb_expire_t *expire, void *data, fr_time_t now) fr_dlist_remove(&expire->head, &old->entry); fr_rb_delete(&expire->tree, re); - }} + } #endif } diff --git a/src/lib/util/udp_queue.c b/src/lib/util/udp_queue.c index 2c31f0cda12..05e40059c67 100644 --- a/src/lib/util/udp_queue.c +++ b/src/lib/util/udp_queue.c @@ -61,9 +61,9 @@ typedef struct { static int _udp_queue_free(fr_udp_queue_t *uq) { - fr_dlist_foreach_safe(&uq->queue, fr_udp_queue_entry_t, entry) { + fr_dlist_foreach(&uq->queue, fr_udp_queue_entry_t, entry) { talloc_free(entry); - }} + } close(uq->fd); @@ -188,7 +188,7 @@ static void udp_queue_writable(UNUSED fr_event_list_t *el, UNUSED int fd, fr_udp_queue_t *uq = talloc_get_type_abort(uctx, fr_udp_queue_t); fr_time_t now = fr_time(); - fr_dlist_foreach_safe(&uq->queue, fr_udp_queue_entry_t, entry) { + fr_dlist_foreach(&uq->queue, fr_udp_queue_entry_t, entry) { ssize_t rcode; int retries = 0; @@ -226,7 +226,7 @@ static void udp_queue_writable(UNUSED fr_event_list_t *el, UNUSED int fd, if (errno != EWOULDBLOCK) return; #endif } - }} + } /* * Nothing more to write, delete the IO handler so that we don't get extraneous signals. diff --git a/src/lib/util/uri.c b/src/lib/util/uri.c index b4c7cdd0b69..55fdcb691a6 100644 --- a/src/lib/util/uri.c +++ b/src/lib/util/uri.c @@ -146,9 +146,9 @@ int fr_uri_escape_list(fr_value_box_list_t *uri, fr_uri_part_t const *uri_parts, fr_strerror_clear(); - fr_value_box_list_foreach_safe(uri, uri_vb) { + fr_value_box_list_foreach(uri, uri_vb) { if (unlikely(fr_uri_escape(uri_vb, &ctx)) < 0) return -1; - }} + } return 0; } @@ -172,7 +172,7 @@ int fr_uri_has_scheme(fr_value_box_list_t *uri, fr_table_num_sorted_t const *sch /* * Fill the scheme buffer with at most sizeof(scheme_buff) - 1 bytes of string data. */ - fr_value_box_list_foreach_safe(uri, vb) { + fr_value_box_list_foreach(uri, vb) { fr_value_box_t tmp; int ret; @@ -190,7 +190,7 @@ int fr_uri_has_scheme(fr_value_box_list_t *uri, fr_table_num_sorted_t const *sch } if (unlikely(ret < 0)) return -1; - }} + } /* * Ensure the first box is a valid scheme diff --git a/src/lib/util/value.c b/src/lib/util/value.c index c37c0f763a7..2f82a81639f 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -6454,11 +6454,11 @@ ssize_t fr_value_box_list_concat_as_string(fr_value_box_t *safety, fr_sbuff_t *s * an issue concatenating them, everything * is still in a known state. */ - fr_value_box_list_foreach_safe(list, vb) { + fr_value_box_list_foreach(list, vb) { if (vb_should_remove(proc_action)) fr_value_box_list_remove(list, vb); if (vb_should_free_value(proc_action)) fr_value_box_clear_value(vb); if (vb_should_free(proc_action)) talloc_free(vb); - }} + } FR_SBUFF_SET_RETURN(sbuff, &our_sbuff); } @@ -6554,11 +6554,11 @@ ssize_t fr_value_box_list_concat_as_octets(fr_value_box_t *safety, fr_dbuff_t *d * an issue concatenating them, everything * is still in a known state. */ - fr_value_box_list_foreach_safe(list, vb) { + fr_value_box_list_foreach(list, vb) { if (vb_should_remove(proc_action)) fr_value_box_list_remove(list, vb); if (vb_should_free_value(proc_action)) fr_value_box_clear_value(vb); if (vb_should_free(proc_action)) talloc_free(vb); - }} + } return fr_dbuff_set(dbuff, &our_dbuff); } @@ -6828,17 +6828,17 @@ int fr_value_box_list_escape_in_place(fr_value_box_list_t *list, fr_value_box_es */ void fr_value_box_flatten(TALLOC_CTX *ctx, fr_value_box_list_t *list, bool steal, bool free) { - fr_value_box_list_foreach_safe(list, child) { + fr_value_box_list_foreach(list, child) { if (!fr_type_is_structural(child->type)) continue; - fr_value_box_list_foreach_safe(&child->vb_group, grandchild) { + fr_value_box_list_foreach(&child->vb_group, grandchild) { fr_value_box_list_remove(&child->vb_group, grandchild); if (steal) talloc_steal(ctx, grandchild); fr_value_box_list_insert_before(list, child, grandchild); - }} + } if (free) talloc_free(child); - }} + } } /** Concatenate the string representations of a list of value boxes together diff --git a/src/lib/util/value.h b/src/lib/util/value.h index 3c187f41fb4..55b4504ceb8 100644 --- a/src/lib/util/value.h +++ b/src/lib/util/value.h @@ -222,7 +222,6 @@ struct value_box_s { FR_DLIST_FUNCS(fr_value_box_list, fr_value_box_t, entry) #define fr_value_box_list_foreach(_list_head, _iter) fr_dlist_foreach(fr_value_box_list_dlist_head(_list_head), fr_value_box_t, _iter) -#define fr_value_box_list_foreach_safe(_list_head, _iter) fr_dlist_foreach_safe(fr_value_box_list_dlist_head(_list_head), fr_value_box_t, _iter) FR_DCURSOR_FUNCS(fr_value_box_dcursor, fr_value_box_list, fr_value_box_t) /** @} */