All of lore.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.