From: Nick Porter <nick@portercomputing.co.uk>
Date: Mon, 2 Sep 2024 15:35:39 +0000 (+0100)
Subject: 2nd attempt to clean up "set leaf num" API
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6738a9e845066e655e00eaf7bf6a9b8d25168fa6;p=thirdparty%2Ffreeradius-server.git

2nd attempt to clean up "set leaf num" API

update section lhs doesn't need &attr changed to &attr[*]
---

diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h
index 443e8f2658..4ce7dbc331 100644
--- a/src/lib/server/tmpl.h
+++ b/src/lib/server/tmpl.h
@@ -1189,9 +1189,7 @@ int			tmpl_attr_set_da(tmpl_t *vpt, fr_dict_attr_t const *da) CC_HINT(nonnull);
 
 int			tmpl_attr_set_leaf_da(tmpl_t *vpt, fr_dict_attr_t const *da) CC_HINT(nonnull);
 
-void			tmpl_attr_set_leaf_num(tmpl_t *vpt, int16_t num) CC_HINT(nonnull);
-
-void			tmpl_attr_rewrite_leaf_num(tmpl_t *vpt, int16_t from, int16_t to) CC_HINT(nonnull);
+void			tmpl_attr_rewrite_leaf_num(tmpl_t *vpt, int16_t num) CC_HINT(nonnull);
 
 void			tmpl_attr_set_request_ref(tmpl_t *vpt, FR_DLIST_HEAD(tmpl_request_list) const *request_def) CC_HINT(nonnull);
 
diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c
index ee39fe842d..f4f79c444a 100644
--- a/src/lib/server/tmpl_tokenize.c
+++ b/src/lib/server/tmpl_tokenize.c
@@ -1175,28 +1175,16 @@ int tmpl_attr_set_leaf_da(tmpl_t *vpt, fr_dict_attr_t const *da)
 	return 0;
 }
 
-void tmpl_attr_set_leaf_num(tmpl_t *vpt, int16_t num)
-{
-	tmpl_attr_t *ar;
-
-	tmpl_assert_type(tmpl_is_attr(vpt) || tmpl_is_attr_unresolved(vpt));
-
-	if (tmpl_attr_list_num_elements(tmpl_attr(vpt)) == 0) {
-		ar = tmpl_attr_add(vpt, TMPL_ATTR_TYPE_UNKNOWN);
-	} else {
-		ar = tmpl_attr_list_tail(tmpl_attr(vpt));
-	}
-
-	ar->ar_filter_type = TMPL_ATTR_FILTER_TYPE_INDEX;
-	ar->ar_num = num;
-
-	TMPL_ATTR_VERIFY(vpt);
-}
-
 /** Rewrite the leaf's instance number
  *
+ *  This function is _only_ called from the compiler, for "update" and "foreach" keywords.  In those cases,
+ *  the user historically did "&foo-bar", but really meant "&foo-bar[*]".  We silently update that for
+ *  "update" sections, and complain about it in "foreach" sections.
+ *
+ *  As the server now supports multiple types of leaf references, we do the rewrite _only_ from "none" (no
+ *  filter), OR where it's a numerical index, AND the index hasn't been specified.
  */
-void tmpl_attr_rewrite_leaf_num(tmpl_t *vpt, int16_t from, int16_t to)
+void tmpl_attr_rewrite_leaf_num(tmpl_t *vpt, int16_t to)
 {
 	tmpl_attr_t *ref = NULL;
 
@@ -1205,7 +1193,17 @@ void tmpl_attr_rewrite_leaf_num(tmpl_t *vpt, int16_t from, int16_t to)
 	if (tmpl_attr_list_num_elements(tmpl_attr(vpt)) == 0) return;
 
 	ref = tmpl_attr_list_tail(tmpl_attr(vpt));
-	if (ref->ar_num == from) ref->ar_num = to;
+
+	if (ref->ar_filter_type == TMPL_ATTR_FILTER_TYPE_NONE) {
+		ref->ar_filter_type = TMPL_ATTR_FILTER_TYPE_INDEX;
+		ref->ar_num = to;
+
+	} else if (ref->ar_filter_type != TMPL_ATTR_FILTER_TYPE_INDEX) {
+		return;
+
+	} else if (ref->ar_num == NUM_UNSPEC) {
+		ref->ar_num = to;
+	}
 
 	TMPL_ATTR_VERIFY(vpt);
 }
diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index 3ff2a331b2..bcd193e624 100644
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -484,25 +484,13 @@ int unlang_fixup_update(map_t *map, void *ctx)
 		}
 	}
 
-	/*
-	 *	Fixup LHS attribute references to change NUM_UNSPEC to NUM_ALL, but only for "update" sections.
-	 */
 	if (!ctx) {
-		switch (map->lhs->type) {
-		case TMPL_TYPE_ATTR:
-			if (!tmpl_is_list(map->lhs)) tmpl_attr_rewrite_leaf_num(map->lhs, NUM_UNSPEC, NUM_ALL);
-			break;
-
-		default:
-			break;
-		}
-
 		/*
 		 *	Fixup RHS attribute references to change NUM_UNSPEC to NUM_ALL.
 		 */
 		switch (map->rhs->type) {
 		case TMPL_TYPE_ATTR:
-			if (!tmpl_is_list(map->rhs)) tmpl_attr_rewrite_leaf_num(map->rhs, NUM_UNSPEC, NUM_ALL);
+			if (!tmpl_is_list(map->rhs)) tmpl_attr_rewrite_leaf_num(map->rhs, NUM_ALL);
 			break;
 
 		default:
@@ -3308,7 +3296,7 @@ static unlang_t *compile_foreach(unlang_t *parent, unlang_compile_t *unlang_ctx,
 
 	if (tmpl_attr_tail_num(vpt) == NUM_UNSPEC) {
 		cf_log_warn(cs, "Attribute reference should be updated to use %s[*]", vpt->name);
-		tmpl_attr_set_leaf_num(vpt, NUM_ALL);
+		tmpl_attr_rewrite_leaf_num(vpt, NUM_ALL);
 	}
 
 	if (tmpl_attr_tail_num(vpt) != NUM_ALL) {