do less copying when editing lists

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 88a054487410db45861f9d9f94cae71b7d1c402d..7027a16e556a3dc0340fb23d1549030ce236353d 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -277,9 +277,11 @@ static int remove_vps(request_t *request, unlang_frame_state_edit_t *state, edit
  */
 static int apply_edits(request_t *request, unlang_frame_state_edit_t *state, edit_map_t *current, map_t const *map)
 {
-       fr_pair_t *vp, *vp_to_free = NULL;
+       fr_pair_t *vp;
        fr_pair_list_t *children;
        fr_value_box_t const *rhs_box = NULL;
+       bool copy_vps = true;
+       int rcode;
 
        fr_assert(current->lhs.vp != NULL);
 
@@ -289,6 +291,7 @@ static int apply_edits(request_t *request, unlang_frame_state_edit_t *state, edi
 
        if (!current->rhs.vpt) {
                children = &current->rhs.pair_list;
+               copy_vps = false;
                goto apply_list;
        }
 
@@ -314,6 +317,7 @@ static int apply_edits(request_t *request, unlang_frame_state_edit_t *state, edi
                if (fr_type_is_group(da->type)) da = fr_dict_root(request->dict);
 
                children = &current->rhs.pair_list;
+               copy_vps = false;
 
                switch (rhs_box->type) {
                case FR_TYPE_STRING:
@@ -414,15 +418,16 @@ static int apply_edits(request_t *request, unlang_frame_state_edit_t *state, edi
         *      treats the RHS as a one-member list.
         */
        if (fr_type_is_leaf(vp->vp_type)) {
-               vp_to_free = fr_pair_copy(request, vp);
-               if (!vp_to_free) return -1;
+               fr_pair_t *vp_copy;
+
+               vp_copy = fr_pair_copy(request, vp);
+               if (!vp_copy) return -1;
 
                fr_assert(fr_pair_list_empty(&current->rhs.pair_list));
 
-               fr_pair_append(&current->rhs.pair_list, vp_to_free);
+               fr_pair_append(&current->rhs.pair_list, vp_copy);
                children = &current->rhs.pair_list;
-
-               vp_to_free = NULL; /* it's not in the pair list, and will be freed there */
+               copy_vps = false;
 
        } else {
                /*
@@ -440,7 +445,7 @@ static int apply_edits(request_t *request, unlang_frame_state_edit_t *state, edi
                        return -1;
                }
 
-               children = &vp->children;
+               children = &vp->vp_group; /* and copy_vps for any VP we edit */
        }
 
        /*
@@ -465,17 +470,14 @@ apply_list:
                RDEBUG2("}");
        }
 
-       if (fr_edit_list_apply_list_assignment(state->el,
-                                              current->lhs.vp,
-                                              map->op,
-                                              children) < 0) {
-               RPERROR("Failed performing list %s operation", fr_tokens[map->op]);
-               talloc_free(vp_to_free);
-               return -1;
-       }
+       rcode = fr_edit_list_apply_list_assignment(state->el, current->lhs.vp, map->op, children, copy_vps);
+       if (rcode < 0) RPERROR("Failed performing list %s operation", fr_tokens[map->op]);
 
-       talloc_free(vp_to_free);
-       return 0;
+       /*
+        *      If the child list wasn't copied, then we just created it, and we need to free it.
+        */
+       if (!copy_vps) fr_pair_list_free(children);
+       return rcode;
 
 leaf:
        /*

diff --git a/src/lib/util/edit.c b/src/lib/util/edit.c
index 4c78e389c5ffef166748e72a707e8f85c7b9f77e..bc21cb6b7ace2d03704faf018baebfbd22602f16 100644 (file)
--- a/src/lib/util/edit.c
+++ b/src/lib/util/edit.c
@@ -747,13 +747,21 @@ int fr_edit_list_apply_pair_assignment(fr_edit_list_t *el, fr_pair_t *vp, fr_tok
        return fr_value_calc_assignment_op(vp, &vp->data, op, in);
 }
 
+#undef COPY
+#define COPY(_x) do { if (copy) { \
+                       c = fr_pair_copy(dst, _x); \
+                       if (!c) return -1; \
+                      } else { \
+                       c = talloc_steal(dst, _x); \
+                     } \
+                } while (0)
 
 /** A UNION B
  *
  */
-static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
+static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src, bool copy)
 {
-       fr_pair_t *a, *b;
+       fr_pair_t *a, *b, *c;
        fr_dcursor_t cursor1, cursor2;
 
        /*
@@ -791,7 +799,9 @@ static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
                 *      A is done, so we always union in B at the end of A.
                 */
                if (!a) {
-                       if (fr_edit_list_insert_pair_tail(el, &dst->children, fr_pair_copy(dst, b)) < 0) {
+                       COPY(b);
+
+                       if (fr_edit_list_insert_pair_tail(el, &dst->children, c) < 0) {
                                return -1;
                        }
 
@@ -821,7 +831,9 @@ static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
                 *      to the destination list, before A.
                 */
                if (rcode > 0) {
-                       if (fr_edit_list_insert_pair_before(el, &dst->children, a, fr_pair_copy(dst, b)) < 0) {
+                       COPY(b);
+
+                       if (fr_edit_list_insert_pair_before(el, &dst->children, a, c) < 0) {
                                return -1;
                        }
 
@@ -849,7 +861,7 @@ static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
                 *      been added in.
                 */
                if (fr_type_is_structural(a->vp_type)) {
-                       rcode = list_union(el, a, &b->children);
+                       rcode = list_union(el, a, &b->children, copy);
                        if (rcode < 0) return rcode;
 
                        fr_dcursor_next(&cursor1);
@@ -857,10 +869,8 @@ static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
                        continue;
                }
 
-               /*
-                *
-                */
-               if (fr_edit_list_insert_pair_after(el, &dst->children, a, fr_pair_copy(dst, b)) < 0) {
+               COPY(b);
+               if (fr_edit_list_insert_pair_after(el, &dst->children, a, c) < 0) {
                        return -1;
                }
 
@@ -877,9 +887,9 @@ static int list_union(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
  *
  * with priority to A
  */
-static int list_merge_lhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
+static int list_merge_lhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src, bool copy)
 {
-       fr_pair_t *a, *b;
+       fr_pair_t *a, *b, *c;
        fr_dcursor_t cursor1, cursor2;
 
        fr_pair_list_sort(&dst->children, fr_pair_cmp_by_parent_num);
@@ -903,7 +913,9 @@ static int list_merge_lhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
                 *      A is done, so we always merge in B at the end of A.
                 */
                if (!a) {
-                       if (fr_edit_list_insert_pair_tail(el, &dst->children, fr_pair_copy(dst, b)) < 0) {
+                       COPY(b);
+
+                       if (fr_edit_list_insert_pair_tail(el, &dst->children, c) < 0) {
                                return -1;
                        }
 
@@ -933,7 +945,9 @@ static int list_merge_lhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
                 *      to before A.
                 */
                if (rcode > 0) {
-                       if (fr_edit_list_insert_pair_before(el, &dst->children, a, fr_pair_copy(dst, b)) < 0) {
+                       COPY(b);
+
+                       if (fr_edit_list_insert_pair_before(el, &dst->children, a, c) < 0) {
                                return -1;
                        }
 
@@ -955,7 +969,7 @@ static int list_merge_lhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
                fr_assert(a->da == b->da);
 
                if (fr_type_is_structural(a->vp_type)) {
-                       rcode = list_merge_lhs(el, a, &b->children);
+                       rcode = list_merge_lhs(el, a, &b->children, copy);
                        if (rcode < 0) return rcode;
                }
 
@@ -973,9 +987,9 @@ static int list_merge_lhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
  *
  * with priority to B
  */
-static int list_merge_rhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src)
+static int list_merge_rhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *src, bool copy)
 {
-       fr_pair_t *a, *b;
+       fr_pair_t *a, *b, *c;
        fr_dcursor_t cursor1, cursor2;
 
        fr_pair_list_sort(&dst->children, fr_pair_cmp_by_parent_num);
@@ -999,7 +1013,9 @@ static int list_merge_rhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
                 *      A is done, so we always merge in B at the end of A.
                 */
                if (!a) {
-                       if (fr_edit_list_insert_pair_tail(el, &dst->children, fr_pair_copy(dst, b)) < 0) {
+                       COPY(b);
+
+                       if (fr_edit_list_insert_pair_tail(el, &dst->children, c) < 0) {
                                return -1;
                        }
 
@@ -1029,7 +1045,9 @@ static int list_merge_rhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
                 *      to before A.
                 */
                if (rcode < 0) {
-                       if (fr_edit_list_insert_pair_before(el, &dst->children, a, fr_pair_copy(dst, b)) < 0) {
+                       COPY(b);
+
+                       if (fr_edit_list_insert_pair_before(el, &dst->children, a, c) < 0) {
                                return -1;
                        }
 
@@ -1051,7 +1069,7 @@ static int list_merge_rhs(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t *sr
                fr_assert(a->da == b->da);
 
                if (fr_type_is_structural(a->vp_type)) {
-                       rcode = list_merge_rhs(el, a, &b->children);
+                       rcode = list_merge_rhs(el, a, &b->children, copy);
                        if (rcode < 0) return rcode;
                }
 
@@ -1182,9 +1200,9 @@ static int list_intersection(fr_edit_list_t *el, fr_pair_t *dst, fr_pair_list_t
  *
  *  The src list is sorted, but is otherwise not modified.
  */
-int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_token_t op, fr_pair_list_t *src)
+int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_token_t op, fr_pair_list_t *src, bool copy)
 {
-       fr_pair_list_t copy;
+       fr_pair_list_t list;
 
        if (!fr_type_is_structural(dst->vp_type)) {
                fr_strerror_printf("Cannot perform list assignment to non-structural type '%s'",
@@ -1192,9 +1210,14 @@ int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_to
                return -1;
        }
 
-#define COPY do { \
-               fr_pair_list_init(&copy); \
-               if (fr_pair_list_copy(dst, &copy, src) < 0) return -1; \
+#undef COPY
+#define COPY do { if (copy) { \
+                   fr_pair_list_init(&list); \
+                   if (fr_pair_list_copy(dst, &list, src) < 0) return -1;\
+                    src = &list; \
+                  } else { \
+                   fr_pair_list_steal(dst, src); \
+                 } \
        } while (0)
 
 
@@ -1216,7 +1239,7 @@ int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_to
                }
 
                COPY;
-               return fr_edit_list_insert_list_tail(el, &dst->children, &copy);
+               return fr_edit_list_insert_list_tail(el, &dst->children, src);
 
        case T_OP_PREPEND:
                if (&dst->children == src) {
@@ -1225,7 +1248,7 @@ int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_to
                }
 
                COPY;
-               return fr_edit_list_insert_list_head(el, &dst->children, &copy);
+               return fr_edit_list_insert_list_head(el, &dst->children, src);
 
        case T_OP_AND_EQ:
                if (&dst->children == src) return 0; /* A INTERSECTION A == A */
@@ -1235,17 +1258,17 @@ int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_to
        case T_OP_OR_EQ:
                if (&dst->children == src) return 0; /* A UNION A == A */
 
-               return list_union(el, dst, src);
+               return list_union(el, dst, src, copy);
 
        case T_OP_GE:
                if (&dst->children == src) return 0; /* A MERGE A == A */
 
-               return list_merge_lhs(el, dst, src);
+               return list_merge_lhs(el, dst, src, copy);
 
        case T_OP_LE:
                if (&dst->children == src) return 0; /* A MERGE A == A */
 
-               return list_merge_rhs(el, dst, src);
+               return list_merge_rhs(el, dst, src, copy);
 
        default:
                break;

diff --git a/src/lib/util/edit.h b/src/lib/util/edit.h
index 835ae7bc0e218aea55f1cbe1d974f74f2f5503e6..f3ef05a2876fd652753c1df5857df5e46bcc766f 100644 (file)
--- a/src/lib/util/edit.h
+++ b/src/lib/util/edit.h
@@ -71,7 +71,7 @@ int fr_edit_list_insert_list_after(fr_edit_list_t *el, fr_pair_list_t *list, fr_
 
 #define fr_edit_list_insert_list_tail(_el, _list, _to_insert) fr_edit_list_insert_list_after(_el, _list, fr_pair_list_tail(_list), _to_insert)
 
-int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_token_t op, fr_pair_list_t *src) CC_HINT(nonnull(1,2,4));
+int fr_edit_list_apply_list_assignment(fr_edit_list_t *el, fr_pair_t *dst, fr_token_t op, fr_pair_list_t *src, bool copy) CC_HINT(nonnull(1,2,4));
 
 #ifdef __cplusplus
 }