public inbox for b.a.t.m.a.n@lists.open-mesh.org
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred
@ 2014-05-24 13:56 Sven Eckelmann
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop Sven Eckelmann
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sven Eckelmann @ 2014-05-24 13:56 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

All new functionality, cleanup and similar things to the hash implementation
were done in alfred. So import the newest version to keep it in sync.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 hash.c | 88 +++++++++++++++++++++++++++++++++++++++++-------------------------
 hash.h | 69 ++++++++++++++++++++++++++++++---------------------
 2 files changed, 96 insertions(+), 61 deletions(-)

diff --git a/hash.c b/hash.c
index 11a97d5..2f4aa9f 100644
--- a/hash.c
+++ b/hash.c
@@ -20,11 +20,11 @@
  */
 
 
-#include <stdio.h>	/* NULL */
 #include "hash.h"
+#include <stdlib.h>
+#include <stdio.h>
 #include "allocate.h"
 
-
 /* clears the hash */
 void hash_init(struct hashtable_t *hash)
 {
@@ -70,9 +70,17 @@ void hash_destroy(struct hashtable_t *hash)
 	debugFree(hash, 1303);
 }
 
+/* free hash_it_t pointer when stopping hash_iterate early */
+void hash_iterate_free(struct hash_it_t *iter_in)
+{
+	debugFree(iter_in, 1304);
+}
+
 /* iterate though the hash. first element is selected with iter_in NULL.
- * use the returned iterator to access the elements until hash_it_t returns NULL. */
-struct hash_it_t *hash_iterate(struct hashtable_t *hash, struct hash_it_t *iter_in)
+ * use the returned iterator to access the elements until hash_it_t returns
+ * NULL. */
+struct hash_it_t *hash_iterate(struct hashtable_t *hash,
+			       struct hash_it_t *iter_in)
 {
 	struct hash_it_t *iter;
 
@@ -81,14 +89,17 @@ struct hash_it_t *hash_iterate(struct hashtable_t *hash, struct hash_it_t *iter_
 		iter->index =  -1;
 		iter->bucket = NULL;
 		iter->prev_bucket = NULL;
-	} else
-		iter= iter_in;
+	} else {
+		iter = iter_in;
+	}
 
