From 0a96b5a833b96f75c279b1d7ce9b2324dc862730 Mon Sep 17 00:00:00 2001 From: gauthier <gauthier@mycompany.com> Date: Tue, 7 Jul 2015 13:10:54 +0000 Subject: [PATCH] bug in obj_hashtable git-svn-id: http://svn.eurecom.fr/openair4G/trunk@7694 818b1a75-f10b-46b9-bf7c-635c3b92a50f --- common/utils/collection/hashtable/hashtable.c | 66 ++++++++-- common/utils/collection/hashtable/hashtable.h | 6 +- .../collection/hashtable/obj_hashtable.c | 114 +++++++++++++++--- .../collection/hashtable/obj_hashtable.h | 18 +-- 4 files changed, 166 insertions(+), 38 deletions(-) diff --git a/common/utils/collection/hashtable/hashtable.c b/common/utils/collection/hashtable/hashtable.c index ba2686c718..0ec4e08024 100755 --- a/common/utils/collection/hashtable/hashtable.c +++ b/common/utils/collection/hashtable/hashtable.c @@ -172,14 +172,19 @@ hashtable_rc_t hashtable_apply_funct_on_elements (hash_table_t *const hashtblP, return HASH_TABLE_OK; } //------------------------------------------------------------------------------------------------------------------------------- -hashtable_rc_t hashtable_dump_content (const hash_table_t * const hashtblP, char * const buffer_pP, int * const remaining_bytes_in_buffer_pP ) +hashtable_rc_t hashtable_dump_content ( + const hash_table_t * const hashtblP, + char * const buffer_pP, + int * const remaining_bytes_in_buffer_pP) //------------------------------------------------------------------------------------------------------------------------------- { hash_node_t *node = NULL; unsigned int i = 0; unsigned int num_elements = 0; + int rc; + if (hashtblP == NULL) { - *remaining_bytes_in_buffer_pP = snprintf( + rc = snprintf( buffer_pP, *remaining_bytes_in_buffer_pP, "HASH_TABLE_BAD_PARAMETER_HASHTABLE"); @@ -189,13 +194,18 @@ hashtable_rc_t hashtable_dump_content (const hash_table_t * const hashtblP, char if (hashtblP->nodes[i] != NULL) { node=hashtblP->nodes[i]; while(node) { - *remaining_bytes_in_buffer_pP = snprintf( - buffer_pP, - *remaining_bytes_in_buffer_pP, - "Key 0x%"PRIx64" Element %p\n", - node->key, - node->data); + rc = snprintf( + buffer_pP, + *remaining_bytes_in_buffer_pP, + "Key 0x%"PRIx64" Element %p\n", + node->key, + node->data); node=node->next; + if ((0 > rc) || (*remaining_bytes_in_buffer_pP < rc)) { + fprintf(stderr, "Error while dumping hashtable content"); + } else { + *remaining_bytes_in_buffer_pP -= rc; + } } } i += 1; @@ -242,10 +252,10 @@ hashtable_rc_t hashtable_insert(hash_table_t * const hashtblP, const hash_key_t } //------------------------------------------------------------------------------------------------------------------------------- /* - * To remove an element from the hash table, we just search for it in the linked list for that hash value, - * and remove it if it is found. If it was not found, it is an error and -1 is returned. + * To free an element from the hash table, we just search for it in the linked list for that hash value, + * and free it if it is found. If it was not found, it is an error and -1 is returned. */ -hashtable_rc_t hashtable_remove(hash_table_t * const hashtblP, const hash_key_t keyP) +hashtable_rc_t hashtable_free(hash_table_t * const hashtblP, const hash_key_t keyP) { hash_node_t *node, *prevnode=NULL; hash_size_t hash = 0; @@ -271,6 +281,36 @@ hashtable_rc_t hashtable_remove(hash_table_t * const hashtblP, const hash_key_t } return HASH_TABLE_KEY_NOT_EXISTS; } +//------------------------------------------------------------------------------------------------------------------------------- +/* + * To remove an element from the hash table, we just search for it in the linked list for that hash value, + * and remove it if it is found. If it was not found, it is an error and -1 is returned. + */ +hashtable_rc_t hashtable_remove(hash_table_t * const hashtblP, const hash_key_t keyP, void** dataP) +{ + hash_node_t *node, *prevnode=NULL; + hash_size_t hash = 0; + + if (hashtblP == NULL) { + return HASH_TABLE_BAD_PARAMETER_HASHTABLE; + } + hash=hashtblP->hashfunc(keyP)%hashtblP->size; + node=hashtblP->nodes[hash]; + while(node) { + if(node->key == keyP) { + if(prevnode) prevnode->next=node->next; + else hashtblP->nodes[hash]=node->next; + *dataP = node->data; + free(node); + hashtblP->num_elements -= 1; + return HASH_TABLE_OK; + } + prevnode=node; + node=node->next; + } + return HASH_TABLE_KEY_NOT_EXISTS; +} + //------------------------------------------------------------------------------------------------------------------------------- /* * Searching for an element is easy. We just search through the linked list for the corresponding hash value. @@ -308,7 +348,7 @@ hashtable_rc_t hashtable_get(const hash_table_t * const hashtblP, const hash_key * If the number of elements are reduced, the hash table will waste memory. That is why we provide a function for resizing the table. * Resizing a hash table is not as easy as a realloc(). All hash values must be recalculated and each element must be inserted into its new position. * We create a temporary hash_table_t object (newtbl) to be used while building the new hashes. - * This allows us to reuse hashtable_insert() and hashtable_remove(), when moving the elements to the new table. + * This allows us to reuse hashtable_insert() and hashtable_free(), when moving the elements to the new table. * After that, we can just free the old table and copy the elements from newtbl to hashtbl. */ @@ -332,7 +372,7 @@ hashtable_rc_t hashtable_resize(hash_table_t * const hashtblP, const hash_size_t next = node->next; hashtable_insert(&newtbl, node->key, node->data); // Lionel GAUTHIER: BAD CODE TO BE REWRITTEN - hashtable_remove(hashtblP, node->key); + hashtable_free(hashtblP, node->key); } } diff --git a/common/utils/collection/hashtable/hashtable.h b/common/utils/collection/hashtable/hashtable.h index 2082e4f85f..cad3a6e8d8 100755 --- a/common/utils/collection/hashtable/hashtable.h +++ b/common/utils/collection/hashtable/hashtable.h @@ -49,6 +49,9 @@ typedef enum hashtable_return_code_e { HASH_TABLE_CODE_MAX } hashtable_rc_t; +#define HASH_TABLE_DEFAULT_HASH_FUNC NULL +#define HASH_TABLE_DEFAULT_FREE_FUNC NULL + typedef struct hash_node_s { hash_key_t key; @@ -72,7 +75,8 @@ hashtable_rc_t hashtable_is_key_exists (const hash_table_t * const hashtbl, con hashtable_rc_t hashtable_apply_funct_on_elements (hash_table_t * const hashtblP, void funct(hash_key_t keyP, void* dataP, void* parameterP), void* parameterP); hashtable_rc_t hashtable_dump_content (const hash_table_t * const hashtblP, char * const buffer_pP, int * const remaining_bytes_in_buffer_pP ); hashtable_rc_t hashtable_insert (hash_table_t * const hashtbl, const hash_key_t key, void *data); -hashtable_rc_t hashtable_remove (hash_table_t * const hashtbl, const hash_key_t key); +hashtable_rc_t hashtable_free (hash_table_t * const hashtbl, const hash_key_t key); +hashtable_rc_t hashtable_remove(hash_table_t * const hashtblP, const hash_key_t keyP, void** dataP); hashtable_rc_t hashtable_get (const hash_table_t * const hashtbl, const hash_key_t key, void **dataP); hashtable_rc_t hashtable_resize (hash_table_t * const hashtbl, const hash_size_t size); diff --git a/common/utils/collection/hashtable/obj_hashtable.c b/common/utils/collection/hashtable/obj_hashtable.c index b1d804335f..4d9683ade3 100755 --- a/common/utils/collection/hashtable/obj_hashtable.c +++ b/common/utils/collection/hashtable/obj_hashtable.c @@ -30,6 +30,7 @@ #include <string.h> #include <stdio.h> #include <stdlib.h> +#include <inttypes.h> #include "obj_hashtable.h" //------------------------------------------------------------------------------------------------------------------------------- /* @@ -38,11 +39,11 @@ * This is a simple/naive hash function which adds the key's ASCII char values. It will probably generate lots of collisions on large hash tables. */ -static hash_size_t def_hashfunc(const void *keyP, int key_sizeP) +static hash_size_t def_hashfunc(const void * const keyP, int key_sizeP) { hash_size_t hash=0; - while(key_sizeP) hash^=((unsigned char*)keyP)[key_sizeP --]; + while(key_sizeP) hash^=((unsigned char*)keyP)[--key_sizeP]; return hash; } @@ -54,7 +55,7 @@ static hash_size_t def_hashfunc(const void *keyP, int key_sizeP) * The user can also specify a hash function. If the hashfunc argument is NULL, a default hash function is used. * If an error occurred, NULL is returned. All other values in the returned obj_hash_table_t pointer should be released with hashtable_destroy(). */ -obj_hash_table_t *obj_hashtable_create(hash_size_t sizeP, hash_size_t (*hashfuncP)(const void*, int ), void (*freekeyfuncP)(void*), void (*freedatafuncP)(void*)) +obj_hash_table_t *obj_hashtable_create(const hash_size_t sizeP, hash_size_t (*hashfuncP)(const void*, int ), void (*freekeyfuncP)(void*), void (*freedatafuncP)(void*)) { obj_hash_table_t *hashtbl; @@ -83,7 +84,7 @@ obj_hash_table_t *obj_hashtable_create(hash_size_t sizeP, hash_size_t (*hashfunc * Cleanup * The hashtable_destroy() walks through the linked lists for each possible hash value, and releases the elements. It also releases the nodes array and the obj_hash_table_t. */ -hashtable_rc_t obj_hashtable_destroy(obj_hash_table_t *hashtblP) +hashtable_rc_t obj_hashtable_destroy(obj_hash_table_t * const hashtblP) { hash_size_t n; obj_hash_node_t *node, *oldnode; @@ -103,7 +104,7 @@ hashtable_rc_t obj_hashtable_destroy(obj_hash_table_t *hashtblP) return HASH_TABLE_OK; } //------------------------------------------------------------------------------------------------------------------------------- -hashtable_rc_t obj_hashtable_is_key_exists (obj_hash_table_t *hashtblP, void* keyP, int key_sizeP) +hashtable_rc_t obj_hashtable_is_key_exists (const obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP) //------------------------------------------------------------------------------------------------------------------------------- { obj_hash_node_t *node; @@ -126,12 +127,55 @@ hashtable_rc_t obj_hashtable_is_key_exists (obj_hash_table_t *hashtblP, void* ke } return HASH_TABLE_KEY_NOT_EXISTS; } +//------------------------------------------------------------------------------------------------------------------------------- +hashtable_rc_t obj_hashtable_dump_content ( + const obj_hash_table_t * const hashtblP, + char * const buffer_pP, + int * const remaining_bytes_in_buffer_pP) +//------------------------------------------------------------------------------------------------------------------------------- +{ + obj_hash_node_t *node = NULL; + unsigned int i = 0; + unsigned int num_elements = 0; + int rc; + + if (hashtblP == NULL) { + rc = snprintf( + buffer_pP, + *remaining_bytes_in_buffer_pP, + "HASH_TABLE_BAD_PARAMETER_HASHTABLE"); + return HASH_TABLE_BAD_PARAMETER_HASHTABLE; + } + while ((i < hashtblP->size) && (*remaining_bytes_in_buffer_pP > 0)) { + if (hashtblP->nodes[i] != NULL) { + node=hashtblP->nodes[i]; + while(node) { + rc = snprintf( + buffer_pP, + *remaining_bytes_in_buffer_pP, + "Hash 0x%x Key 0x%"PRIx64" Element %p\n", + i, + node->key, + node->data); + node=node->next; + if ((0 > rc) || (*remaining_bytes_in_buffer_pP < rc)) { + fprintf(stderr, "Error while dumping hashtable content"); + } else { + *remaining_bytes_in_buffer_pP -= rc; + } + } + } + i += 1; + } + return HASH_TABLE_OK; +} + //------------------------------------------------------------------------------------------------------------------------------- /* * Adding a new element * To make sure the hash value is not bigger than size, the result of the user provided hash function is used modulo size. */ -hashtable_rc_t obj_hashtable_insert(obj_hash_table_t *hashtblP, void* keyP, int key_sizeP, void *dataP) +hashtable_rc_t obj_hashtable_insert(obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP, void *dataP) { obj_hash_node_t *node; hash_size_t hash; @@ -146,15 +190,21 @@ hashtable_rc_t obj_hashtable_insert(obj_hash_table_t *hashtblP, void* keyP, int if (node->data) { hashtblP->freedatafunc(node->data); } - node->data=dataP; + node->data = dataP; + node->key_size = key_sizeP; // waste of memory here (keyP is lost) we should free it now return HASH_TABLE_INSERT_OVERWRITTEN_DATA; } node=node->next; } if(!(node=malloc(sizeof(obj_hash_node_t)))) return -1; - node->key=keyP; - node->data=dataP; + if(!(node->key=malloc(key_sizeP))) { + free(node); + return -1; + } + memcpy(node->key, keyP, key_sizeP); + node->data = dataP; + node->key_size = key_sizeP; if (hashtblP->nodes[hash]) { node->next=hashtblP->nodes[hash]; } else { @@ -162,13 +212,45 @@ hashtable_rc_t obj_hashtable_insert(obj_hash_table_t *hashtblP, void* keyP, int } hashtblP->nodes[hash]=node; return HASH_TABLE_OK; +}//------------------------------------------------------------------------------------------------------------------------------- +/* + * To remove an element from the hash table, we just search for it in the linked list for that hash value, + * and remove it if it is found. If it was not found, it is an error and -1 is returned. + */ +hashtable_rc_t obj_hashtable_free(obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP) +{ + obj_hash_node_t *node, *prevnode=NULL; + hash_size_t hash; + + if (hashtblP == NULL) { + return HASH_TABLE_BAD_PARAMETER_HASHTABLE; + } + + hash=hashtblP->hashfunc(keyP, key_sizeP)%hashtblP->size; + node=hashtblP->nodes[hash]; + while(node) { + if ((node->key == keyP) || ((node->key_size == key_sizeP) && (memcmp(node->key, keyP, key_sizeP) == 0))){ + if(prevnode) { + prevnode->next=node->next; + } else { + hashtblP->nodes[hash]=node->next; + } + hashtblP->freekeyfunc(node->key); + hashtblP->freedatafunc(node->data); + free(node); + return HASH_TABLE_OK; + } + prevnode=node; + node=node->next; + } + return HASH_TABLE_KEY_NOT_EXISTS; } //------------------------------------------------------------------------------------------------------------------------------- /* * To remove an element from the hash table, we just search for it in the linked list for that hash value, * and remove it if it is found. If it was not found, it is an error and -1 is returned. */ -hashtable_rc_t obj_hashtable_remove(obj_hash_table_t *hashtblP, const void* keyP, int key_sizeP) +hashtable_rc_t obj_hashtable_remove(obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP, void** dataP) { obj_hash_node_t *node, *prevnode=NULL; hash_size_t hash; @@ -187,7 +269,7 @@ hashtable_rc_t obj_hashtable_remove(obj_hash_table_t *hashtblP, const void* keyP hashtblP->nodes[hash]=node->next; } hashtblP->freekeyfunc(node->key); - hashtblP->freedatafunc(node->data); + *dataP = node->data; free(node); return HASH_TABLE_OK; } @@ -201,7 +283,7 @@ hashtable_rc_t obj_hashtable_remove(obj_hash_table_t *hashtblP, const void* keyP * Searching for an element is easy. We just search through the linked list for the corresponding hash value. * NULL is returned if we didn't find it. */ -hashtable_rc_t obj_hashtable_get(obj_hash_table_t *hashtblP, const void* keyP, int key_sizeP, void** dataP) +hashtable_rc_t obj_hashtable_get(const obj_hash_table_t *const hashtblP, const void* const keyP, const int key_sizeP, void** dataP) { obj_hash_node_t *node; hash_size_t hash; @@ -231,7 +313,7 @@ hashtable_rc_t obj_hashtable_get(obj_hash_table_t *hashtblP, const void* keyP, i /* * Function to return all keys of an object hash table */ -hashtable_rc_t obj_hashtable_get_keys(obj_hash_table_t *hashtblP, void ** keysP, unsigned int *sizeP) +hashtable_rc_t obj_hashtable_get_keys(const obj_hash_table_t * const hashtblP, void ** keysP, unsigned int * sizeP) { size_t n = 0; obj_hash_node_t *node = NULL; @@ -258,10 +340,10 @@ hashtable_rc_t obj_hashtable_get_keys(obj_hash_table_t *hashtblP, void ** keysP, * If the number of elements are reduced, the hash table will waste memory. That is why we provide a function for resizing the table. * Resizing a hash table is not as easy as a realloc(). All hash values must be recalculated and each element must be inserted into its new position. * We create a temporary obj_hash_table_t object (newtbl) to be used while building the new hashes. - * This allows us to reuse hashtable_insert() and hashtable_remove(), when moving the elements to the new table. + * This allows us to reuse hashtable_insert() and hashtable_free(), when moving the elements to the new table. * After that, we can just free the old table and copy the elements from newtbl to hashtbl. */ -hashtable_rc_t obj_hashtable_resize(obj_hash_table_t *hashtblP, hash_size_t sizeP) +hashtable_rc_t obj_hashtable_resize(obj_hash_table_t * const hashtblP, const hash_size_t sizeP) { obj_hash_table_t newtbl; hash_size_t n; @@ -280,7 +362,7 @@ hashtable_rc_t obj_hashtable_resize(obj_hash_table_t *hashtblP, hash_size_t size for(node=hashtblP->nodes[n]; node; node=next) { next = node->next; obj_hashtable_insert(&newtbl, node->key, node->key_size, node->data); - obj_hashtable_remove(hashtblP, node->key, node->key_size); + obj_hashtable_free(hashtblP, node->key, node->key_size); } } diff --git a/common/utils/collection/hashtable/obj_hashtable.h b/common/utils/collection/hashtable/obj_hashtable.h index bf7ea89b96..6bd17793fa 100755 --- a/common/utils/collection/hashtable/obj_hashtable.h +++ b/common/utils/collection/hashtable/obj_hashtable.h @@ -52,14 +52,16 @@ typedef struct obj_hash_table_s { void (*freedatafunc)(void*); } obj_hash_table_t; -obj_hash_table_t *obj_hashtable_create (hash_size_t size, hash_size_t (*hashfunc)(const void*, int ), void (*freekeyfunc)(void*), void (*freedatafunc)(void*)); -hashtable_rc_t obj_hashtable_destroy (obj_hash_table_t *hashtblP); -hashtable_rc_t obj_hashtable_is_key_exists (obj_hash_table_t *hashtblP, void* keyP, int key_sizeP); -hashtable_rc_t obj_hashtable_insert (obj_hash_table_t *hashtblP, void* keyP, int key_sizeP, void *dataP); -hashtable_rc_t obj_hashtable_remove (obj_hash_table_t *hashtblP, const void* keyP, int key_sizeP); -hashtable_rc_t obj_hashtable_get (obj_hash_table_t *hashtblP, const void* keyP, int key_sizeP, void ** dataP); -hashtable_rc_t obj_hashtable_get_keys(obj_hash_table_t *hashtblP, void ** keysP, unsigned int *sizeP); -hashtable_rc_t obj_hashtable_resize (obj_hash_table_t *hashtblP, hash_size_t sizeP); +obj_hash_table_t *obj_hashtable_create (const hash_size_t size, hash_size_t (*hashfunc)(const void*, int ), void (*freekeyfunc)(void*), void (*freedatafunc)(void*)); +hashtable_rc_t obj_hashtable_destroy (obj_hash_table_t * const hashtblP); +hashtable_rc_t obj_hashtable_is_key_exists (const obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP); +hashtable_rc_t obj_hashtable_insert (obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP, void *dataP); +hashtable_rc_t obj_hashtable_dump_content (const obj_hash_table_t * const hashtblP,char * const buffer_pP,int * const remaining_bytes_in_buffer_pP); +hashtable_rc_t obj_hashtable_free (obj_hash_table_t *hashtblP, const void* keyP, const int key_sizeP); +hashtable_rc_t obj_hashtable_remove(obj_hash_table_t *hashtblP, const void* keyP, const int key_sizeP, void** dataP); +hashtable_rc_t obj_hashtable_get (const obj_hash_table_t * const hashtblP, const void* const keyP, const int key_sizeP, void ** dataP); +hashtable_rc_t obj_hashtable_get_keys(const obj_hash_table_t * const hashtblP, void ** keysP, unsigned int * sizeP); +hashtable_rc_t obj_hashtable_resize (obj_hash_table_t * const hashtblP, const hash_size_t sizeP); #endif -- GitLab