From 7b4be598c46da33b0fb4addf949ebb8aa7e8ab8a Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 20 Dec 2013 14:19:23 +0100 Subject: [PATCH] radix: don't modify node prefix on lookup The radix tree stores user data. However, it had no function to return this data to the consumers of the API. Instead, on lookup, it would set a field "user_data_result" in the nodes prefix structure which could then be read by the caller. Apart for this not being a very nice design as it exposes API internals to the caller, it is not thread safe. By updating the global data structure without any form (or suggestion) of locking, threads could overwrite the same field unexpectedly. This patch modifies the lookup logic to get rid of this stored user_data_result. Instead, all the lookup functions how take an addition argument: void **user_data_result. Through this pointer the user data is returned. It's allowed to be NULL, in this case the user data is ignored. This is a significant API change, that affects a lot of tests and callers. These will be updated in follow up patches. Bug #1073. --- src/util-radix-tree.c | 55 +++++++++++++++++++++++-------------------- src/util-radix-tree.h | 32 ++++++------------------- 2 files changed, 37 insertions(+), 50 deletions(-) diff --git a/src/util-radix-tree.c b/src/util-radix-tree.c index a5c471aba3..d62de37df9 100644 --- a/src/util-radix-tree.c +++ b/src/util-radix-tree.c @@ -303,7 +303,8 @@ static int SCRadixPrefixNetmaskCount(SCRadixPrefix *prefix) */ static int SCRadixPrefixContainNetmaskAndSetUserData(SCRadixPrefix *prefix, uint16_t netmask, - int exact_match) + int exact_match, + void **user_data_result) { SCRadixUserData *user_data = NULL; @@ -317,7 +318,8 @@ static int SCRadixPrefixContainNetmaskAndSetUserData(SCRadixPrefix *prefix, * netblock, i.e. an ip with a netmask of 32(ipv4) or 128(ipv6) */ if (exact_match) { if (user_data->netmask == netmask) { - prefix->user_data_result = user_data->user; + if (user_data_result) + *user_data_result = user_data->user; return 1; } else { goto no_match; @@ -327,13 +329,16 @@ static int SCRadixPrefixContainNetmaskAndSetUserData(SCRadixPrefix *prefix, /* Check for the user_data entry for this netmask_value */ while (user_data != NULL) { if (user_data->netmask == netmask) { - prefix->user_data_result = user_data->user; + if (user_data_result) + *user_data_result = user_data->user; return 1; } user_data = user_data->next; } no_match: + if (user_data_result != NULL) + *user_data_result = NULL; return 0; } @@ -1321,7 +1326,7 @@ void SCRadixRemoveKeyIPV6(uint8_t *key_stream, SCRadixTree *tree) * \param node Pointer to the node from where we have to climb the tree */ static inline SCRadixNode *SCRadixFindKeyIPNetblock(uint8_t *key_stream, uint8_t key_bitlen, - SCRadixNode *node) + SCRadixNode *node, void **user_data_result) { SCRadixNode *netmask_node = NULL; int mask = 0; @@ -1371,13 +1376,13 @@ static inline SCRadixNode *SCRadixFindKeyIPNetblock(uint8_t *key_stream, uint8_t if (key_bitlen % 8 == 0 || (node->prefix->stream[bytes] & mask) == (key_stream[bytes] & mask)) { - if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, netmask_node->netmasks[j], 0)) + if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, netmask_node->netmasks[j], 0, user_data_result)) return node; } } } - return SCRadixFindKeyIPNetblock(key_stream, key_bitlen, netmask_node->parent); + return SCRadixFindKeyIPNetblock(key_stream, key_bitlen, netmask_node->parent, user_data_result); } /** @@ -1390,7 +1395,7 @@ static inline SCRadixNode *SCRadixFindKeyIPNetblock(uint8_t *key_stream, uint8_t * \param exact_match The key to be searched is an ip address */ static SCRadixNode *SCRadixFindKey(uint8_t *key_stream, uint16_t key_bitlen, - SCRadixTree *tree, int exact_match) + SCRadixTree *tree, int exact_match, void **user_data_result) { if (tree == NULL || tree->head == NULL) return NULL; @@ -1429,7 +1434,7 @@ static SCRadixNode *SCRadixFindKey(uint8_t *key_stream, uint16_t key_bitlen, if (key_bitlen % 8 == 0 || (node->prefix->stream[bytes] & mask) == (tmp_stream[bytes] & mask)) { - if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, key_bitlen, 1)) { + if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, key_bitlen, 1, user_data_result)) { return node; } } @@ -1440,7 +1445,7 @@ static SCRadixNode *SCRadixFindKey(uint8_t *key_stream, uint16_t key_bitlen, return NULL; } - SCRadixNode *ret = SCRadixFindKeyIPNetblock(tmp_stream, key_bitlen, node); + SCRadixNode *ret = SCRadixFindKeyIPNetblock(tmp_stream, key_bitlen, node, user_data_result); return ret; } @@ -1452,9 +1457,9 @@ static SCRadixNode *SCRadixFindKey(uint8_t *key_stream, uint16_t key_bitlen, * \param tree Pointer to the Radix tree instance */ SCRadixNode *SCRadixFindKeyGeneric(uint8_t *key_stream, uint16_t key_bitlen, - SCRadixTree *tree) + SCRadixTree *tree, void **user_data_result) { - return SCRadixFindKey(key_stream, key_bitlen, tree, 1); + return SCRadixFindKey(key_stream, key_bitlen, tree, 1, user_data_result); } /** @@ -1464,9 +1469,9 @@ SCRadixNode *SCRadixFindKeyGeneric(uint8_t *key_stream, uint16_t key_bitlen, * an IPV4 address * \param tree Pointer to the Radix tree instance */ -SCRadixNode *SCRadixFindKeyIPV4ExactMatch(uint8_t *key_stream, SCRadixTree *tree) +SCRadixNode *SCRadixFindKeyIPV4ExactMatch(uint8_t *key_stream, SCRadixTree *tree, void **user_data_result) { - return SCRadixFindKey(key_stream, 32, tree, 1); + return SCRadixFindKey(key_stream, 32, tree, 1, user_data_result); } /** @@ -1476,9 +1481,9 @@ SCRadixNode *SCRadixFindKeyIPV4ExactMatch(uint8_t *key_stream, SCRadixTree *tree * an IPV4 address * \param tree Pointer to the Radix tree instance */ -SCRadixNode *SCRadixFindKeyIPV4BestMatch(uint8_t *key_stream, SCRadixTree *tree) +SCRadixNode *SCRadixFindKeyIPV4BestMatch(uint8_t *key_stream, SCRadixTree *tree, void **user_data_result) { - return SCRadixFindKey(key_stream, 32, tree, 0); + return SCRadixFindKey(key_stream, 32, tree, 0, user_data_result); } /** @@ -1489,14 +1494,14 @@ SCRadixNode *SCRadixFindKeyIPV4BestMatch(uint8_t *key_stream, SCRadixTree *tree) * \param tree Pointer to the Radix tree instance */ SCRadixNode *SCRadixFindKeyIPV4Netblock(uint8_t *key_stream, SCRadixTree *tree, - uint8_t netmask) + uint8_t netmask, void **user_data_result) { SCRadixNode *node = NULL; - node = SCRadixFindKey(key_stream, 32, tree, 0); + node = SCRadixFindKey(key_stream, 32, tree, 0, user_data_result); if (node == NULL) return node; - if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, netmask, 1)) + if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, netmask, 1, user_data_result)) return node; else return NULL; @@ -1510,14 +1515,14 @@ SCRadixNode *SCRadixFindKeyIPV4Netblock(uint8_t *key_stream, SCRadixTree *tree, * \param tree Pointer to the Radix tree instance */ SCRadixNode *SCRadixFindKeyIPV6Netblock(uint8_t *key_stream, SCRadixTree *tree, - uint8_t netmask) + uint8_t netmask, void **user_data_result) { SCRadixNode *node = NULL; - node = SCRadixFindKey(key_stream, 128, tree, 0); + node = SCRadixFindKey(key_stream, 128, tree, 0, user_data_result); if (node == NULL) return node; - if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, (uint16_t)netmask, 1)) + if (SCRadixPrefixContainNetmaskAndSetUserData(node->prefix, (uint16_t)netmask, 1, user_data_result)) return node; else return NULL; @@ -1530,9 +1535,9 @@ SCRadixNode *SCRadixFindKeyIPV6Netblock(uint8_t *key_stream, SCRadixTree *tree, * an IPV6 address * \param tree Pointer to the Radix tree instance */ -SCRadixNode *SCRadixFindKeyIPV6ExactMatch(uint8_t *key_stream, SCRadixTree *tree) +SCRadixNode *SCRadixFindKeyIPV6ExactMatch(uint8_t *key_stream, SCRadixTree *tree, void **user_data_result) { - return SCRadixFindKey(key_stream, 128, tree, 1); + return SCRadixFindKey(key_stream, 128, tree, 1, user_data_result); } /** @@ -1542,9 +1547,9 @@ SCRadixNode *SCRadixFindKeyIPV6ExactMatch(uint8_t *key_stream, SCRadixTree *tree * an IPV6 address * \param tree Pointer to the Radix tree instance */ -SCRadixNode *SCRadixFindKeyIPV6BestMatch(uint8_t *key_stream, SCRadixTree *tree) +SCRadixNode *SCRadixFindKeyIPV6BestMatch(uint8_t *key_stream, SCRadixTree *tree, void **user_data_result) { - return SCRadixFindKey(key_stream, 128, tree, 0); + return SCRadixFindKey(key_stream, 128, tree, 0, user_data_result); } /** diff --git a/src/util-radix-tree.h b/src/util-radix-tree.h index 1461a7b868..e9e63457c6 100644 --- a/src/util-radix-tree.h +++ b/src/util-radix-tree.h @@ -26,19 +26,6 @@ #define SC_RADIX_BITTEST(x, y) ((x) & (y)) -/** - * \brief Macro to fetch the user data from a node. It checks if node is a - * valid pointer and if node->prefix is as well. - * - * \param node Variable name/expression containing the node - * \param type User data type in which the node points to - * - * \returns User data within the node - */ -#define SC_RADIX_NODE_USERDATA(node, type) \ - ((type *)(((node) != NULL) ? (((node)->prefix != NULL) ? \ - (node)->prefix->user_data_result : NULL) : NULL)) - /** * \brief Structure that hold the user data and the netmask associated with it. */ @@ -66,11 +53,6 @@ typedef struct SCRadixPrefix_ { * with any of the the 32 or 128 netblocks. Also for non-ips, we store the * netmask as 255 in SCRadixUserData->netmask */ SCRadixUserData *user_data; - - /* Used to hold the user data from radix tree search. More of a convenience - * that lets anyone using the API, directly get a reference to the user - * data which is associated with the search results */ - void *user_data_result; } SCRadixPrefix; /** @@ -135,15 +117,15 @@ void SCRadixRemoveKeyIPV4(uint8_t *, SCRadixTree *); void SCRadixRemoveKeyIPV6Netblock(uint8_t *, SCRadixTree *, uint8_t); void SCRadixRemoveKeyIPV6(uint8_t *, SCRadixTree *); -SCRadixNode *SCRadixFindKeyGeneric(uint8_t *, uint16_t, SCRadixTree *); +SCRadixNode *SCRadixFindKeyGeneric(uint8_t *, uint16_t, SCRadixTree *, void **); -SCRadixNode *SCRadixFindKeyIPV4ExactMatch(uint8_t *, SCRadixTree *); -SCRadixNode *SCRadixFindKeyIPV4Netblock(uint8_t *, SCRadixTree *, uint8_t); -SCRadixNode *SCRadixFindKeyIPV4BestMatch(uint8_t *, SCRadixTree *); +SCRadixNode *SCRadixFindKeyIPV4ExactMatch(uint8_t *, SCRadixTree *, void **); +SCRadixNode *SCRadixFindKeyIPV4Netblock(uint8_t *, SCRadixTree *, uint8_t, void **); +SCRadixNode *SCRadixFindKeyIPV4BestMatch(uint8_t *, SCRadixTree *, void **); -SCRadixNode *SCRadixFindKeyIPV6ExactMatch(uint8_t *, SCRadixTree *); -SCRadixNode *SCRadixFindKeyIPV6Netblock(uint8_t *, SCRadixTree *, uint8_t); -SCRadixNode *SCRadixFindKeyIPV6BestMatch(uint8_t *, SCRadixTree *); +SCRadixNode *SCRadixFindKeyIPV6ExactMatch(uint8_t *, SCRadixTree *, void **); +SCRadixNode *SCRadixFindKeyIPV6Netblock(uint8_t *, SCRadixTree *, uint8_t, void **); +SCRadixNode *SCRadixFindKeyIPV6BestMatch(uint8_t *, SCRadixTree *, void **); void SCRadixPrintTree(SCRadixTree *); void SCRadixPrintNodeInfo(SCRadixNode *, int, void (*PrintData)(void*));