Add a manual unregister function to the map proc code

- Constify instance data
- Take a separate ctx for map proc allocation
- Link map proc memory using a linking ctx instead of allocating it directly

diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c
index 83c15f9be0f2d11d5320029a020602b6d5acb09a..4a5e1e85d3d3ba634fe9d7c3c57134a32336144b 100644 (file)
--- a/src/bin/unit_test_module.c
+++ b/src/bin/unit_test_module.c
@@ -47,8 +47,6 @@ RCSID("$Id$")
 #  include <getopt.h>
 #endif
 
-#include <ctype.h>
-
 #define EXIT_WITH_FAILURE \
 do { \
        ret = EXIT_FAILURE; \
@@ -586,7 +584,7 @@ static bool do_xlats(fr_event_list_t *el, request_t *request, char const *filena
 /*
  *     Verify the result of the map.
  */
-static int map_proc_verify(CONF_SECTION *cs, UNUSED void *mod_inst, UNUSED void *proc_inst,
+static int map_proc_verify(CONF_SECTION *cs, UNUSED void const *mod_inst, UNUSED void *proc_inst,
                           tmpl_t const *src, UNUSED map_list_t const *maps)
 {
        if (!src) {
@@ -598,7 +596,7 @@ static int map_proc_verify(CONF_SECTION *cs, UNUSED void *mod_inst, UNUSED void
        return 0;
 }
 
-static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, UNUSED void *mod_inst, UNUSED void *proc_inst,
+static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, UNUSED void const *mod_inst, UNUSED void *proc_inst,
                                    UNUSED request_t *request, UNUSED fr_value_box_list_t *src,
                                    UNUSED map_list_t const *maps)
 {
@@ -882,7 +880,7 @@ int main(int argc, char *argv[])
                EXIT_WITH_FAILURE;
        }
 
-       if (map_proc_register(NULL, "test-fail", mod_map_proc, map_proc_verify, 0, 0) < 0) {
+       if (map_proc_register(NULL, NULL, "test-fail", mod_map_proc, map_proc_verify, 0, 0) < 0) {
                EXIT_WITH_FAILURE;
        }
 
@@ -1172,6 +1170,8 @@ cleanup:
        }
 #endif
 
+       map_proc_unregister("test-fail");
+
        /*
         *      Free thread data
         */

diff --git a/src/lib/server/map_proc.c b/src/lib/server/map_proc.c
index 3452d679c08ed0c180c7969b1ff2f00a7b960e86..e58c8c7545d085b62556f5e361984300037cda88 100644 (file)
--- a/src/lib/server/map_proc.c
+++ b/src/lib/server/map_proc.c
@@ -24,13 +24,15 @@
  * @copyright 2015 Arran Cudbard-bell (a.cudbardb@freeradius.org)
  */
 
-
 RCSID("$Id$")
 
 #include <freeradius-devel/server/base.h>
+#include <freeradius-devel/server/map_proc.h>
 #include <freeradius-devel/server/map_proc_priv.h>
+#include <freeradius-devel/util/atexit.h>
 #include <freeradius-devel/util/debug.h>
 #include <freeradius-devel/util/value.h>
+#include <freeradius-devel/util/talloc.h>
 
 static fr_rb_tree_t *map_proc_root = NULL;
 
@@ -70,11 +72,6 @@ static int _map_proc_talloc_free(map_proc_t *proc)
        return 0;
 }
 
-static void _map_proc_tree_free(void *proc)
-{
-       talloc_free(proc);
-}
-
 /** Find a map processor by name
  *
  * @param[in] name of map processor.
@@ -94,10 +91,28 @@ map_proc_t *map_proc_find(char const *name)
        return fr_rb_find(map_proc_root, &find);
 }
 
+static int _map_proc_tree_init(UNUSED void *uctx)
+{
+       MEM(map_proc_root = fr_rb_inline_talloc_alloc(NULL, map_proc_t, node, map_proc_cmp, NULL));
+       return 0;
+}
+
+static int _map_proc_tree_free(UNUSED void *uctx)
+{
+       fr_rb_tree_t *mpr = map_proc_root;
+
+       fr_assert_msg(fr_rb_num_elements(mpr) == 0, "map_proc_t still registered");
+
+       map_proc_root = NULL;
+       talloc_free(mpr);
+       return 0;
+}
+
 /** Register a map processor
  *
  * This should be called by every module that provides a map processing function.
  *
+ * @param[in] ctx              if non-null, the ctx to bind this map processor to.
  * @param[in] mod_inst         of module registering the map_proc.
  * @param[in] name             of map processor. If processor already exists, it is replaced.
  * @param[in] evaluate         Module's map processor function.
@@ -108,7 +123,7 @@ map_proc_t *map_proc_find(char const *name)
  *     - 0 on success.
  *     - -1 on failure.
  */
-int map_proc_register(void *mod_inst, char const *name,
+int map_proc_register(TALLOC_CTX *ctx, void const *mod_inst, char const *name,
                      map_proc_func_t evaluate,
                      map_proc_instantiate_t instantiate, size_t inst_size, fr_value_box_safe_for_t literals_safe_for)
 {
@@ -116,21 +131,21 @@ int map_proc_register(void *mod_inst, char const *name,
 
        fr_assert(name && name[0]);
 
-       if (!map_proc_root) {
-               map_proc_root = fr_rb_inline_talloc_alloc(NULL, map_proc_t, node,
-                                                         map_proc_cmp, _map_proc_tree_free);
-               if (!map_proc_root) {
-                       DEBUG("map_proc: Failed to create tree");
-                       return -1;
-               }
-       }
+       fr_atexit_global_once(_map_proc_tree_init, _map_proc_tree_free, NULL);
 
        /*
         *      If it already exists, replace it.
         */
        proc = map_proc_find(name);
        if (!proc) {
-               proc = talloc_zero(mod_inst, map_proc_t);
+               /*
+               *       Don't allocate directly in the parent ctx, it might be mprotected
+               *       later, and that'll cause segfaults if any of the map_proc_t are still
+               *       protected when we start shuffling the contents of the rbtree.
+               */
+               proc = talloc_zero(NULL, map_proc_t);
+               if (ctx) talloc_link_ctx(ctx, proc);
+
                strlcpy(proc->name, name, sizeof(proc->name));
                proc->length = strlen(proc->name);
 
@@ -153,6 +168,27 @@ int map_proc_register(void *mod_inst, char const *name,
        return 0;
 }
 
+/** Unregister a map processor by name
+ *
+ * @param[in] name     of map processor to unregister.
+ * @return
+ *     - 0 if map processor was found and unregistered.
+ *     - -1 if map processor was not found.
+ */
+int map_proc_unregister(char const *name)
+{
+       map_proc_t *proc;
+
+       proc = map_proc_find(name);
+       if (proc) {
+               talloc_free(proc);
+               return 0;
+       }
+
+       return -1;
+}
+
+
 /** Create a new map proc instance
  *
  * This should be called for every map {} section in the configuration.
@@ -207,14 +243,3 @@ unlang_action_t map_proc(rlm_rcode_t *p_result, request_t *request, map_proc_ins
 {
        return inst->proc->evaluate(p_result, inst->proc->mod_inst, inst->data, request, result, inst->maps);
 }
-
-/** Free all map_processors unregistering them
- *
- */
-void map_proc_free(void)
-{
-       fr_rb_tree_t *mpr = map_proc_root;
-
-       map_proc_root = NULL;
-       talloc_free(mpr);
-}

diff --git a/src/lib/server/map_proc.h b/src/lib/server/map_proc.h
index c75c3d72039fe10d04f1b1e3a4ab0fef99d5b4a5..ea9731fee140ed988b65b82a57081d731c9a8cf4 100644 (file)
--- a/src/lib/server/map_proc.h
+++ b/src/lib/server/map_proc.h
@@ -60,7 +60,7 @@ extern "C" {
  * @param[in] maps             Head of the list of maps to process.
  * @return one of UNLANG_ACTION_*
  */
-typedef unlang_action_t (*map_proc_func_t)(rlm_rcode_t *p_result, void *mod_inst, void *proc_inst, request_t *request,
+typedef unlang_action_t (*map_proc_func_t)(rlm_rcode_t *p_result, void const *mod_inst, void *proc_inst, request_t *request,
                                           fr_value_box_list_t *result, map_list_t const *maps);
 
 /** Allocate new instance data for a map processor
@@ -74,16 +74,17 @@ typedef unlang_action_t (*map_proc_func_t)(rlm_rcode_t *p_result, void *mod_inst
  *     - 0 on success.
  *     - -1 on failure.
  */
-typedef int (*map_proc_instantiate_t)(CONF_SECTION *cs, void *mod_inst, void *proc_inst,
+typedef int (*map_proc_instantiate_t)(CONF_SECTION *cs, void const *mod_inst, void *proc_inst,
                                      tmpl_t const *src, map_list_t const *maps);
 
 map_proc_t     *map_proc_find(char const *name);
 
-void           map_proc_free(void);
-int            map_proc_register(void *mod_inst, char const *name,
+int            map_proc_register(TALLOC_CTX *ctx, void const *mod_inst, char const *name,
                                  map_proc_func_t evaluate,
                                  map_proc_instantiate_t instantiate, size_t inst_size, fr_value_box_safe_for_t safe_for);
 
+int            map_proc_unregister(char const *name);
+
 map_proc_inst_t *map_proc_instantiate(TALLOC_CTX *ctx, map_proc_t const *proc,
                                      CONF_SECTION *cs, tmpl_t const *src, map_list_t const *maps);
 

diff --git a/src/lib/server/map_proc_priv.h b/src/lib/server/map_proc_priv.h
index 4b333bfdc02553d8ca059e3274c9fc091213d154..584571ca507729842812733fc5a3aa284169a561 100644 (file)
--- a/src/lib/server/map_proc_priv.h
+++ b/src/lib/server/map_proc_priv.h
@@ -38,7 +38,7 @@ extern "C" {
  */
 struct map_proc {
        fr_rb_node_t            node;                   //!< Entry in the map processor tree.
-       void                    *mod_inst;              //!< Module instance.
+       void const              *mod_inst;              //!< Module instance.
        char                    name[FR_MAX_STRING_LEN];        //!< Name of the map function.
        int                     length;                 //!< Length of name.
 

diff --git a/src/modules/rlm_client/rlm_client.c b/src/modules/rlm_client/rlm_client.c
index 58d92d13cd82dbe463b7ce53fc18be3a9e5097c1..6f91236ca10ba4feae57345dc813efa804864e61 100644 (file)
--- a/src/modules/rlm_client/rlm_client.c
+++ b/src/modules/rlm_client/rlm_client.c
@@ -116,7 +116,7 @@ static int _map_proc_client_get_vp(TALLOC_CTX *ctx, fr_pair_list_t *out, request
  * @param[in] maps             Head of the map list.
  * @return UNLANG_ACTION_CALCULATE_RESULT
  */
-static unlang_action_t map_proc_client(rlm_rcode_t *p_result, UNUSED void *mod_inst, UNUSED void *proc_inst,
+static unlang_action_t map_proc_client(rlm_rcode_t *p_result, UNUSED void const *mod_inst, UNUSED void *proc_inst,
                                       request_t *request, fr_value_box_list_t *client_override, map_list_t const *maps)
 {
        rlm_rcode_t             rcode = RLM_MODULE_OK;
@@ -357,7 +357,7 @@ static int mod_load(void)
        if (unlikely((xlat = xlat_func_register(NULL, "client", xlat_client, FR_TYPE_STRING)) == NULL)) return -1;
        xlat_func_args_set(xlat, xlat_client_args);
 
-       map_proc_register(NULL, "client", map_proc_client, NULL, 0, 0);
+       map_proc_register(NULL, NULL, "client", map_proc_client, NULL, 0, 0);
 
        return 0;
 }
@@ -365,6 +365,7 @@ static int mod_load(void)
 static void mod_unload(void)
 {
        xlat_func_unregister("client");
+       map_proc_unregister("client");
 }
 
 /*

diff --git a/src/modules/rlm_csv/rlm_csv.c b/src/modules/rlm_csv/rlm_csv.c
index b3221eba1eed0b09356e58d0131f9254cd663da8..d849ad3d0480c5b09db6d65b584fdb3fc40670f2 100644 (file)
--- a/src/modules/rlm_csv/rlm_csv.c
+++ b/src/modules/rlm_csv/rlm_csv.c
@@ -31,7 +31,7 @@ RCSID("$Id$")
 
 #include <freeradius-devel/server/map_proc.h>
 
-static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, void *mod_inst, UNUSED void *proc_inst, request_t *request,
+static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, void const *mod_inst, UNUSED void *proc_inst, request_t *request,
                                    fr_value_box_list_t *key, map_list_t const *maps);
 
 /*
@@ -481,7 +481,7 @@ static int csv_map_verify(map_t *map, void *instance)
 /*
  *     Verify the result of the map.
  */
-static int csv_maps_verify(CONF_SECTION *cs, void *mod_inst, UNUSED void *proc_inst,
+static int csv_maps_verify(CONF_SECTION *cs, void const *mod_inst, UNUSED void *proc_inst,
                          tmpl_t const *src, map_list_t const *maps)
 {
        map_t const *map = NULL;
@@ -496,7 +496,7 @@ static int csv_maps_verify(CONF_SECTION *cs, void *mod_inst, UNUSED void *proc_i
                /*
                 *      This function doesn't change the map, so it's OK.
                 */
-               if (csv_map_verify(UNCONST(map_t *, map), mod_inst) < 0) return -1;
+               if (csv_map_verify(UNCONST(map_t *, map), UNCONST(void *, mod_inst)) < 0) return -1;
        }
 
        return 0;
@@ -737,7 +737,7 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx)
        /*
         *      And register the `map csv <key> { ... }` function.
         */
-       map_proc_register(inst, mctx->mi->name, mod_map_proc, csv_maps_verify, 0, 0);
+       map_proc_register(inst, inst, mctx->mi->name, mod_map_proc, csv_maps_verify, 0, 0);
 
        return 0;
 }
@@ -975,7 +975,7 @@ finish:
  * @param[in] maps     Head of the map list.
  * @return UNLANG_ACTION_CALCULATE_RESULT
  */
-static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, void *mod_inst, UNUSED void *proc_inst, request_t *request,
+static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, void const *mod_inst, UNUSED void *proc_inst, request_t *request,
                                    fr_value_box_list_t *key, map_list_t const *maps)
 {
        rlm_csv_t               *inst = talloc_get_type_abort(mod_inst, rlm_csv_t);