Add "old" argument to fr_*_replace to return the old data

This is to avoid multiple lookups in pair.c

diff --git a/src/lib/server/map_proc.c b/src/lib/server/map_proc.c
index b285a37e5cdbca0fd0451b00f280b9198f8b1e32..ff40ab68a2e360e3b685b55a0e9d28407a259a92 100644 (file)
--- a/src/lib/server/map_proc.c
+++ b/src/lib/server/map_proc.c
@@ -134,7 +134,7 @@ int map_proc_register(void *mod_inst, char const *name,
                strlcpy(proc->name, name, sizeof(proc->name));
                proc->length = strlen(proc->name);
 
-               if (fr_rb_replace(map_proc_root, proc) < 0) {
+               if (fr_rb_replace(NULL, map_proc_root, proc) < 0) {
                        talloc_free(proc);
                        return -1;
                }

diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c
index dfefd560083a0c551fd6ce31bf19cfaaf2a73ee8..1db8dcff75e3083098299768ba6983c14217df4f 100644 (file)
--- a/src/lib/server/virtual_servers.c
+++ b/src/lib/server/virtual_servers.c
@@ -1236,7 +1236,7 @@ int virtual_namespace_register(char const *namespace, fr_dict_t const *dict,
                }
        }
 
-       if (fr_rb_replace(vns_tree, vns) < 0) {
+       if (fr_rb_replace(NULL, vns_tree, vns) < 0) {
                ERROR("Failed inserting namespace into tree");
                return -1;
        }

diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c
index 749921b75757b64da706447850933b352254f159..fba3ca6ed998b5c014621ef38433c2d23aa45a26 100644 (file)
--- a/src/lib/unlang/xlat_builtin.c
+++ b/src/lib/unlang/xlat_builtin.c
@@ -221,7 +221,7 @@ xlat_t *xlat_register_legacy(void *mod_inst, char const *name,
 
        DEBUG3("%s: %s", __FUNCTION__, c->name);
 
-       if (is_new && (fr_rb_replace(xlat_root, c) < 0)) {
+       if (is_new && (fr_rb_replace(NULL, xlat_root, c) < 0)) {
                ERROR("Failed inserting xlat registration for %s",
                      c->name);
                talloc_free(c);
@@ -292,7 +292,7 @@ xlat_t *xlat_register(TALLOC_CTX *ctx, char const *name, xlat_func_t func, bool
        talloc_set_destructor(c, _xlat_func_talloc_free);
        DEBUG3("%s: %s", __FUNCTION__, c->name);
 
-       if (fr_rb_replace(xlat_root, c) < 0) {
+       if (fr_rb_replace(NULL, xlat_root, c) < 0) {
                ERROR("%s: Failed inserting xlat registration for %s", __FUNCTION__, c->name);
                talloc_free(c);
                return NULL;

diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index 4ff9629d70d0561f50d0b63318788e4a62928f3c..3e0562842c40bda213c03c4d26078fa9ac0aea0a 100644 (file)
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -827,7 +827,7 @@ int dict_vendor_add(fr_dict_t *dict, char const *name, unsigned int num)
         *      files, but when we're printing them, (and looking up
         *      by value) we want to use the NEW name.
         */
-       if (fr_hash_table_replace(dict->vendors_by_num, vendor) < 0) {
+       if (fr_hash_table_replace(NULL, dict->vendors_by_num, vendor) < 0) {
                fr_strerror_printf("%s: Failed inserting vendor %s", __FUNCTION__, name);
                return -1;
        }
@@ -1049,7 +1049,7 @@ int dict_attr_add_to_namespace(fr_dict_attr_t const *parent, fr_dict_attr_t *da)
                 *      dictionary but entry in the name hash table is
                 *      updated to point to the new definition.
                 */
-               if (fr_hash_table_replace(namespace, da) < 0) {
+               if (fr_hash_table_replace(NULL, namespace, da) < 0) {
                        fr_strerror_const("Internal error storing attribute");
                        goto error;
                }
@@ -1316,7 +1316,7 @@ int dict_attr_enum_add_name(fr_dict_attr_t *da, char const *name,
         *      take care of that here.
         */
        if (takes_precedence) {
-               if (fr_hash_table_replace(ext->name_by_value, enumv) < 0) {
+               if (fr_hash_table_replace(NULL, ext->name_by_value, enumv) < 0) {
                        fr_strerror_printf("%s: Failed inserting value %s", __FUNCTION__, name);
                        return -1;
                }

diff --git a/src/lib/util/hash.c b/src/lib/util/hash.c
index a65b5099121e1d2ab480e9e0365e421c49a5cdaf..454e1fc1078a32e45f7be4901413e154d689fbb4 100644 (file)
--- a/src/lib/util/hash.c
+++ b/src/lib/util/hash.c
@@ -503,22 +503,33 @@ bool fr_hash_table_insert(fr_hash_table_t *ht, void const *data)
 
 /** Replace old data with new data, OR insert if there is no old
  *
+ * @param[out] old     data that was replaced.  If this argument
+ *                     is not NULL, then the old data will not
+ *                     be freed, even if a free function is
+ *                     configured.
  * @param[in] ht       to insert data into.
  * @param[in] data     to replace.  Will be passed to the
  *                     hashing function.
  * @return
- *      - 1 if data was inserted.
- *     - 0 if data was replaced.
+ *      - 1 if data was replaced.
+ *     - 0 if data was inserted.
  *      - -1 if we failed to replace data
  */
-int fr_hash_table_replace(fr_hash_table_t *ht, void const *data)
+int fr_hash_table_replace(void **old, fr_hash_table_t *ht, void const *data)
 {
        fr_hash_entry_t *node;
 
        node = hash_table_find(ht, ht->hash(data), data);
-       if (!node) return fr_hash_table_insert(ht, data) ? 1 : -1;
+       if (!node) {
+               if (old) *old = NULL;
+               return fr_hash_table_insert(ht, data) ? 1 : -1;
+       }
 
-       if (ht->free) ht->free(node->data);
+       if (old) {
+               *old = node->data;
+       } else if (ht->free) {
+               ht->free(node->data);
+       }
 
        node->data = UNCONST(void *, data);
 

diff --git a/src/lib/util/hash.h b/src/lib/util/hash.h
index d00fe5cefa021c1f955dabdd0998e73f75c707e5..0b3d75c887a305fc6e16743a3254f1527534376c 100644 (file)
--- a/src/lib/util/hash.h
+++ b/src/lib/util/hash.h
@@ -66,7 +66,7 @@ void          *fr_hash_table_find_by_key(fr_hash_table_t *ht, uint32_t key, void const *
 
 bool           fr_hash_table_insert(fr_hash_table_t *ht, void const *data) CC_HINT(nonnull);
 
-int            fr_hash_table_replace(fr_hash_table_t *ht, void const *data) CC_HINT(nonnull);
+int            fr_hash_table_replace(void **old, fr_hash_table_t *ht, void const *data) CC_HINT(nonnull(2,3));
 
 void           *fr_hash_table_remove(fr_hash_table_t *ht, void const *data) CC_HINT(nonnull);
 

diff --git a/src/lib/util/htrie.h b/src/lib/util/htrie.h
index df35998e461243141190d33bcc6464d102b7c941..da723844114044bacf7dd34a11f8bf67ad120f7e 100644 (file)
--- a/src/lib/util/htrie.h
+++ b/src/lib/util/htrie.h
@@ -38,7 +38,7 @@ typedef void *(*fr_htrie_find_t)(fr_htrie_t *ht, void const *data);
 
 typedef bool (*fr_htrie_insert_t)(fr_htrie_t *ht, void const *data);
 
-typedef int (*fr_htrie_replace_t)(fr_htrie_t *ht, void const *data);
+typedef int (*fr_htrie_replace_t)(void **old, fr_htrie_t *ht, void const *data);
 
 typedef void *(*fr_htrie_remove_t)(fr_htrie_t *ht, void const *data);
 
@@ -99,9 +99,9 @@ static inline CC_HINT(nonnull) bool fr_htrie_insert(fr_htrie_t *ht, void const *
 /** Replace data in a htrie, freeing previous data if free_data cb was passed to fr_htrie_alloc
  *
  */
-static inline CC_HINT(nonnull) int fr_htrie_replace(fr_htrie_t *ht, void const *data)
+static inline CC_HINT(nonnull(2,3)) int fr_htrie_replace(void **old, fr_htrie_t *ht, void const *data)
 {
-       return ht->funcs.replace(ht->store, data);
+       return ht->funcs.replace(old, ht->store, data);
 }
 
 /** Remove data from a htrie without freeing it

diff --git a/src/lib/util/rb.c b/src/lib/util/rb.c
index 35d23235d47f3791469178947a008d68a51e8185..ef7c8081bfe191c0f8e875869300e17f8e917814 100644 (file)
--- a/src/lib/util/rb.c
+++ b/src/lib/util/rb.c
@@ -33,7 +33,7 @@ static fr_rb_node_t sentinel = { NIL, NIL, NULL, NULL, BLACK, false };
 #  define RB_MAGIC (0x5ad09c42)
 #endif
 
-static fr_rb_node_t    *insert_node(fr_rb_tree_t *tree, void *data) CC_HINT(nonnull);
+static int insert_node(fr_rb_node_t **existing, fr_rb_tree_t *tree, void *data) CC_HINT(nonnull);
 
 static inline CC_HINT(always_inline) void node_data_free(fr_rb_tree_t const *tree, fr_rb_node_t *node)
 {
@@ -333,8 +333,16 @@ static inline CC_HINT(always_inline) void insert_fixup(fr_rb_tree_t *tree, fr_rb
 
 /** Insert an element into the tree
  *
+ * @param[out] existing                if a node exists, and existing is not NULL
+ *                             this will be populated with the node.
+ * @param[in] tree             to search in.
+ * @param[in] data             to search for.
+ * @return
+ *     - 1 on existing (with existing populated).
+ *      - 0 on success.
+ *     - -1 on failure.
  */
-static fr_rb_node_t *insert_node(fr_rb_tree_t *tree, void *data)
+static int insert_node(fr_rb_node_t **existing, fr_rb_tree_t *tree, void *data)
 {
        fr_rb_node_t *current, *parent, *x;
 
@@ -354,7 +362,10 @@ static fr_rb_node_t *insert_node(fr_rb_tree_t *tree, void *data)
                 *      See if two entries are identical.
                 */
                result = tree->data_cmp(data, current->data);
-               if (result == 0) return NULL;
+               if (result == 0) {
+                       if (existing) *existing = current;
+                       return 1
+               }
 
                parent = current;
                current = (result < 0) ? current->left : current->right;
@@ -362,7 +373,7 @@ static fr_rb_node_t *insert_node(fr_rb_tree_t *tree, void *data)
 
        /* setup new node */
        x = tree->node_alloc(tree, data);
-       if (unlikely(!x)) return NULL;
+       if (unlikely(!x)) return -1;
 
        *x = (fr_rb_node_t){
                .data = data,
@@ -387,7 +398,7 @@ static fr_rb_node_t *insert_node(fr_rb_tree_t *tree, void *data)
 
        tree->num_elements++;
 
-       return x;
+       return 0;
 }
 
 /** Maintain RED-BLACK tree balance after deleting node x
@@ -573,6 +584,35 @@ void *fr_rb_find(fr_rb_tree_t *tree, void const *data)
        return x->data;
 }
 
+/** Attempt to find current data in the tree, if it does not exist insert it
+ *
+ * @param[out] found   Pre-existing data we found.
+ * @param[in] tree     to search/insert into.
+ * @param[in] data     to find.
+ * @return
+ *     - 1 if existing data was found, found will be populated.
+ *     - 0 if no existing data was found.
+ *     - -1 on insert error.
+ */
+int fr_rb_find_or_insert(void **found, fr_rb_tree_t *tree, void const *data)
+{
+       fr_rb_node_t *existing;
+
+       switch (insert_node(existing, tree, UNCONST(void *, data))) {
+       case 1:
+               if (found) *found = existing->data;
+               return 1;
+
+       case 0:
+               if (found) *found = NULL;
+               return 0;
+
+       default:
+               if (found) *found = NULL;
+               return -1;
+       }
+}
+
 /** Insert data into a tree
  *
  * @param[in] tree     to insert data into.
@@ -583,44 +623,61 @@ void *fr_rb_find(fr_rb_tree_t *tree, void const *data)
  */
 bool fr_rb_insert(fr_rb_tree_t *tree, void const *data)
 {
-       if (insert_node(tree, UNCONST(void *, data))) return true;
+       if (insert_node(NULL, tree, UNCONST(void *, data)) == 0) return true;
 
        return false;
 }
 
 /** Replace old data with new data, OR insert if there is no old
  *
+ * @param[out] old     data that was replaced.  If this argument
+ *                     is not NULL, then the old data will not
+ *                     be freed, even if a free function is
+ *                     configured.
  * @param[in] tree     to insert data into.
  * @param[in] data     to replace.
  * @return
- *      - 1 if data was inserted.
- *     - 0 if data was replaced.
+ *      - 1 if data was replaced.
+ *     - 0 if data was inserted.
  *      - -1 if we failed to replace data
  */
-int fr_rb_replace(fr_rb_tree_t *tree, void const *data)
+int fr_rb_replace(void **old, fr_rb_tree_t *tree, void const *data)
 {
        fr_rb_node_t    *node;
-       void            *old_data;
 
-       node = find_node(tree, UNCONST(void *, data));
-       if (!node) return insert_node(tree, UNCONST(void *, data)) ? 1 : -1;
-       old_data = node->data;
+       switch (insert_node(&node, tree, UNCONST(void *, data))) {
+       case 1: /* Something exists */
+       {
+               void    *old_data = node->data;
 
-       /*
-        *      If the fr_node_t is inline with the
-        *      data structure, we need to delete
-        *      the old node out of the tree, and
-        *      perform a normal insert operation.
-        */
-       if (tree->node_alloc == _node_inline_alloc) {
-               delete_internal(tree, node, false);
-               insert_node(tree, UNCONST(void *, data));
-       } else {
-               node->data = UNCONST(void *, data);
+               /*
+                *      If the fr_node_t is inline with the
+                *      data structure, we need to delete
+                *      the old node out of the tree, and
+                *      perform a normal insert operation.
+                */
+               if (tree->node_alloc == _node_inline_alloc) {
+                       delete_internal(tree, node, false);
+                       insert_node(NULL, tree, UNCONST(void *, data));
+               } else {
+                       node->data = UNCONST(void *, data);
+               }
+
+               if (old) {
+                       *old = old_data;
+               } else if (tree->data_free) {
+                       tree->data_free(old_data);
+               }
+               return 1;
        }
-       if (tree->data_free) tree->data_free(old_data);
+       case 0: /* New node was inserted - There was no pre-existing node */
+               if (old) *old = NULL;
+               return 0;
 
-       return 0;
+       default:
+               if (old) *old = NULL;
+               return -1;
+       }
 }
 
 /** Remove an entry from the tree, without freeing the data

diff --git a/src/lib/util/rb.h b/src/lib/util/rb.h
index 0509f29a284841ae33b1e254cf29fa5b97af9303..84df3ae3168c7451364c8331640f05893bd6a457 100644 (file)
--- a/src/lib/util/rb.h
+++ b/src/lib/util/rb.h
@@ -287,9 +287,11 @@ fr_rb_tree_t       *_fr_rb_alloc(TALLOC_CTX *ctx, ssize_t offset, char const *type,
 /** @hidecallergraph */
 void           *fr_rb_find(fr_rb_tree_t *tree, void const *data) CC_HINT(nonnull);
 
+int            fr_rb_find_or_insert(void **found, fr_rb_tree_t *tree, void const *data) CC_HINT(nonnull);
+
 bool           fr_rb_insert(fr_rb_tree_t *tree, void const *data) CC_HINT(nonnull);
 
-int            fr_rb_replace(fr_rb_tree_t *tree, void const *data) CC_HINT(nonnull);
+int            fr_rb_replace(void **old, fr_rb_tree_t *tree, void const *data) CC_HINT(nonnull(2,3));
 
 void           *fr_rb_remove(fr_rb_tree_t *tree, void const *data) CC_HINT(nonnull);
 

diff --git a/src/lib/util/trie.c b/src/lib/util/trie.c
index dc15e88626395352959c48e5e0c688c139a62b9a..ba11b045963c0c78f7227cb101ff3907cdf0490e 100644 (file)
--- a/src/lib/util/trie.c
+++ b/src/lib/util/trie.c
@@ -2685,20 +2685,24 @@ bool fr_trie_insert(fr_trie_t *ft, void const *data)
 
 /** Replace old data with new data, OR insert if there is no old
  *
+ * @param[out] old     data that was replaced.  If this argument
+ *                     is not NULL, then the old data will not
+ *                     be freed, even if a free function is
+ *                     configured.
  * @param[in] ft       to insert data into.
  * @param[in] data     to replace.
  * @return
- *      - 1 if data was inserted.
- *     - 0 if data was replaced.
+ *     - 1 if data was replaced.
+ *      - 0 if data was inserted.
  *      - -1 if we failed to replace data
  */
-int fr_trie_replace(fr_trie_t *ft, void const *data)
+int fr_trie_replace(void **old, fr_trie_t *ft, void const *data)
 {
-       fr_trie_user_t *user = (fr_trie_user_t *) ft;
-       fr_trie_ctx_t *uctx = talloc_get_type_abort(user->data, fr_trie_ctx_t);
-       uint8_t *key;
-       size_t keylen;
-       void *found;
+       fr_trie_user_t  *user = (fr_trie_user_t *) ft;
+       fr_trie_ctx_t   *uctx = talloc_get_type_abort(user->data, fr_trie_ctx_t);
+       uint8_t         *key;
+       size_t          keylen;
+       void            *found;
 
        key = &uctx->buffer[0];
        keylen = sizeof(uctx->buffer) * 8;
@@ -2709,13 +2713,18 @@ int fr_trie_replace(fr_trie_t *ft, void const *data)
 
        found = trie_key_match(ft, key, 0, keylen, true); /* do exact match */
        if (found) {
+               if (old) *old = found;
                if (fr_trie_remove_by_key(ft, key, keylen) != found) return -1;
+       } else {
+               if (old) *old = NULL;
        }
 
        /*
         *      Insert the new key.
         */
-       return fr_trie_insert_by_key(ft, key, keylen, data);
+       if (fr_trie_insert_by_key(ft, key, keylen, data) < 0) return -1;
+
+       return found ? 1 : 0;
 }
 
 /** Remove an entry, without freeing the data

diff --git a/src/lib/util/trie.h b/src/lib/util/trie.h
index b7c11b004511e7bccb04b5788bef01f80a3926ff..b20b44960b5540821749b3abcd46797120421835 100644 (file)
--- a/src/lib/util/trie.h
+++ b/src/lib/util/trie.h
@@ -74,7 +74,7 @@ void          *fr_trie_find(fr_trie_t *ft, void const *data) CC_HINT(nonnull);
 
 bool           fr_trie_insert(fr_trie_t *ft, void const *data) CC_HINT(nonnull);
 
-int            fr_trie_replace(fr_trie_t *ft, void const *data) CC_HINT(nonnull);
+int            fr_trie_replace(void **old, fr_trie_t *ft, void const *data) CC_HINT(nonnull(2, 3));
 
 void           *fr_trie_remove(fr_trie_t *ft, void const *data) CC_HINT(nonnull);