fix xlats for virtual modules

don't even bother for "group".

Register the xlats in the "bootstrap" stage, by walking over the
virtual modules.

switch xlat_redundant() etc. to use the new XLAT API.

diff --git a/src/lib/server/module.c b/src/lib/server/module.c
index f8d479d4ba93e1860148526188bf7c9934fa891d..c3c8c58a6c0aae2e7361db9b46c1f933d1b2cd89 100644 (file)
--- a/src/lib/server/module.c
+++ b/src/lib/server/module.c
@@ -61,6 +61,7 @@ typedef struct {
        fr_rb_node_t                    name_node;      //!< Entry in the name tree.
        char const                      *name;          //!< module name
        CONF_SECTION                    *cs;            //!< CONF_SECTION where it is defined
+       bool                            all_same;
 } virtual_module_t;
 
 
@@ -1627,7 +1628,7 @@ CONF_SECTION *module_by_name_virtual(char const *asked_name)
 static int virtual_module_bootstrap(CONF_SECTION *cs)
 {
        char const              *name;
-       bool                    all_same = true;
+       bool                    all_same;
        module_t const  *last = NULL;
        CONF_ITEM               *sub_ci = NULL;
        CONF_PAIR               *cp;
@@ -1672,6 +1673,11 @@ static int virtual_module_bootstrap(CONF_SECTION *cs)
                return -1;
        }
 
+       /*
+        *      Don't bother registering redundant xlats for a simple "group".
+        */
+       all_same = (strcmp(cf_section_name1(cs), "group") != 0);
+
        /*
         *      Ensure that the modules we reference here exist.
         */
@@ -1687,6 +1693,10 @@ static int virtual_module_bootstrap(CONF_SECTION *cs)
                         *      Allow "foo.authorize" in subsections.
                         *
                         *      Note that we don't care what the method is, just that it exists.
+                        *
+                        *      This check is needed only because we
+                        *      want to know if we need to register a
+                        *      redundant xlat for the virtual module.
                         */
                        mi = module_by_name_and_method(NULL, NULL, NULL, NULL, cf_pair_attr(cp));
                        if (!mi) {
@@ -1708,20 +1718,17 @@ static int virtual_module_bootstrap(CONF_SECTION *cs)
                }
 
                /*
-                *      Don't check subsections for now.
+                *      Don't check subsections for now.  That check
+                *      happens later in the unlang compiler.
                 */
-       } /* loop over modules in a "redundant foo" section */
-
-       /*
-        *      Register a redundant xlat
-        */
-       if (all_same && (xlat_register_legacy_redundant(cs) < 0)) return -1;
+       } /* loop over things in a virtual module section */
 
        inst = talloc_zero(cs, virtual_module_t);
        if (!inst) return -1;
 
        inst->cs = cs;
        inst->name = talloc_strdup(inst, name);
