allow for loops in foreign dictionaries

by adding two flags, "loading", which is set in begin-proto,
and cleared when the dictionary is done.  And "loaded" which is
set by fr_protocol_afrom_file(), to indicate that it has loaded
the dictionary file. and initialized the protocol library.

The dict routines now call proto->init() and proto->free(), so that
when a protocol library is loaded, everything it needs is also
loaded and initialized.

diff --git a/src/lib/util/dict_fixup.c b/src/lib/util/dict_fixup.c
index c8042b331c98ddb082f5f36c17319d358161ad8c..c88e559812a1cd381094924c73775946d550afc4 100644 (file)
--- a/src/lib/util/dict_fixup.c
+++ b/src/lib/util/dict_fixup.c
@@ -273,20 +273,6 @@ static fr_dict_attr_t const *dict_protocol_reference(fr_dict_t **dict_def, char
                        return NULL;
                }
 
-               /*
-                *      If we call init(), make sure that we call free();
-                */
-               if (dict->proto && dict->proto->init) {
-                       if (dict->proto->init() < 0) return NULL;
-
-                       /*
-                        *      Mark the *referencing* dictionary as autofree.
-                        *
-                        *      Changing this to dict->autofree=true will break things. :(
-                        */
-                       (*dict_def)->autofree = true;
-               }
-
                /*
                 *      The reference is to the root of the foreign protocol, we're done.
                 */

diff --git a/src/lib/util/dict_priv.h b/src/lib/util/dict_priv.h
index d204f0a2fccc3c6c2486fea3cb3c9065752c66ed..ffe704f92806e0bb6b78a7aa52eeff6b73e2b96f 100644 (file)
--- a/src/lib/util/dict_priv.h
+++ b/src/lib/util/dict_priv.h
@@ -80,11 +80,11 @@ struct fr_dict {
        bool                    in_protocol_by_num;     //!< Whether the dictionary has been inserted into the
                                                        //!< protocol_by_num table.
 
-       bool                    autoloaded;             //!< manual vs autoload
-
        bool                    string_based;           //!< TACACS, etc.
 
-       bool                    autofree;               //!< from dict_fixup.
+       bool                    loading;                //!< from fr_dict_protocol_afrom_file();
+
+       bool                    loaded;                 //!< from fr_dict_protocol_afrom_file();
 
        fr_hash_table_t         *vendors_by_name;       //!< Lookup vendor by name.
        fr_hash_table_t         *vendors_by_num;        //!< Lookup vendor by PEN.

diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c
index bb99d93724eeeb24ad8cd2de8c4759e51f416477..360335c50799423ff095978bb26afb241c0009f9 100644 (file)
--- a/src/lib/util/dict_tokenize.c
+++ b/src/lib/util/dict_tokenize.c
@@ -2363,11 +2363,11 @@ static int _dict_from_file(dict_tokenize_ctx_t *ctx,
                         */
                        dict_fixup_init(NULL, &ctx->fixup);
 
-                       // check if there's a linked library for the
-                       // protocol.  The values can be unknown (we
-                       // try to load one), or non-existent, or
-                       // known.  For the last two, we don't try to
-                       // load anything.
+                       /*
+                        *      We're in the middle of loading this dictionary.  Tell
+                        *      fr_dict_protocol_afrom_file() to suppress recursive references.
+                        */
+                       found->loading = true;
 
                        ctx->dict = found;
 
@@ -2909,10 +2909,28 @@ int fr_dict_protocol_afrom_file(fr_dict_t **out, char const *proto_name, char co
         *      has already been loaded and return that.
         */
        dict = dict_by_protocol_name(proto_name);
-       if (dict && dict->autoloaded) {
-                dict_dependent_add(dict, dependent);
-               *out = dict;
-               return 0;
+       if (dict) {
+               /*
+                *      If we're in the middle of loading this dictionary, then the only way we get back here
+                *      is via a circular reference.  So we catch that, and drop the circular dependency.
+                *
+                *      When we have A->B->A, it means that we don't need to track B->A, because we track
+                *      A->B.  And if A is freed, then B is freed.
+                */
+               if (!dict->loading) dict_dependent_add(dict, dependent);
+
+               /*
+                *      But we only return a pre-existing dict if _this function_ has loaded it.
+                */
+               if (dict->loaded) {
+                       *out = dict;
+                       return 0;
+               }
+
+               /*
+                *      Set the flag to true _before_ loading the file.  That prevents recursion.
+                */
+               dict->loaded = true;
        }
 
        if (!proto_dir) {
@@ -2934,6 +2952,7 @@ int fr_dict_protocol_afrom_file(fr_dict_t **out, char const *proto_name, char co
         */
        if (dict_from_file(dict_gctx->internal, dict_dir, FR_DICTIONARY_FILE, NULL, 0) < 0) {
        error:
+               dict->loading = false;
                talloc_free(dict_dir);
                return -1;
        }
@@ -2947,13 +2966,15 @@ int fr_dict_protocol_afrom_file(fr_dict_t **out, char const *proto_name, char co
                goto error;
        }
 
-       talloc_free(dict_dir);
-
        /*
-        *      If we're autoloading a previously defined dictionary,
-        *      then mark up the dictionary as now autoloaded.
+        *      Initialize the library.
         */
-       if (!dict->autoloaded) dict->autoloaded = true;
+       if (dict->proto && dict->proto->init) {
+               if (dict->proto->init() < 0) goto error;
+       }
+       dict->loading = false;
+
+       talloc_free(dict_dir);
 
        dict_dependent_add(dict, dependent);
 

diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index 417e20479562339b682e6f882c6389e8c28620af..68762f619b0cc7504303f9d1db31300897e83463 100644 (file)
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -3347,7 +3347,7 @@ static int _dict_free(fr_dict_t *dict)
        /*
         *      If we called init(), then call free()
         */
-       if (dict->autofree) {
+       if (dict->loaded) {
                fr_assert(dict->proto);
                fr_assert(dict->proto->free);
                dict->proto->free();