-	/* sanity checks first (if our bucket got deleted in the last iteration): */
+	/* sanity checks first (if our bucket got deleted in the last
+	 * iteration): */
 	if (iter->bucket != NULL) {
 		if (iter->first_bucket != NULL) {
 
-			/* we're on the first element and it got removed after the last iteration. */
+			/* we're on the first element and it got removed after
+			 * the last iteration. */
 			if ((*iter->first_bucket) != iter->bucket) {
 
 				/* there are still other elements in the list */
@@ -105,9 +116,10 @@ struct hash_it_t *hash_iterate(struct hashtable_t *hash, struct hash_it_t *iter_
 
 		} else if (iter->prev_bucket != NULL) {
 
-			/* we're not on the first element, and the bucket got removed after the last iteration.
-			* the last bucket's next pointer is not pointing to our actual bucket anymore.
-			* select the next. */
+			/* we're not on the first element, and the bucket got
+			 * removed after the last iteration. The last bucket's
+			 * next pointer is not pointing to our actual bucket
+			 * anymore. Select the next. */
 			if (iter->prev_bucket->next != iter->bucket)
 				iter->bucket = iter->prev_bucket;
 
@@ -124,7 +136,8 @@ struct hash_it_t *hash_iterate(struct hashtable_t *hash, struct hash_it_t *iter_
 			return iter;
 		}
 	}
-	/* if not returned yet, we've reached the last one on the index and have to search forward */
+	/* if not returned yet, we've reached the last one on the index and
+	 * have to search forward */
 
 	iter->index++;
 	/* go through the entries of the hash table */
@@ -142,12 +155,13 @@ struct hash_it_t *hash_iterate(struct hashtable_t *hash, struct hash_it_t *iter_
 	}
 
 	/* nothing to iterate over anymore */
-	debugFree(iter, 1304);
+	hash_iterate_free(iter);
 	return NULL;
 }
 
 /* allocates and clears the hash */
-struct hashtable_t *hash_new(int size, hashdata_compare_cb compare, hashdata_choose_cb choose)
+struct hashtable_t *hash_new(int size, hashdata_compare_cb compare,
+			     hashdata_choose_cb choose)
 {
 	struct hashtable_t *hash;
 
@@ -155,8 +169,8 @@ struct hashtable_t *hash_new(int size, hashdata_compare_cb compare, hashdata_cho
 	if (!hash)
 		return NULL;
 
-	hash->size= size;
-	hash->table= debugMalloc(sizeof(struct element_t *) * size, 303);
+	hash->size = size;
+	hash->table = debugMalloc(sizeof(struct element_t *)*size, 303);
 
 	if (!hash->table) {
 		debugFree(hash, 1305);
@@ -183,11 +197,11 @@ int hash_add(struct hashtable_t *hash, void *data)
 			return -1;
 
 		prev_bucket = bucket;
-		bucket= bucket->next;
+		bucket = bucket->next;
 	}
 
 	/* found the tail of the list, add new element */
-	bucket = debugMalloc(sizeof(struct element_t),304);
+	bucket = debugMalloc(sizeof(struct element_t), 304);
 
 	if (!bucket)
 		return -1;
@@ -197,17 +211,17 @@ int hash_add(struct hashtable_t *hash, void *data)
 	bucket->next = NULL;
 
 	/* and link it */
-	if (prev_bucket == NULL) {
+	if (prev_bucket == NULL)
 		hash->table[index] = bucket;
-	} else {
+	else
 		prev_bucket->next = bucket;
-	}
 
 	hash->elements++;
 	return 0;
 }
 
-/* finds data, based on the key in keydata. returns the found data on success, or NULL on error */
+/* finds data, based on the key in keydata. returns the found data on success,
+ * or NULL on error */
 void *hash_find(struct hashtable_t *hash, void *keydata)
 {
 	int index;
@@ -226,20 +240,21 @@ void *hash_find(struct hashtable_t *hash, void *keydata)
 	return NULL;
 }
 
-/* remove bucket (this might be used in hash_iterate() if you already found the bucket
- * you want to delete and don't need the overhead to find it again with hash_remove().
- * But usually, you don't want to use this function, as it fiddles with hash-internals. */
+/* remove bucket (this might be used in hash_iterate() if you already found
+ * the bucket you want to delete and don't need the overhead to find it again
+ * with hash_remove(). But usually, you don't want to use this function, as it
+ * fiddles with hash-internals. */
 void *hash_remove_bucket(struct hashtable_t *hash, struct hash_it_t *hash_it_t)
 {
 	void *data_save;
 
-	data_save = hash_it_t->bucket->data;	/* save the pointer to the data */
+	/* save the pointer to the data */
+	data_save = hash_it_t->bucket->data;
 
-	if (hash_it_t->prev_bucket != NULL) {
+	if (hash_it_t->prev_bucket != NULL)
 		hash_it_t->prev_bucket->next = hash_it_t->bucket->next;
-	} else if (hash_it_t->first_bucket != NULL) {
+	else if (hash_it_t->first_bucket != NULL)
 		(*hash_it_t->first_bucket) = hash_it_t->bucket->next;
-	}
 
 	debugFree(hash_it_t->bucket, 1306);
 
@@ -261,7 +276,12 @@ void *hash_remove(struct hashtable_t *hash, void *data)
 
 	while (hash_it_t.bucket != NULL) {
 		if (hash->compare(hash_it_t.bucket->data, data)) {
-			hash_it_t.first_bucket = (hash_it_t.bucket == hash->table[hash_it_t.index] ? &hash->table[ hash_it_t.index ] : NULL);
+			int bucket_same;
+			bucket_same = (hash_it_t.bucket ==
+				       hash->table[hash_it_t.index]);
+			hash_it_t.first_bucket = (bucket_same ?
+						  &hash->table[hash_it_t.index] :
+						  NULL);
 			return hash_remove_bucket(hash, &hash_it_t);
 		}
 
@@ -272,7 +292,8 @@ void *hash_remove(struct hashtable_t *hash, void *data)
 	return NULL;
 }
 
-/* resize the hash, returns the pointer to the new hash or NULL on error. removes the old hash on success. */
+/* resize the hash, returns the pointer to the new hash or NULL on error.
+ * removes the old hash on success. */
 struct hashtable_t *hash_resize(struct hashtable_t *hash, int size)
 {
 	struct hashtable_t *new_hash;
@@ -292,8 +313,9 @@ struct hashtable_t *hash_resize(struct hashtable_t *hash, int size)
 			bucket = bucket->next;
 		}
 	}
-
-	hash_delete(hash, NULL);	/* remove hash and eventual overflow buckets but not the content itself. */
+	/* remove hash and eventual overflow buckets but not the
+	 * content itself. */
+	hash_delete(hash, NULL);
 	return new_hash;
 }
 
diff --git a/hash.h b/hash.h
index 452f836..c9c8fb1 100644
--- a/hash.h
+++ b/hash.h
@@ -28,8 +28,8 @@ typedef int (*hashdata_choose_cb)(void *, int);
 typedef void (*hashdata_free_cb)(void *);
 
 struct element_t {
-	void *data;						/* pointer to the data */
-	struct element_t *next;			/* overflow bucket pointer */
+	void *data;		/* pointer to the data */
+	struct element_t *next;	/* overflow bucket pointer */
 };
 
 struct hash_it_t {
@@ -40,56 +40,69 @@ struct hash_it_t {
 };
 
 struct hashtable_t {
-	struct element_t **table;					/* the hashtable itself, with the buckets */
-	int elements;								/* number of elements registered */
-	int size;									/* size of hashtable */
-	hashdata_compare_cb compare;			    /* callback to a compare function.
-												 * should compare 2 element datas for their keys,
-												 * return 0 if same and not 0 if not same */
-	hashdata_choose_cb choose;					/* the hashfunction, should return an index based
-												 * on the key in the data of the first argument
-												 * and the size the second */
+	struct element_t **table;	/* the hashtable itself, with the
+					 * buckets */
+	int elements;			/* number of elements registered */
+	int size;			/* size of hashtable */
+	hashdata_compare_cb compare;	/* callback to a compare function.
+					 * should compare 2 element datas for
+					 * their keys, return 0 if same and not
+					 *  0 if not same */
+	hashdata_choose_cb choose;	/* the hashfunction, should return an
+					 * index based on the key in the data
+					 * of the first argument and the size
+					 * the second */
 };
 
 /* clears the hash */
-void 				 hash_init(struct hashtable_t *hash);
+void hash_init(struct hashtable_t *hash);
 
 /* allocates and clears the hash */
-struct hashtable_t	*hash_new(int size, hashdata_compare_cb compare, hashdata_choose_cb choose);
+struct hashtable_t *hash_new(int size, hashdata_compare_cb compare,
+				 hashdata_choose_cb choose);
 
-/* remove bucket (this might be used in hash_iterate() if you already found the bucket
- * you want to delete and don't need the overhead to find it again with hash_remove().
- * But usually, you don't want to use this function, as it fiddles with hash-internals. */
-void 				*hash_remove_bucket(struct hashtable_t *hash, struct hash_it_t *hash_it_t);
+/* remove bucket (this might be used in hash_iterate() if you already found
+ * the bucket you want to delete and don't need the overhead to find it again
+ * with hash_remove(). But usually, you don't want to use this function, as it
+ * fiddles with hash-internals. */
+void *hash_remove_bucket(struct hashtable_t *hash,
+			 struct hash_it_t *hash_it_t);
 
 /* remove the hash structure. if hashdata_free_cb != NULL,
  * this function will be called to remove the elements inside of the hash.
  * if you don't remove the elements, memory might be leaked. */
-void 				 hash_delete(struct hashtable_t *hash, hashdata_free_cb free_cb);
+void hash_delete(struct hashtable_t *hash, hashdata_free_cb free_cb);
 
 /* free only the hashtable and the hash itself. */
-void 				 hash_destroy(struct hashtable_t *hash);
+void hash_destroy(struct hashtable_t *hash);
 
 /* adds data to the hashtable. returns 0 on success, -1 on error */
-int 				 hash_add(struct hashtable_t *hash, void *data);
+int hash_add(struct hashtable_t *hash, void *data);
 
 /* removes data from hash, if found. returns pointer do data on success,
  * so you can remove the used structure yourself, or NULL on error .
  * data could be the structure you use with just the key filled,
  * we just need the key for comparing. */
-void 				*hash_remove(struct hashtable_t *hash, void *data);
+void *hash_remove(struct hashtable_t *hash, void *data);
 
-/* finds data, based on the key in keydata. returns the found data on success, or NULL on error */
-void 				*hash_find(struct hashtable_t *hash, void *keydata);
+/* finds data, based on the key in keydata. returns the found data on success,
+ * or NULL on error */
+void *hash_find(struct hashtable_t *hash, void *keydata);
 
-/* resize the hash, returns the pointer to the new hash or NULL on error. removes the old hash on success */
-struct hashtable_t	*hash_resize(struct hashtable_t *hash, int size);
+/* resize the hash, returns the pointer to the new hash or NULL on error.
+ * removes the old hash on success */
+struct hashtable_t *hash_resize(struct hashtable_t *hash, int size);
 
 /* print the hash table for debugging */
-void 				 hash_debug( struct hashtable_t *hash);
+void hash_debug(struct hashtable_t *hash);
 
 /* iterate though the hash. first element is selected with iter_in NULL.
- * use the returned iterator to access the elements until hash_it_t returns NULL. */
-struct hash_it_t 	*hash_iterate(struct hashtable_t *hash, struct hash_it_t *iter_in);
+ * use the returned iterator to access the elements until hash_it_t
+ * returns NULL. */
+struct hash_it_t *hash_iterate(struct hashtable_t *hash,
+			       struct hash_it_t *iter_in);
+
+/* free hash_it_t pointer when stopping hash_iterate early */
+void hash_iterate_free(struct hash_it_t *iter_in);
 
 #endif
-- 
2.0.0.rc2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop
  2014-05-24 13:56 [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Sven Eckelmann
@ 2014-05-24 13:56 ` Sven Eckelmann
  2014-06-12  9:29   ` Marek Lindner
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 3/3] batctl: Use memleak/error path free implementation of hash_resize Sven Eckelmann
  2014-06-12  9:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Marek Lindner
  2 siblings, 1 reply; 7+ messages in thread
From: Sven Eckelmann @ 2014-05-24 13:56 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

The hash iterator is automatically allocated and freed by the hash_iterate
function. But when using break (or goto outside of loop) during the iteration
loop, the caller has to handle the free-operation of the hash_iterator to avoid
memory leaks.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 bisect_iv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bisect_iv.c b/bisect_iv.c
index e9e7326..7105c78 100644
--- a/bisect_iv.c
+++ b/bisect_iv.c
@@ -1304,8 +1304,10 @@ static void trace_seqnos(char *trace_orig, long long seqno_min, long long seqno_
 
 				res = seqno_trace_add(&trace_list, bat_node, seqno_event, print_trace);
 
-				if (res < 1)
+				if (res < 1) {
+					hash_iterate_free(hashit);
 					goto out;
+				}
 			}
 		}
 	}
-- 
2.0.0.rc2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [B.A.T.M.A.N.] [PATCH 3/3] batctl: Use memleak/error path free implementation of hash_resize
  2014-05-24 13:56 [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Sven Eckelmann
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop Sven Eckelmann
@ 2014-05-24 13:56 ` Sven Eckelmann
  2014-05-27 14:23   ` [B.A.T.M.A.N.] [PATCHv2 " Sven Eckelmann
  2014-06-12  9:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Marek Lindner
  2 siblings, 1 reply; 7+ messages in thread
From: Sven Eckelmann @ 2014-05-24 13:56 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

The current implementation of hash_resize uses hash_add directly to initialize
two a new hash table. But hash_add has two error cases: Data already exists and
malloc fails.

The check for the duplicated data is not really harmful (beside increasing the
time to re-add elements) but the malloc can potentially return an error. This
malloc is unnecessary and just takes extra time and is a potential candidate
for errors. Instead the bucket from the old hash table can be re-used.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 hash.c | 72 ++++++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/hash.c b/hash.c
index 2f4aa9f..14debcb 100644
--- a/hash.c
+++ b/hash.c
@@ -63,6 +63,40 @@ void hash_delete(struct hashtable_t *hash, hashdata_free_cb free_cb)
 	hash_destroy(hash);
 }
 
+/* adds data to the hashtable and reuse bucket.
+ * returns 0 on success, -1 on error  */
+static int hash_add_bucket(struct hashtable_t *hash, void *data,
+			   struct element_t *bucket, int check_duplicate)
+{
+	int index;
+	struct element_t *bucket_it, *prev_bucket = NULL;
+
+	index = hash->choose(data, hash->size);
+	bucket_it = hash->table[index];
+
+	while (bucket_it != NULL) {
+		if (check_duplicate &&
+		    hash->compare(bucket_it->data, data))
+			return -1;
+
+		prev_bucket = bucket_it;
+		bucket_it = bucket_it->next;
+	}
+
+	/* init the new bucket */
+	bucket->data = data;
+	bucket->next = NULL;
+
+	/* and link it */
+	if (prev_bucket == NULL)
+		hash->table[index] = bucket;
+	else
+		prev_bucket->next = bucket;
+
+	hash->elements++;
+	return 0;
+}
+
 /* free only the hashtable and the hash itself. */
 void hash_destroy(struct hashtable_t *hash)
 {
@@ -186,19 +220,8 @@ struct hashtable_t *hash_new(int size, hashdata_compare_cb compare,
 /* adds data to the hashtable. returns 0 on success, -1 on error */
 int hash_add(struct hashtable_t *hash, void *data)
 {
-	int index;
-	struct element_t *bucket, *prev_bucket = NULL;
-
-	index = hash->choose(data, hash->size);
-	bucket = hash->table[index];
-
-	while (bucket != NULL) {
-		if (hash->compare(bucket->data, data))
-			return -1;
-
-		prev_bucket = bucket;
-		bucket = bucket->next;
-	}
+	int ret;
+	struct element_t *bucket;
 
 	/* found the tail of the list, add new element */
 	bucket = debugMalloc(sizeof(struct element_t), 304);
@@ -206,18 +229,11 @@ int hash_add(struct hashtable_t *hash, void *data)
 	if (!bucket)
 		return -1;
 
-	/* init the new bucket */
-	bucket->data = data;
-	bucket->next = NULL;
+	ret = hash_add_bucket(hash, data, bucket, 1);
+	if (ret < 0)
+		debugFree(bucket, 1307);
 
-	/* and link it */
-	if (prev_bucket == NULL)
-		hash->table[index] = bucket;
-	else
-		prev_bucket->next = bucket;
-
-	hash->elements++;
-	return 0;
+	return ret;
 }
 
 /* finds data, based on the key in keydata. returns the found data on success,
@@ -307,10 +323,10 @@ struct hashtable_t *hash_resize(struct hashtable_t *hash, int size)
 
 	/* copy the elements */
 	for (i = 0; i < hash->size; i++) {
-		bucket = hash->table[i];
-		while (bucket != NULL) {
-			hash_add(new_hash, bucket->data);
-			bucket = bucket->next;
+		while (hash->table[i]) {
+			bucket = hash->table[i];
+			hash->table[i] = bucket->next;
+			hash_add_bucket(new_hash, bucket->data, bucket, 0);
 		}
 	}
 	/* remove hash and eventual overflow buckets but not the
-- 
2.0.0.rc2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [B.A.T.M.A.N.] [PATCHv2 3/3] batctl: Use memleak/error path free implementation of hash_resize
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 3/3] batctl: Use memleak/error path free implementation of hash_resize Sven Eckelmann
@ 2014-05-27 14:23   ` Sven Eckelmann
  2014-06-12  9:30     ` Marek Lindner
  0 siblings, 1 reply; 7+ messages in thread
From: Sven Eckelmann @ 2014-05-27 14:23 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

The current implementation of hash_resize uses hash_add directly to initialize
a new hash table. But hash_add has two possible situations when it returns an
error and hash_resize would leak the data:

 * data already exists
 * malloc fails

The check for the duplicated data is not really harmful (beside increasing the
time to re-add elements) but the malloc can potentially return an error. This
malloc is unnecessary and just takes extra time. Instead the bucket from the
old hash table can be re-used.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
 * fixed commit message

 hash.c | 72 ++++++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/hash.c b/hash.c
index 2f4aa9f..14debcb 100644
--- a/hash.c
+++ b/hash.c
@@ -63,6 +63,40 @@ void hash_delete(struct hashtable_t *hash, hashdata_free_cb free_cb)
 	hash_destroy(hash);
 }
 
+/* adds data to the hashtable and reuse bucket.
+ * returns 0 on success, -1 on error  */
+static int hash_add_bucket(struct hashtable_t *hash, void *data,
+			   struct element_t *bucket, int check_duplicate)
+{
+	int index;
+	struct element_t *bucket_it, *prev_bucket = NULL;
+
+	index = hash->choose(data, hash->size);
+	bucket_it = hash->table[index];
+
+	while (bucket_it != NULL) {
+		if (check_duplicate &&
+		    hash->compare(bucket_it->data, data))
+			return -1;
+
+		prev_bucket = bucket_it;
+		bucket_it = bucket_it->next;
+	}
+
+	/* init the new bucket */
+	bucket->data = data;
+	bucket->next = NULL;
+
+	/* and link it */
+	if (prev_bucket == NULL)
+		hash->table[index] = bucket;
+	else
+		prev_bucket->next = bucket;
+
+	hash->elements++;
+	return 0;
+}
+
 /* free only the hashtable and the hash itself. */
 void hash_destroy(struct hashtable_t *hash)
 {
@@ -186,19 +220,8 @@ struct hashtable_t *hash_new(int size, hashdata_compare_cb compare,
 /* adds data to the hashtable. returns 0 on success, -1 on error */
 int hash_add(struct hashtable_t *hash, void *data)
 {
-	int index;
-	struct element_t *bucket, *prev_bucket = NULL;
-
-	index = hash->choose(data, hash->size);
-	bucket = hash->table[index];
-
-	while (bucket != NULL) {
-		if (hash->compare(bucket->data, data))
-			return -1;
-
-		prev_bucket = bucket;
-		bucket = bucket->next;
-	}
+	int ret;
+	struct element_t *bucket;
 
 	/* found the tail of the list, add new element */
 	bucket = debugMalloc(sizeof(struct element_t), 304);
@@ -206,18 +229,11 @@ int hash_add(struct hashtable_t *hash, void *data)
 	if (!bucket)
 		return -1;
 
-	/* init the new bucket */
-	bucket->data = data;
-	bucket->next = NULL;
+	ret = hash_add_bucket(hash, data, bucket, 1);
+	if (ret < 0)
+		debugFree(bucket, 1307);
 
-	/* and link it */
-	if (prev_bucket == NULL)
-		hash->table[index] = bucket;
-	else
-		prev_bucket->next = bucket;
-
-	hash->elements++;
-	return 0;
+	return ret;
 }
 
 /* finds data, based on the key in keydata. returns the found data on success,
@@ -307,10 +323,10 @@ struct hashtable_t *hash_resize(struct hashtable_t *hash, int size)
 
 	/* copy the elements */
 	for (i = 0; i < hash->size; i++) {
-		bucket = hash->table[i];
-		while (bucket != NULL) {
-			hash_add(new_hash, bucket->data);
-			bucket = bucket->next;
+		while (hash->table[i]) {
+			bucket = hash->table[i];
+			hash->table[i] = bucket->next;
+			hash_add_bucket(new_hash, bucket->data, bucket, 0);
 		}
 	}
 	/* remove hash and eventual overflow buckets but not the
-- 
2.0.0.rc2



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred
  2014-05-24 13:56 [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Sven Eckelmann
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop Sven Eckelmann
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 3/3] batctl: Use memleak/error path free implementation of hash_resize Sven Eckelmann
@ 2014-06-12  9:28 ` Marek Lindner
  2 siblings, 0 replies; 7+ messages in thread
From: Marek Lindner @ 2014-06-12  9:28 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Saturday 24 May 2014 15:56:16 Sven Eckelmann wrote:
> All new functionality, cleanup and similar things to the hash implementation
> were done in alfred. So import the newest version to keep it in sync.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
>  hash.c | 88
> +++++++++++++++++++++++++++++++++++++++++------------------------- hash.h |
> 69 ++++++++++++++++++++++++++++++---------------------
>  2 files changed, 96 insertions(+), 61 deletions(-)

Applied in revision 63c0932.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop
  2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop Sven Eckelmann
@ 2014-06-12  9:29   ` Marek Lindner
  0 siblings, 0 replies; 7+ messages in thread
From: Marek Lindner @ 2014-06-12  9:29 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

On Saturday 24 May 2014 15:56:17 Sven Eckelmann wrote:
> The hash iterator is automatically allocated and freed by the hash_iterate
> function. But when using break (or goto outside of loop) during the
> iteration loop, the caller has to handle the free-operation of the
> hash_iterator to avoid memory leaks.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
>  bisect_iv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied in revision 690d846.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv2 3/3] batctl: Use memleak/error path free implementation of hash_resize
  2014-05-27 14:23   ` [B.A.T.M.A.N.] [PATCHv2 " Sven Eckelmann
@ 2014-06-12  9:30     ` Marek Lindner
  0 siblings, 0 replies; 7+ messages in thread
From: Marek Lindner @ 2014-06-12  9:30 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Tuesday 27 May 2014 16:23:32 Sven Eckelmann wrote:
> The current implementation of hash_resize uses hash_add directly to
> initialize a new hash table. But hash_add has two possible situations when
> it returns an error and hash_resize would leak the data:
> 
>  * data already exists
>  * malloc fails
> 
> The check for the duplicated data is not really harmful (beside increasing
> the time to re-add elements) but the malloc can potentially return an
> error. This malloc is unnecessary and just takes extra time. Instead the
> bucket from the old hash table can be re-used.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2:
>  * fixed commit message
> 
>  hash.c | 72
> ++++++++++++++++++++++++++++++++++++++++-------------------------- 1 file
> changed, 44 insertions(+), 28 deletions(-)

Applied in revision 6f03fcf.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-06-12  9:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-24 13:56 [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Sven Eckelmann
2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 2/3] batctl: Free hash iterator when breaking out of loop Sven Eckelmann
2014-06-12  9:29   ` Marek Lindner
2014-05-24 13:56 ` [B.A.T.M.A.N.] [PATCH 3/3] batctl: Use memleak/error path free implementation of hash_resize Sven Eckelmann
2014-05-27 14:23   ` [B.A.T.M.A.N.] [PATCHv2 " Sven Eckelmann
2014-06-12  9:30     ` Marek Lindner
2014-06-12  9:28 ` [B.A.T.M.A.N.] [PATCH 1/3] batctl: Import hash table version from alfred Marek Lindner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox