From: James Jones <jejones3141@gmail.com>
Date: Thu, 2 Apr 2020 14:48:54 +0000 (-0500)
Subject: make dhcpv4 use an iterator. This includes making dhcpv4_encode_xlat() only
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fec3c0ca02fac0bdb460cfa9ce82382858488bfc;p=thirdparty%2Ffreeradius-server.git

make dhcpv4 use an iterator. This includes making dhcpv4_encode_xlat() only
pass dhcpv4-encodable items to fr_dhcpv4_encode_option().
---

diff --git a/src/modules/rlm_dhcpv4/rlm_dhcpv4.c b/src/modules/rlm_dhcpv4/rlm_dhcpv4.c
index c227a506ac3..5c53cc6a5ee 100644
--- a/src/modules/rlm_dhcpv4/rlm_dhcpv4.c
+++ b/src/modules/rlm_dhcpv4/rlm_dhcpv4.c
@@ -166,6 +166,11 @@ static xlat_action_t dhcpv4_encode_xlat(TALLOC_CTX *ctx, fr_cursor_t *out,
 	if (!fr_cursor_head(cursor)) return XLAT_ACTION_DONE;	/* Nothing to encode */
 
 	while ((vp = fr_cursor_current(cursor))) {
+		if ((vp->da->flags.internal) || (vp->da->dict != dict_dhcpv4)) {
+			fr_cursor_next(cursor);
+			continue;
+		}
+
 		len = fr_dhcpv4_encode_option(p, end - p, cursor,
 					      &(fr_dhcpv4_ctx_t){ .root = fr_dict_root(dict_dhcpv4) });
 		if (len < 0) {
diff --git a/src/protocols/dhcpv4/base.c b/src/protocols/dhcpv4/base.c
index 4b31cc55cad..2a039dc52ac 100644
--- a/src/protocols/dhcpv4/base.c
+++ b/src/protocols/dhcpv4/base.c
@@ -470,7 +470,7 @@ ssize_t fr_dhcpv4_encode(uint8_t *buffer, size_t buflen, dhcp_packet_t *original
 	 *  operates correctly. This changes the order of the list, but never mind...
 	 */
 	fr_pair_list_sort(&vps, fr_dhcpv4_attr_cmp);
-	fr_cursor_init(&cursor, &vps);
+	fr_cursor_talloc_iter_init(&cursor, &vps, fr_proto_next_encodable, dict_dhcpv4, VALUE_PAIR);
 
 	/*
 	 *  Each call to fr_dhcpv4_encode_option will encode one complete DHCP option,
diff --git a/src/protocols/dhcpv4/encode.c b/src/protocols/dhcpv4/encode.c
index 309c1c76768..c76e43e918b 100644
--- a/src/protocols/dhcpv4/encode.c
+++ b/src/protocols/dhcpv4/encode.c
@@ -34,51 +34,6 @@
 #include "dhcpv4.h"
 #include "attrs.h"
 
-static inline bool is_encodable(fr_dict_attr_t const *root, VALUE_PAIR *vp)
-{
-	if (!vp) return false;
-	if (vp->da->flags.internal) return false;
-	if (!fr_dict_parent_common(root, vp->da, true)) return false;
-
-	return true;
-}
-
-/** Find the next attribute to encode
- *
- * @param[in] cursor		to iterate over.
- * @param[in] encoder_ctx	Containing DHCPv4 dictionary.
- * @return
- *	- encodable VALUE_PAIR.
- *	- NULL if none available.
- */
-static inline VALUE_PAIR *next_encodable(fr_cursor_t *cursor, void *encoder_ctx)
-{
-	VALUE_PAIR	*vp;
-	fr_dhcpv4_ctx_t	*packet_ctx = encoder_ctx;
-
-	do { vp = fr_cursor_next(cursor); } while (vp && !is_encodable(packet_ctx->root, vp));
-	return fr_cursor_current(cursor);
-}
-
-/** Determine if the current attribute is encodable, or find the first one that is
- *
- * @param[in] cursor		to iterate over.
- * @param[in] encoder_ctx	Containing DHCPv4 dictionary.
- * @return
- *	- encodable VALUE_PAIR.
- *	- NULL if none available.
- */
-static inline VALUE_PAIR *first_encodable(fr_cursor_t *cursor, void *encoder_ctx)
-{
-	VALUE_PAIR	*vp;
-	fr_dhcpv4_ctx_t	*packet_ctx = encoder_ctx;
-
-	vp = fr_cursor_current(cursor);
-	if (is_encodable(packet_ctx->root, vp)) return vp;
-
-	return next_encodable(cursor, encoder_ctx);
-}
-
 /** Write DHCP option value into buffer
  *
  * Does not include DHCP option length or number.
@@ -96,7 +51,7 @@ static inline VALUE_PAIR *first_encodable(fr_cursor_t *cursor, void *encoder_ctx
  */
 static ssize_t encode_value(uint8_t *out, size_t outlen,
 			    fr_da_stack_t *da_stack, unsigned int depth,
-			    fr_cursor_t *cursor, fr_dhcpv4_ctx_t *encoder_ctx)
+			    fr_cursor_t *cursor, UNUSED fr_dhcpv4_ctx_t *encoder_ctx)
 {
 	VALUE_PAIR	*vp = fr_cursor_current(cursor);
 	uint8_t		*p = out;
@@ -144,10 +99,10 @@ static ssize_t encode_value(uint8_t *out, size_t outlen,
 
 	default:
 		fr_strerror_printf("Unsupported option type %d", vp->vp_type);
-		(void)next_encodable(cursor, encoder_ctx);
+		(void)fr_cursor_next(cursor);
 		return -2;
 	}
-	vp = next_encodable(cursor, encoder_ctx);	/* We encoded a leaf, advance the cursor */
+	vp = fr_cursor_next(cursor);	/* We encoded a leaf, advance the cursor */
 	fr_proto_da_stack_build(da_stack, vp ? vp->da : NULL);
 
 	FR_PROTO_STACK_PRINT(da_stack, depth);
@@ -478,14 +433,14 @@ ssize_t fr_dhcpv4_encode_option(uint8_t *out, size_t outlen, fr_cursor_t *cursor
 	fr_da_stack_t		da_stack;
 	ssize_t			len;
 
-	vp = first_encodable(cursor, encoder_ctx);
+	vp = fr_cursor_current(cursor);
 	if (!vp) return -1;
 
 	if (vp->da == attr_dhcp_message_type) goto next; /* already done */
 	if ((vp->da->attr > 255) && (vp->da->attr != FR_DHCP_OPTION_82)) {
 	next:
 		fr_strerror_printf("Attribute \"%s\" is not a DHCP option", vp->da->name);
-		next_encodable(cursor, encoder_ctx);
+		(void)fr_cursor_next(cursor);
 		return 0;
 	}