make fr_pair_update_by_da_parent() do nested attributes

which automatically means that modules like mschap will create
attributes with the correct hierarchy.

Note that ALL callers of pair_update_request() and pair_update_reply()
will be affected.  However, most of them create attributes at the
"root", and will not be affected by this change.

diff --git a/src/lib/server/pair.h b/src/lib/server/pair.h
index 3a7b85e6d9fd12a0eb773fbd94caccab1a804672..0a9a608ae8b574439458ac723bb4c4719810504a 100644 (file)
--- a/src/lib/server/pair.h
+++ b/src/lib/server/pair.h
@@ -115,7 +115,7 @@ RCSIDH(server_pair_h, "$Id$")
  *     - 0 if we allocated a new attribute.
  *     - -1 on failure.
  */
-#define pair_update_request(_attr, _da) fr_pair_update_by_da(request->request_ctx, _attr, _da)
+#define pair_update_request(_attr, _da) fr_pair_update_by_da_parent(request->request_ctx, _attr, _da)
 
 /** Return or allocate a fr_pair_t in the reply list
  *
@@ -126,7 +126,7 @@ RCSIDH(server_pair_h, "$Id$")
  *     - 0 if we allocated a new attribute.
  *     - -1 on failure.
  */
-#define pair_update_reply(_attr, _da) fr_pair_update_by_da(request->reply_ctx, _attr, _da)
+#define pair_update_reply(_attr, _da) fr_pair_update_by_da_parent(request->reply_ctx, _attr, _da)
 
 /** Return or allocate a fr_pair_t in the control list
  *
@@ -137,7 +137,7 @@ RCSIDH(server_pair_h, "$Id$")
  *     - 0 if we allocated a new attribute.
  *     - -1 on failure.
  */
-#define pair_update_control(_attr, _da) fr_pair_update_by_da(request->control_ctx, _attr, _da)
+#define pair_update_control(_attr, _da) fr_pair_update_by_da_parent(request->control_ctx, _attr, _da)
 
 /** Return or allocate a fr_pair_t in the session_state list
  *
@@ -148,7 +148,7 @@ RCSIDH(server_pair_h, "$Id$")
  *     - 0 if we allocated a new attribute.
  *     - -1 on failure.
  */
-#define pair_update_session_state(_attr, _da) fr_pair_update_by_da(request->session_state_ctx, _attr, _da)
+#define pair_update_session_state(_attr, _da) fr_pair_update_by_da_parent(request->session_state_ctx, _attr, _da)
 
 /** Delete one or move fr_pair_t in a list
  *

diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index 9847e0a6d6f0decb82869a9a1b63bcfc290e1a7b..6f0d6e81f2d8abf8721fba8827f78f75aad19b32 100644 (file)
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -1488,28 +1488,69 @@ int fr_pair_append_by_da_parent(TALLOC_CTX *ctx, fr_pair_t **out, fr_pair_list_t
  *     - 0 if we allocated a new attribute.
  *     - -1 on failure.
  */