+       inst->all_same = all_same;
 
        if (!fr_cond_assert(fr_rb_insert(virtual_module_name_tree, inst))) {
                talloc_free(inst);
@@ -1745,6 +1752,8 @@ int modules_bootstrap(CONF_SECTION *root)
 {
        CONF_ITEM *ci;
        CONF_SECTION *cs, *modules;
+       virtual_module_t        *vm;
+       fr_rb_iter_inorder_t    iter;
 
        /*
         *      Remember where the modules were stored.
@@ -1860,5 +1869,18 @@ int modules_bootstrap(CONF_SECTION *root)
                }
        }
 
+       /*
+        *      Now that all of the xlat things have been registered,
+        *      register our redundant xlats.  But only when all of
+        *      the items in such a section are the same.
+        */
+       for (vm = fr_rb_iter_init_inorder(&iter, virtual_module_name_tree);
+            vm;
+            vm = fr_rb_iter_next_inorder(&iter)) {
+               if (!vm->all_same) continue;
+
+               if (xlat_register_redundant(vm->cs) < 0) return -1;
+       }
+
        return 0;
 }

diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h
index 58bc4a15ebaff93ff2a4e0ca156b0f76477d63df..05313f1a5efddcab6f3f351c4defbba02ac8528a 100644 (file)
--- a/src/lib/unlang/xlat.h
+++ b/src/lib/unlang/xlat.h
@@ -409,7 +409,7 @@ void _xlat_async_thread_instantiate_set(xlat_t const *xlat,
 
 void           xlat_unregister(char const *name);
 void           xlat_unregister_module(void *instance);
-int            xlat_register_legacy_redundant(CONF_SECTION *cs);
+int            xlat_register_redundant(CONF_SECTION *cs);
 int            xlat_init(void);
 void           xlat_free(void);
 

diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c
index 9a27e0dde61802ac3a20dd61a720de945d0a7fa2..3014939535117888449fa0e1898f589ddf97aa90 100644 (file)
--- a/src/lib/unlang/xlat_builtin.c
+++ b/src/lib/unlang/xlat_builtin.c
@@ -515,6 +515,16 @@ typedef struct {
 } xlat_redundant_t;
 
 
+/** Make module instance available to xlats
+ *
+ */
+static int xlat_redundant_instantiate(void *xlat_inst, UNUSED xlat_exp_t const *exp, void *uctx)
+{
+       *((void **)xlat_inst) = talloc_get_type_abort(uctx, xlat_redundant_t);
+       return 0;
+}
+
+
 /** xlat "redundant" processing
  *
  * Processes xlat calls for modules defined in "redundant"
@@ -522,16 +532,18 @@ typedef struct {
  *
  * @ingroup xlat_functions
  */
-static ssize_t xlat_redundant(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size_t outlen,
-                             void const *mod_inst, UNUSED void const *xlat_inst,
-                             request_t *request, char const *fmt)
+static xlat_action_t xlat_redundant(TALLOC_CTX *ctx, fr_dcursor_t *out,
+                                   request_t *request, void const *xlat_inst,
+                                   UNUSED void *xlat_thread_inst, fr_value_box_list_t *in)
 {
-       xlat_redundant_t const *xr = mod_inst;
+       xlat_redundant_t const *xr;
        CONF_ITEM *ci;
        char const *name;
        xlat_t *xlat;
 
-       fr_assert((*out == NULL) && (outlen == 0));     /* Caller must not have allocated buf */
+       memcpy(&xr, xlat_inst, sizeof(xr));
+       xr = talloc_get_type_abort(xr, xlat_redundant_t);
+
        fr_assert(xr->type == XLAT_REDUNDANT);
 
        /*
@@ -540,36 +552,26 @@ static ssize_t xlat_redundant(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size_t
        for (ci = cf_item_next(xr->cs, NULL);
             ci != NULL;
             ci = cf_item_next(xr->cs, ci)) {
-               ssize_t ret;
-
                if (!cf_item_is_pair(ci)) continue;
 
                name = cf_pair_attr(cf_item_to_pair(ci));
                fr_assert(name != NULL);
 
+               /*
+                *      @todo - cache these in a fixed size array in
+                *      the xlat_inst, which should save some run-time
+                *      cost.
+                */
                xlat = xlat_func_find(name, -1);
                if (!xlat) continue;
 
-               if (xlat->buf_len > 0) {
-                       *out = talloc_array(ctx, char, xlat->buf_len);
-                       **out = '\0';   /* Be sure the string is \0 terminated */
-               } else {
-                       *out = NULL;
-               }
-
-               ret = xlat->func.sync(ctx, out, xlat->buf_len, xlat->mod_inst, NULL, request, fmt);
-               if (ret <= 0) {
-                       TALLOC_FREE(*out);
-                       continue;
-               }
-               return ret;
+               return xlat->func.async(ctx, out, request, xlat->mod_inst, xlat->thread_uctx, in);
        }
 
        /*
-        *      Everything failed.  Oh well.
+        *      Everything failed.  Don't modify the output.  Just return failure.
         */
-       *out = NULL;
-       return 0;
+       return XLAT_ACTION_FAIL;
 }
 
 
@@ -580,18 +582,19 @@ static ssize_t xlat_redundant(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size_t
  *
  * @ingroup xlat_functions
  */
-static ssize_t xlat_load_balance(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size_t outlen,
-                                void const *mod_inst, UNUSED void const *xlat_inst,
-                                request_t *request, char const *fmt)
+static xlat_action_t xlat_load_balance(TALLOC_CTX *ctx, fr_dcursor_t *out,
+                                      request_t *request, void const *xlat_inst,
+                                      UNUSED void *xlat_thread_inst, fr_value_box_list_t *in)
 {
        uint32_t count = 0;
-       xlat_redundant_t const *xr = mod_inst;
+       xlat_redundant_t const *xr;
        CONF_ITEM *ci;
        CONF_ITEM *found = NULL;
        char const *name;
        xlat_t *xlat;
 
-       fr_assert((*out == NULL) && (outlen == 0));     /* Caller must not have allocated buf */
+       memcpy(&xr, xlat_inst, sizeof(xr));
+       xr = talloc_get_type_abort(xr, xlat_redundant_t);
 
        /*
         *      Choose a child at random.
@@ -615,23 +618,18 @@ static ssize_t xlat_load_balance(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size
         *      Plain load balancing: do one child, and only one child.
         */
        if (xr->type == XLAT_LOAD_BALANCE) {
-               ssize_t slen;
                name = cf_pair_attr(cf_item_to_pair(found));
                fr_assert(name != NULL);
 
+               /*
+                *      @todo - cache these in a fixed size array in
+                *      the xlat_inst, which should save some run-time
+                *      cost.
+                */
                xlat = xlat_func_find(name, -1);
                if (!xlat) return -1;
 
-               if (xlat->buf_len > 0) {
-                       *out = talloc_array(ctx, char, xlat->buf_len);
-                       **out = '\0';   /* Be sure the string is \0 terminated */
-               } else {
-                       *out = NULL;
-               }
-               slen = xlat->func.sync(ctx, out, xlat->buf_len, xlat->mod_inst, NULL, request, fmt);
-               if (slen <= 0) TALLOC_FREE(*out);
-
-               return slen;
+               return xlat->func.async(ctx, out, request, xlat->mod_inst, xlat->thread_uctx, in);
        }
 
        fr_assert(xr->type == XLAT_REDUNDANT_LOAD_BALANCE);
@@ -647,17 +645,14 @@ static ssize_t xlat_load_balance(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size
 
                xlat = xlat_func_find(name, -1);
                if (xlat) {
-                       ssize_t ret;
+                       xlat_action_t xa;
 
-                       if (xlat->buf_len > 0) {
-                               *out = talloc_array(ctx, char, xlat->buf_len);
-                               **out = '\0';   /* Be sure the string is \0 terminated */
-                       } else {
-                               *out = NULL;
-                       }
-                       ret = xlat->func.sync(ctx, out, xlat->buf_len, xlat->mod_inst, NULL, request, fmt);
-                       if (ret > 0) return ret;
-                       TALLOC_FREE(*out);
+                       /*
+                        *      The function shouldn't muck with the
+                        *      output list, unless it succeeds.
+                        */
+                       xa = xlat->func.async(ctx, out, request, xlat->mod_inst, xlat->thread_uctx, in);
+                       if (xa != XLAT_ACTION_FAIL) return xa;
                }
 
                /*
@@ -667,7 +662,7 @@ static ssize_t xlat_load_balance(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size
                if (!ci) ci = cf_item_next(xr->cs, NULL);
        } while (ci != found);
 
-       return -1;
+       return XLAT_ACTION_FAIL;
 }
 
 
@@ -686,10 +681,13 @@ static ssize_t xlat_load_balance(TALLOC_CTX *ctx, char **out, NDEBUG_UNUSED size
  *     - -1 on error.
  *     - 1 if the modules in the section do not have an xlat method.
  */
-int xlat_register_legacy_redundant(CONF_SECTION *cs)
+int xlat_register_redundant(CONF_SECTION *cs)
 {
        char const *name1, *name2;
        xlat_redundant_t *xr;
+       xlat_t const *xlat, *old = NULL;
+       CONF_ITEM *ci = NULL;
+       xlat_func_t func;
 
        name1 = cf_section_name1(cs);
        name2 = cf_section_name2(cs);
@@ -703,10 +701,16 @@ int xlat_register_legacy_redundant(CONF_SECTION *cs)
 
        if (strcmp(name1, "redundant") == 0) {
                xr->type = XLAT_REDUNDANT;
+               func = xlat_redundant;
+
        } else if (strcmp(name1, "redundant-load-balance") == 0) {
                xr->type = XLAT_REDUNDANT_LOAD_BALANCE;
+               func = xlat_load_balance;
+
        } else if (strcmp(name1, "load-balance") == 0) {
                xr->type = XLAT_LOAD_BALANCE;
+               func = xlat_load_balance;
+
        } else {
                fr_assert(0);
        }
@@ -714,44 +718,45 @@ int xlat_register_legacy_redundant(CONF_SECTION *cs)
        xr->cs = cs;
 
        /*
-        *      Get the number of children for load balancing.
+        *      Count the number of children for load-balance, and
+        *      also find out a little bit more about the old xlats.
         */
-       if (xr->type == XLAT_REDUNDANT) {
-               if (!xlat_register_legacy(xr, name2, xlat_redundant, NULL, NULL, 0, 0)) {
-                       ERROR("Registering xlat for redundant section failed");
-                       talloc_free(xr);
-                       return -1;
-               }
-
-       } else {
-               CONF_ITEM *ci = NULL;
+       while ((ci = cf_item_next(cs, ci))) {
+               char const *attr;
 
-               while ((ci = cf_item_next(cs, ci))) {
-                       char const *attr;
+               if (!cf_item_is_pair(ci)) continue;
 
-                       if (!cf_item_is_pair(ci)) continue;
+               attr = cf_pair_attr(cf_item_to_pair(ci));
 
-                       attr = cf_pair_attr(cf_item_to_pair(ci));
+               /*
+                *      This is ok, it just means the module
+                *      doesn't have an xlat method.
+                */
+               old = xlat_func_find(attr, -1);
+               if (!old) {
+                       talloc_free(xr);
+                       return 1;
+               }
 
-                       /*
-                        *      This is ok, it just means the module
-                        *      doesn't have an xlat method.
-                        */
-                       if (!xlat_func_find(attr, -1)) {
-                               talloc_free(xr);
-                               return 1;
-                       }
+               xr->count++;
+       }
 
-                       xr->count++;
-               }
+       /*
+        *      At least one "old" xlat has to exist.  Look at it in
+        *      order to find out which arguments we need to pass to
+        *      xlat_register()
+        */
+       if (!old) return 1;
 
-               if (!xlat_register_legacy(xr, name2, xlat_load_balance, NULL, NULL, 0, 0)) {
-                       ERROR("Registering xlat for load-balance section failed");
-                       talloc_free(xr);
-                       return -1;
-               }
+       xlat = xlat_register(NULL, name2, func, old->needs_async);
+       if (!xlat) {
+               ERROR("Registering xlat for load-balance section failed");
+               talloc_free(xr);
+               return -1;
        }
 
+       xlat_async_instantiate_set(xlat, xlat_redundant_instantiate, xlat_redundant_t *, NULL, xr);
+
        return 0;
 }
 

diff --git a/src/tests/keywords/virtual_xlat b/src/tests/keywords/virtual_xlat
new file mode 100644 (file)
index 0000000..bc63c93
--- /dev/null
+++ b/src/tests/keywords/virtual_xlat
@@ -0,0 +1,8 @@
+#
+#  PRE: if
+#
+if ("%{redundant_test:foo bar}" != "foo bar") {
+       test_fail
+}
+
+success