New function to return rbnode_t * when a node is inserted.
authoraland <aland>
Wed, 18 Apr 2007 11:08:38 +0000 (11:08 +0000)
committeraland <aland>
Wed, 18 Apr 2007 11:08:38 +0000 (11:08 +0000)
When deleting nodes, do NOT move Node->Data from one to another,
as the caller may be pointing to Node.

Cache rbnode_t* in lrad_event_t, which speeds up deletions
enormously.

src/include/libradius.h
src/lib/event.c
src/lib/rbtree.c

index db9db91..910b8d7 100644 (file)
@@ -132,6 +132,18 @@ typedef struct dict_vendor {
        char                    name[1];
 } DICT_VENDOR;
 
+typedef union value_pair_data {
+       char                    strvalue[MAX_STRING_LEN];
+       uint8_t                 octets[MAX_STRING_LEN];
+       struct in_addr          ipaddr;
+       struct in6_addr         ipv6addr;
+       uint32_t                date;
+       uint32_t                integer;
+       uint8_t                 filter[32];
+       uint8_t                 ifid[8]; /* struct? */
+       uint8_t                 ipv6prefix[18]; /* struct? */
+} VALUE_PAIR_DATA;
+
 typedef struct value_pair {
        char                    name[40];
        int                     attribute;
@@ -142,17 +154,7 @@ typedef struct value_pair {
        uint32_t                lvalue; /* DELETE ME ASAP */
         ATTR_FLAGS              flags;
        struct value_pair       *next;
-       union {
-               char                    strvalue[MAX_STRING_LEN];
-               uint8_t                 octets[MAX_STRING_LEN];
-               struct in_addr          ipaddr;
-               struct in6_addr         ipv6addr;
-               uint32_t                date;
-               uint32_t                integer;
-               uint8_t                 filter[32];
-               uint8_t                 ifid[8]; /* struct? */
-               uint8_t                 ipv6prefix[18]; /* struct? */
-       } data;
+       VALUE_PAIR_DATA         data;
 } VALUE_PAIR;
 #define vp_strvalue   data.strvalue
 #define vp_octets     data.octets
@@ -391,6 +393,7 @@ rbtree_t       *rbtree_create(int (*Compare)(const void *, const void *),
                               int replace_flag);
 void           rbtree_free(rbtree_t *tree);
 int            rbtree_insert(rbtree_t *tree, void *Data);
+rbnode_t       *rbtree_insertnode(rbtree_t *tree, void *Data);
 void           rbtree_delete(rbtree_t *tree, rbnode_t *Z);
 int            rbtree_deletebydata(rbtree_t *tree, const void *data);
 rbnode_t       *rbtree_find(rbtree_t *tree, const void *Data);
index a4d5da2..22e13df 100644 (file)
@@ -57,6 +57,7 @@ struct lrad_event_t {
        void                    *ctx;
        struct timeval          when;
        lrad_event_t            **ev_p;
+       rbnode_t                *node;
 };
 
 
@@ -119,7 +120,7 @@ int lrad_event_delete(lrad_event_list_t *el, lrad_event_t **ev_p)
        ev = *ev_p;
        *(ev->ev_p) = NULL;
 
-       rbtree_deletebydata(el->times, ev);
+       rbtree_delete(el->times, ev->node);
 
        return 1;
 }
@@ -151,7 +152,8 @@ int lrad_event_insert(lrad_event_list_t *el,
         *      increase the usec counter by 1, in order to avoid the
         *      duplicate.  If we can't insert it after 10 tries, die.
         */
-       if (!rbtree_insert(el->times, ev)) {
+       ev->node = rbtree_insertnode(el->times, ev);
+       if (!ev->node) {
                if (rbtree_finddata(el->times, ev)) {
                        int i;
 
@@ -166,7 +168,8 @@ int lrad_event_insert(lrad_event_list_t *el,
                                        continue;
                                }
 
-                               if (!rbtree_insert(el->times, ev)) {
+                               ev->node = rbtree_insertnode(el->times, ev);
+                               if (!ev->node) { /* error */
                                        break;
                                }
 
index f656b9a..37708eb 100644 (file)
@@ -228,7 +228,7 @@ static void InsertFixup(rbtree_t *tree, rbnode_t *X)
 /*
  *     Insert an element into the tree.
  */
-int rbtree_insert(rbtree_t *tree, void *Data)
+rbnode_t *rbtree_insertnode(rbtree_t *tree, void *Data)
 {
        rbnode_t *Current, *Parent, *X;
 
@@ -251,7 +251,7 @@ int rbtree_insert(rbtree_t *tree, void *Data)
                         *      Don't replace the entry.
                         */
                        if (tree->replace_flag == 0) {
-                               return 0;
+                               return NULL;
                        }
 
                        /*
@@ -259,7 +259,7 @@ int rbtree_insert(rbtree_t *tree, void *Data)
                         */
                        if (tree->freeNode) tree->freeNode(Current->Data);
                        Current->Data = Data;
-                       return 1;
+                       return Current;
                }
 
                Parent = Current;
@@ -291,7 +291,13 @@ int rbtree_insert(rbtree_t *tree, void *Data)
 
        tree->num_elements++;
 
-       return 1;
+       return X;
+}
+
+int rbtree_insert(rbtree_t *tree, void *Data)
+{
+       if (rbtree_insertnode(tree, Data)) return 1;
+       return 0;
 }
 
 static void DeleteFixup(rbtree_t *tree, rbnode_t *X, rbnode_t *Parent)
@@ -366,6 +372,7 @@ static void DeleteFixup(rbtree_t *tree, rbnode_t *X, rbnode_t *Parent)
  */
 void rbtree_delete(rbtree_t *tree, rbnode_t *Z)
 {
+       int fixup = 0;
        rbnode_t *X, *Y;
        rbnode_t *Parent;
 
@@ -403,21 +410,41 @@ void rbtree_delete(rbtree_t *tree, rbnode_t *Z)
                tree->Root = X;
 
        if (Y != Z) {
-               /*
-                *      Move the child's data to here, and then
-                *      re-balance the tree.
-                */
                if (tree->freeNode) tree->freeNode(Z->Data);
                Z->Data = Y->Data;
                Y->Data = NULL;
-       } else if (tree->freeNode) {
-               tree->freeNode(Z->Data);
-       }
 
-       if (Y->Color == Black && X != NIL)
-               DeleteFixup(tree, X, Parent);
+               if (Y->Color == Black && X != NIL)
+                       DeleteFixup(tree, X, Parent);
+               
+               /*
+                *      The user structure in Y->Data MAY include a
+                *      pointer to Y.  In that case, we CANNOT delete
+                *      Y.  Instead, we copy Z (which is now in the
+                *      tree) to Y, and fix up the parent/child
+                *      pointers.
+                */
+               memcpy(Y, Z, sizeof(*Y));
+
+               if (!Y->Parent) {
+                       tree->Root = Y;
+               } else {
+                       if (Y->Parent->Left == Z) Y->Parent->Left = Y;
+                       if (Y->Parent->Right == Z) Y->Parent->Right = Y;
+               }
+               if (Y->Left->Parent == Z) Y->Left->Parent = Y;
+               if (Y->Right->Parent == Z) Y->Right->Parent = Y;
+               
+               free(Z);
 
-       free(Y);
+       } else {
+               if (tree->freeNode) tree->freeNode(Y->Data);
+               
+               if (Y->Color == Black && X != NIL)
+                       DeleteFixup(tree, X, Parent);
+               
+               free(Y);
+       }
 
        tree->num_elements--;
 }