clean up handling of := deletion

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 72fe938c298fac503a7d26c33ec873e3b034fd27..211edb8a7f98d7cf25c91f07faded9522d64b896 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -507,20 +507,35 @@ done:
        return rcode;
 }
 
-static int edit_set_eq(request_t *request, edit_map_t *current, bool delete)
+static int edit_delete_lhs(request_t *request, edit_map_t *current, bool delete)
 {
        tmpl_dcursor_ctx_t cc;
        fr_dcursor_t cursor;
-       bool first = fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type);
+
+       /*
+        *      These are magic.
+        */
+       if (delete) {
+               fr_dict_attr_t const *da = tmpl_attr_tail_da(current->lhs.vpt);
+
+               if (fr_type_is_structural(da->type) &&
+                   ((da == request_attr_request) ||
+                    (da == request_attr_reply) ||
+                    (da == request_attr_control) ||
+                    (da == request_attr_state))) {
+                       delete = false;
+               }
+       }
 
        while (true) {
                int err;
                fr_pair_t *vp, *parent;
 
                /*
-                *      Reinitialize the cursor for every VP.  This is because
-                *      fr_dcursor_remove() does not work with tmpl_dcursors, as the
-                *      tmpl_dcursor code does not set the "remove" callback.
+                *      Reinitialize the cursor for every VP.  This is because fr_dcursor_remove() does not
+                *      work with tmpl_dcursors, as the tmpl_dcursor code does not set the "remove" callback.
+                *      And the tmpl is NUM_UNSPEC, which means "the first one", whereas for T_OP_SET_EQ, we
+                *      really mean "delete all except the first one".
                 *
                 *      Once that's implemented, we also need to update the edit list API to
                 *      allow for "please delete children"?
@@ -528,51 +543,29 @@ static int edit_set_eq(request_t *request, edit_map_t *current, bool delete)
                vp = tmpl_dcursor_init(&err, current->ctx, &cc, &cursor, request, current->lhs.vpt);
                if (!vp) break;
 
-               /*
-                *      For structural attributes, we leave the first one, and delete the subsequent
-                *      ones.  That way we leave the main lists alone ("request", "reply", "control", etc.)
-                *
-                *      For leaf attributes, we just skip this step, as "first" is always "false".
-                */
-               if (first) {
-                       first = false;
-                       if (fr_edit_list_free_pair_children(current->el, vp) < 0) return -1;
-                       vp = fr_dcursor_next(&cursor);
-                       if (!vp) goto clear;
-                       continue;
-
-               } else if (fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type)) {
-                       /*
-                        *      We skipped the first structural member, so keep skipping it for all of the next vps.
-                        */
-                       vp = fr_dcursor_next(&cursor);
-                       if (!vp) {
-                       clear:
-                               tmpl_dcursor_clear(&cc);
-                               break;
-                       }
-               }
-
                parent = fr_pair_parent(vp);
                fr_assert(parent != NULL);
 
-               /*
-                *      We can't delete these ones.
-                */
-               fr_assert(vp != request->pair_list.request);
-               fr_assert(vp != request->pair_list.reply);
-               fr_assert(vp != request->pair_list.control);
-               fr_assert(vp != request->pair_list.state);
+               if (!delete) {
+                       if (fr_type_is_structural(vp->vp_type)) {
+                               if (fr_edit_list_free_pair_children(current->el, vp) < 0) return -1;
+                       } else {
+                               /*
+                                *      No need to save value, as fr_edit_list_apply_pair_assignment() will do
+                                *      that for us.
+                                */
+                       }
 
-               /*
-                *      Delete if we're not over-riding a particular value.
-                */
-               if (delete) {
-                       if (fr_edit_list_pair_delete(current->el, &parent->vp_group, vp) < 0) return -1;
+                       current->lhs.vp = vp;
                        tmpl_dcursor_clear(&cc);
-               } else {
-                       goto clear;
+                       return 0;
                }
+
+               /*
+                *      Delete all of them.  We'll create one later for the SET operation.
+                */
+               if (fr_edit_list_pair_delete(current->el, &parent->vp_group, vp) < 0) return -1;
+               tmpl_dcursor_clear(&cc);
        }
 
        return 0;
@@ -623,7 +616,8 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
 
        if (!current->rhs.vpt) {
                /*
-                *      There's no RHS tmpl, so the result must be in in the parent RHS tmpl as data, OR in the RHS result list.
+                *      There's no RHS tmpl, so the result must be in in the parent RHS tmpl as data, OR in
+                *      the RHS result list.
                 */
                if (tmpl_is_data(map->rhs)) {
                        box = tmpl_value(map->rhs);
@@ -672,7 +666,7 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
                fr_assert(tmpl_is_attr(current->rhs.vpt));
 
                /*
-                *      Get a cursor over the pairs.  If there are no matching pairs, then we do nothing.
+                *      Get a cursor over the RHS pairs.
                 */
                vp = tmpl_dcursor_init(&err, request, &cc, &pair_cursor, request, current->rhs.vpt);
                if (!vp) {
@@ -680,7 +674,10 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
 
                        if (map->op != T_OP_SET) return 0;
 
-                       return edit_set_eq(request, current, true);
+                       /*
+                        *      No RHS pairs means we can finally delete all of the LHS.
+                        */
+                       return edit_delete_lhs(request, current, true);
                }
 
                box = fr_pair_dcursor_nested_init(&cursor, &pair_cursor); // the list is unused
@@ -958,7 +955,7 @@ static int check_rhs(request_t *request, unlang_frame_state_edit_t *state, edit_
        if ((map->op == T_OP_SET) &&
            ((tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || (tmpl_attr_tail_num(current->lhs.vpt) > 0) ||
             !current->map->rhs)) {
-               if (edit_set_eq(request, current,
+               if (edit_delete_lhs(request, current,
                                (tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || !current->map->rhs) < 0) return -1;
        }