From: Nick Porter <nick@portercomputing.co.uk>
Date: Wed, 10 Aug 2022 11:20:54 +0000 (+0100)
Subject: Stop fr_dcursor_next() advancing before calling iterator (#4660)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cb5466ec54d7c1514e4eae6c2db90d537abd8569;p=thirdparty%2Ffreeradius-server.git

Stop fr_dcursor_next() advancing before calling iterator (#4660)
---

diff --git a/src/lib/util/dcursor.h b/src/lib/util/dcursor.h
index cd0cc48a9a6..e7acc94e865 100644
--- a/src/lib/util/dcursor.h
+++ b/src/lib/util/dcursor.h
@@ -162,23 +162,16 @@ static inline void *dcursor_next(fr_dcursor_t *cursor, fr_dcursor_iter_t iter, v
 	if (!current) {
 		if (cursor->at_end) return NULL;				/* At tail of the list */
 
-		next = iter(cursor->dlist, fr_dlist_head(cursor->dlist), cursor->iter_uctx);
+		next = iter(cursor->dlist, NULL, cursor->iter_uctx);
 		VALIDATE(next);
 		return next;
 	}
 	VALIDATE(current);
 
 	/*
-	 *	Get the next entry, and pass it to the iterator.
+	 *	The iterator will advance to the next matching entry.
 	 */
-	next = fr_dlist_next(cursor->dlist, current);
-	if (!next) return NULL;
-
-	/*
-	 *	The iterator can just return what it was passed for curr
-	 *	if it just wants to advance by one.
-	 */
-	next = iter(cursor->dlist, next, cursor->iter_uctx);
+	next = iter(cursor->dlist, current, cursor->iter_uctx);
 	VALIDATE(next);
 
 	return next;
diff --git a/src/lib/util/dcursor_tests.c b/src/lib/util/dcursor_tests.c
index cfdaeb3f71d..4a58cdbddb2 100644
--- a/src/lib/util/dcursor_tests.c
+++ b/src/lib/util/dcursor_tests.c
@@ -15,9 +15,9 @@ typedef struct {
         fr_dlist_head_t head;
 } test_item_list_t;
 
-static void *test_iter(UNUSED fr_dlist_head_t *list, void *current, UNUSED void *uctx)
+static void *test_iter(fr_dlist_head_t *list, void *current, UNUSED void *uctx)
 {
-	return current;
+	return fr_dlist_next(list, current);
 }
 
 static void test_list_init(test_item_list_t *list)
@@ -1503,14 +1503,12 @@ typedef struct {
 	char	val;
 } item_filter;
 
-static void *iter_name_check(fr_dlist_head_t *list, void *to_eval, void *uctx)
+static void *iter_name_check(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	test_item_t	*c;
+	test_item_t	*c = current;
 	item_filter	*f = uctx;
 
-	if (!to_eval) return NULL;
-
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while((c = fr_dlist_next(list, c))) {
 		if (c->name[f->pos] == f->val) break;
 	}
 
diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index 95ddc85ec0d..2439c69405c 100644
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -600,19 +600,19 @@ int fr_pair_to_unknown(fr_pair_t *vp)
 /** Iterate over pairs with a specified da
  *
  * @param[in] list	to iterate over.
- * @param[in] to_eval	The fr_pair_t after cursor->current.  Will be checked to
+ * @param[in] current	The fr_pair_t cursor->current.  Will be advanced and checked to
  *			see if it matches the specified fr_dict_attr_t.
  * @param[in] uctx	The fr_dict_attr_t to search for.
  * @return
  *	- Next matching fr_pair_t.
  *	- NULL if not more matching fr_pair_ts could be found.
  */
-static void *fr_pair_iter_next_by_da(fr_dlist_head_t *list, void *to_eval, void *uctx)
+static void *fr_pair_iter_next_by_da(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_attr_t	*da = uctx;
 
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 		if (c->da == da) break;
 	}
@@ -623,19 +623,19 @@ static void *fr_pair_iter_next_by_da(fr_dlist_head_t *list, void *to_eval, void
 /** Iterate over pairs which are decedents of the specified da
  *
  * @param[in] list	to itterate over.
- * @param[in] to_eval	The fr_pair_t after cursor->current.  Will be checked to
+ * @param[in] current	The fr_pair_t cursor->current.  Will be advanced and checked to
  *			see if it matches the specified fr_dict_attr_t.
  * @param[in] uctx	The fr_dict_attr_t to search for.
  * @return
  *	- Next matching fr_pair_t.
  *	- NULL if not more matching fr_pair_ts could be found.
  */
-static void *fr_pair_iter_next_by_ancestor(fr_dlist_head_t *list, void *to_eval, void *uctx)
+static void *fr_pair_iter_next_by_ancestor(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_attr_t	*da = uctx;
 
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 		if (fr_dict_attr_common_parent(da, c->da, true)) break;
 	}
diff --git a/src/lib/util/proto.c b/src/lib/util/proto.c
index 5e8edd7daf5..ae9e3ce15d6 100644
--- a/src/lib/util/proto.c
+++ b/src/lib/util/proto.c
@@ -90,21 +90,19 @@ void fr_proto_da_stack_print(char const *file, int line, char const *func, fr_da
 /** Implements the default iterator to encode pairs belonging to a specific dictionary that are not internal
  *
  * @param[in] list	to itterate over.
- * @param[in] to_eval	The fr_pair_t after cursor->current.  Will be checked to
+ * @param[in] current	The fr_pair_t cursor->current.  Will be advanced and checked to
  *			see if it matches the specified fr_dict_t.
  * @param[in] uctx	The fr_dict_t to search for.
  * @return
  *	- Next matching fr_pair_t.
  *	- NULL if not more matching fr_pair_ts could be found.
  */
-void *fr_proto_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx)
+void *fr_proto_next_encodable(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_t	*dict = talloc_get_type_abort(uctx, fr_dict_t);
 
-	if (!to_eval) return NULL;
-
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 		if ((c->da->dict == dict) && (!c->da->flags.internal)) break;
 	}
diff --git a/src/lib/util/proto.h b/src/lib/util/proto.h
index 531a2bf51b4..5c8b72f7662 100644
--- a/src/lib/util/proto.h
+++ b/src/lib/util/proto.h
@@ -62,7 +62,7 @@ void fr_proto_print_hex_data(char const *file, int line, uint8_t const *data, si
 
 void fr_proto_print_hex_marker(char const *file, int line, uint8_t const *data, size_t data_len, ssize_t slen, char const *fmt, ...);
 
-void *fr_proto_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx);
+void *fr_proto_next_encodable(fr_dlist_head_t *list, void *current, void *uctx);
 
 void fr_proto_da_stack_print(char const *file, int line, char const *func, fr_da_stack_t *da_stack, unsigned int depth);
 
diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index 7faf3a07582..a359fa0c083 100644
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -439,14 +439,12 @@ static int8_t pair_sort_increasing(void const *a, void const *b)
 	return CMP_PREFER_SMALLER(my_a->da->attr, my_b->da->attr);
 }
 
-static void *struct_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx)
+static void *struct_next_encodable(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_attr_t	*parent = talloc_get_type_abort(uctx, fr_dict_attr_t);
 
-	if (!to_eval) return NULL;
-
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 
 		if (c->da->dict != parent->dict || c->da->flags.internal) continue;
diff --git a/src/protocols/dhcpv4/base.c b/src/protocols/dhcpv4/base.c
index 362fef559f0..e0f49074208 100644
--- a/src/protocols/dhcpv4/base.c
+++ b/src/protocols/dhcpv4/base.c
@@ -291,14 +291,12 @@ bool fr_dhcpv4_is_encodable(void const *item, UNUSED void const *uctx)
 /** DHCPV4-specific iterator
  *
  */
-void *fr_dhcpv4_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx)
+void *fr_dhcpv4_next_encodable(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_t	*dict = talloc_get_type_abort(uctx, fr_dict_t);
 
-	if (!to_eval) return NULL;
-
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 		if (c->da->dict != dict || c->da->flags.internal) continue;
 
diff --git a/src/protocols/dhcpv6/attrs.h b/src/protocols/dhcpv6/attrs.h
index ee01e0b6378..628021f60a3 100644
--- a/src/protocols/dhcpv6/attrs.h
+++ b/src/protocols/dhcpv6/attrs.h
@@ -40,4 +40,4 @@ extern HIDDEN fr_dict_attr_t const *attr_relay_message;
 /*
  *	A private function that is used only in base.c and encode.c
  */
-void *fr_dhcpv6_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx);
+void *fr_dhcpv6_next_encodable(fr_dlist_head_t *list, void *current, void *uctx);
diff --git a/src/protocols/dhcpv6/base.c b/src/protocols/dhcpv6/base.c
index 0379c2cd77c..fab357b534e 100644
--- a/src/protocols/dhcpv6/base.c
+++ b/src/protocols/dhcpv6/base.c
@@ -696,14 +696,12 @@ decode_options:
 /** DHCPV6-specific iterator
  *
  */
-void *fr_dhcpv6_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx)
+void *fr_dhcpv6_next_encodable(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_t	*dict = talloc_get_type_abort(uctx, fr_dict_t);
 
-	if (!to_eval) return NULL;
-
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 		if (c->da->dict != dict || c->da->flags.internal) continue;
 		if (c->da->type == FR_TYPE_BOOL && !c->vp_bool) continue;
diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index d4e5a3bd7a6..af0ce10cae4 100644
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -815,16 +815,14 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *original,
 	return 0;
 }
 
-void *fr_radius_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx);
+void *fr_radius_next_encodable(fr_dlist_head_t *list, void *current, void *uctx);
 
-void *fr_radius_next_encodable(fr_dlist_head_t *list, void *to_eval, void *uctx)
+void *fr_radius_next_encodable(fr_dlist_head_t *list, void *current, void *uctx)
 {
-	fr_pair_t	*c;
+	fr_pair_t	*c = current;
 	fr_dict_t	*dict = talloc_get_type_abort(uctx, fr_dict_t);
 
-	if (!to_eval) return NULL;
-
-	for (c = to_eval; c; c = fr_dlist_next(list, c)) {
+	while ((c = fr_dlist_next(list, c))) {
 		PAIR_VERIFY(c);
 		if ((c->da->dict == dict) &&
 		    (!c->da->flags.internal || ((c->da->attr > FR_TAG_BASE) && (c->da->attr < (FR_TAG_BASE + 0x20))))) {