-int fr_pair_update_by_da(fr_pair_t *parent, fr_pair_t **out,
-                        fr_dict_attr_t const *da)
+int fr_pair_update_by_da_parent(fr_pair_t *parent, fr_pair_t **out,
+                               fr_dict_attr_t const *da)
 {
-       fr_pair_t       *vp;
+       fr_pair_t               *vp = NULL;
+       fr_da_stack_t           da_stack;
+       fr_dict_attr_t const    **find;
+       TALLOC_CTX              *pair_ctx = parent;
+       fr_pair_list_t          *list = &parent->vp_group;
 
-       vp = fr_pair_find_by_da_idx(&parent->vp_group, da, 0);
-       if (vp) {
-               PAIR_VERIFY_WITH_LIST(&parent->vp_group, vp);
-               if (out) *out = vp;
-               return 1;
-       }
+       /*
+        *      Fast path for non-nested attributes
+        */
+       if (da->depth <= 1) {
+               vp = fr_pair_find_by_da(list, NULL, da);
+               if (vp) {
+                       *out = vp;
+                       return 1;
+               }
 
-       vp = fr_pair_afrom_da(parent, da);
-       if (unlikely(!vp)) {
-               if (out) *out = NULL;
-               return -1;
+               return fr_pair_append_by_da(parent, out, list, da);
        }
 
-       fr_pair_append(&parent->vp_group, vp);
-       if (out) *out = vp;
+       fr_proto_da_stack_build(&da_stack, da);
+       find = &da_stack.da[0];
 
-       return 0;
+       /*
+        *      Walk down the da stack looking for candidate parent
+        *      attributes and then allocating the leaf.
+        */
+       while (true) {
+               fr_assert((*find)->depth <= da->depth);
+
+               /*
+                *      We're not at the leaf, look for a potential parent
+                */
+               if ((*find) != da) vp = fr_pair_find_by_da(list, NULL, *find);
+
+               /*
+                *      Nothing found, create the pair
+                */
+               if (!vp) {
+                       if (fr_pair_append_by_da(pair_ctx, &vp, list, *find) < 0) {
+                               *out = NULL;
+                               return -1;
+                       }
+               }
+
+               /*
+                *      We're at the leaf, return
+                */
+               if ((*find) == da) {
+                       *out = vp;
+                       return 0;
+               }
+
+               /*
+                *      Prepare for next level
+                */
+               list = &vp->vp_group;
+               pair_ctx = vp;
+               vp = NULL;
+               find++;
+       }
 }
 
 /** Delete matching pairs from the specified list

diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h
index 4748d12cefdfcb9cc7b9d1d76f2453d06e4031fe..ee1cea78c3fc57b163dc79d9151e92d10f8681e1 100644 (file)
--- a/src/lib/util/pair.h
+++ b/src/lib/util/pair.h
@@ -501,8 +501,8 @@ int         fr_pair_prepend_by_da(TALLOC_CTX *ctx, fr_pair_t **out, fr_pair_list_t *lis
 int            fr_pair_append_by_da_parent(TALLOC_CTX *ctx, fr_pair_t **out, fr_pair_list_t *list,
                                            fr_dict_attr_t const *da) CC_HINT(nonnull(3,4));
 
-int            fr_pair_update_by_da(fr_pair_t *parent, fr_pair_t **out,
-                                    fr_dict_attr_t const *da) CC_HINT(nonnull(1,3));
+int            fr_pair_update_by_da_parent(fr_pair_t *parent, fr_pair_t **out,
+                                           fr_dict_attr_t const *da) CC_HINT(nonnull(1,3));
 
 int            fr_pair_delete_by_da(fr_pair_list_t *head, fr_dict_attr_t const *da) CC_HINT(nonnull);
 

diff --git a/src/lib/util/pair_tests.c b/src/lib/util/pair_tests.c
index 0549cd998a4a0eec481c7344be58956490a59269..c8fb7538896c98fed834f94ae78fdfd7cd2de3ce 100644 (file)
--- a/src/lib/util/pair_tests.c
+++ b/src/lib/util/pair_tests.c
@@ -523,14 +523,14 @@ static void test_fr_pair_prepend_by_da(void)
        fr_pair_list_free(&local_pairs);
 }
 
-static void test_fr_pair_update_by_da(void)
+static void test_fr_pair_update_by_da_parent(void)
 {
        fr_pair_t *vp, *group;
 
        TEST_CHECK((group = fr_pair_afrom_da(autofree, fr_dict_attr_test_group)) != NULL);
 
        TEST_CASE("Update Add using fr_pair_prepend_by_da()");
-       TEST_CHECK(fr_pair_update_by_da(group, &vp, fr_dict_attr_test_uint32) == 0); /* attribute doesn't exist in this group */
+       TEST_CHECK(fr_pair_update_by_da_parent(group, &vp, fr_dict_attr_test_uint32) == 0); /* attribute doesn't exist in this group */
        vp->vp_uint32 = 54321;
 
        TEST_CASE("Expected fr_dict_attr_test_uint32 (vp->vp_uint32 == 54321)");
@@ -1360,7 +1360,7 @@ TEST_LIST = {
        { "fr_pair_prepend_by_da",                test_fr_pair_prepend_by_da },
        { "fr_pair_append_by_da_parent",          test_fr_pair_append_by_da_parent },
        { "fr_pair_delete_by_child_num",          test_fr_pair_delete_by_child_num },
-       { "fr_pair_update_by_da",                 test_fr_pair_update_by_da },
+       { "fr_pair_update_by_da_parent",          test_fr_pair_update_by_da_parent },
        { "fr_pair_delete",                       test_fr_pair_delete },
        { "fr_pair_delete_by_da",                 test_fr_pair_delete_by_da },