From: Rusty Russell Date: Thu, 11 Oct 2018 23:54:10 +0000 (+1030) Subject: htable: add and integrate htable_check function. X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=b6e8d929d2c0c37afc840ac99e3e0f469e33e6c0 htable: add and integrate htable_check function. Trying to debug what looked like an htable fail in my own code (but seems it's not), I added htable_check a-la list_check et al. This means we now depend on ccan/str (for stringify), which breaks the _info example which also defined streq, and played nasty with const pointers. Signed-off-by: Rusty Russell --- diff --git a/ccan/htable/_info b/ccan/htable/_info index a3bb76db..ea11beb4 100644 --- a/ccan/htable/_info +++ b/ccan/htable/_info @@ -41,7 +41,7 @@ * } * * // Comparison function. - * static bool streq(const void *e, void *string) + * static bool nameeq(const void *e, void *string) * { * return strcmp(((struct name_to_digit *)e)->name, string) == 0; * } @@ -49,13 +49,13 @@ * // We let them add their own aliases, eg. --alias=v=5 * static void add_alias(struct htable *ht, const char *alias) * { - * char *eq; + * char *eq, *name; * struct name_to_digit *n; * * n = malloc(sizeof(*n)); - * n->name = strdup(alias); + * n->name = name = strdup(alias); * - * eq = strchr(n->name, '='); + * eq = strchr(name, '='); * if (!eq || ((n->val = atoi(eq+1)) == 0 && !strcmp(eq+1, "0"))) * errx(1, "Usage: --alias=="); * *eq = '\0'; @@ -89,7 +89,7 @@ * for (val = 0; i < argc; i++) { * struct name_to_digit *n; * n = htable_get(&ht, hash_string(argv[i]), - * streq, argv[i]); + * nameeq, argv[i]); * if (!n) * errx(1, "Invalid digit name %s", argv[i]); * // Append it to the value we are building up. @@ -110,6 +110,7 @@ int main(int argc, char *argv[]) if (strcmp(argv[1], "depends") == 0) { printf("ccan/compiler\n"); + printf("ccan/str\n"); return 0; } diff --git a/ccan/htable/htable.c b/ccan/htable/htable.c index e1d13699..f3568230 100644 --- a/ccan/htable/htable.c +++ b/ccan/htable/htable.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ bool htable_init_sized(struct htable *ht, return false; } htable_adjust_capacity(ht); + (void)htable_debug(ht, HTABLE_LOC); return true; } @@ -88,7 +90,7 @@ void htable_clear(struct htable *ht) htable_init(ht, ht->rehash, ht->priv); } -bool htable_copy(struct htable *dst, const struct htable *src) +bool htable_copy_(struct htable *dst, const struct htable *src) { uintptr_t *htable = malloc(sizeof(size_t) << src->bits); @@ -122,21 +124,21 @@ static void *htable_val(const struct htable *ht, return NULL; } -void *htable_firstval(const struct htable *ht, - struct htable_iter *i, size_t hash) +void *htable_firstval_(const struct htable *ht, + struct htable_iter *i, size_t hash) { i->off = hash_bucket(ht, hash); return htable_val(ht, i, hash, ht->perfect_bit); } -void *htable_nextval(const struct htable *ht, - struct htable_iter *i, size_t hash) +void *htable_nextval_(const struct htable *ht, + struct htable_iter *i, size_t hash) { i->off = (i->off + 1) & ((1 << ht->bits)-1); return htable_val(ht, i, hash, 0); } -void *htable_first(const struct htable *ht, struct htable_iter *i) +void *htable_first_(const struct htable *ht, struct htable_iter *i) { for (i->off = 0; i->off < (size_t)1 << ht->bits; i->off++) { if (entry_is_valid(ht->table[i->off])) @@ -145,7 +147,7 @@ void *htable_first(const struct htable *ht, struct htable_iter *i) return NULL; } -void *htable_next(const struct htable *ht, struct htable_iter *i) +void *htable_next_(const struct htable *ht, struct htable_iter *i) { for (i->off++; i->off < (size_t)1 << ht->bits; i->off++) { if (entry_is_valid(ht->table[i->off])) @@ -154,7 +156,7 @@ void *htable_next(const struct htable *ht, struct htable_iter *i) return NULL; } -void *htable_prev(const struct htable *ht, struct htable_iter *i) +void *htable_prev_(const struct htable *ht, struct htable_iter *i) { for (;;) { if (!i->off) @@ -215,6 +217,8 @@ static COLD bool double_table(struct htable *ht) free(oldtable); } ht->deleted = 0; + + (void)htable_debug(ht, HTABLE_LOC); return true; } @@ -240,6 +244,7 @@ static COLD void rehash_table(struct htable *ht) } } ht->deleted = 0; + (void)htable_debug(ht, HTABLE_LOC); } /* We stole some bits, now we need to put them back... */ @@ -261,6 +266,7 @@ static COLD void update_common(struct htable *ht, const void *p) ht->common_mask = ~((uintptr_t)1 << i); ht->common_bits = ((uintptr_t)p & ht->common_mask); ht->perfect_bit = 1; + (void)htable_debug(ht, HTABLE_LOC); return; } @@ -283,9 +289,10 @@ static COLD void update_common(struct htable *ht, const void *p) ht->common_mask &= ~maskdiff; ht->common_bits &= ~maskdiff; ht->perfect_bit &= ~maskdiff; + (void)htable_debug(ht, HTABLE_LOC); } -bool htable_add(struct htable *ht, size_t hash, const void *p) +bool htable_add_(struct htable *ht, size_t hash, const void *p) { if (ht->elems+1 > ht->max && !double_table(ht)) return false; @@ -300,7 +307,7 @@ bool htable_add(struct htable *ht, size_t hash, const void *p) return true; } -bool htable_del(struct htable *ht, size_t h, const void *p) +bool htable_del_(struct htable *ht, size_t h, const void *p) { struct htable_iter i; void *c; @@ -314,7 +321,7 @@ bool htable_del(struct htable *ht, size_t h, const void *p) return false; } -void htable_delval(struct htable *ht, struct htable_iter *i) +void htable_delval_(struct htable *ht, struct htable_iter *i) { assert(i->off < (size_t)1 << ht->bits); assert(entry_is_valid(ht->table[i->off])); @@ -323,3 +330,53 @@ void htable_delval(struct htable *ht, struct htable_iter *i) ht->table[i->off] = HTABLE_DELETED; ht->deleted++; } + +struct htable *htable_check(const struct htable *ht, const char *abortstr) +{ + void *p; + struct htable_iter i; + size_t n = 0; + + /* Use non-DEBUG versions here, to avoid infinite recursion with + * CCAN_HTABLE_DEBUG! */ + for (p = htable_first_(ht, &i); p; p = htable_next_(ht, &i)) { + struct htable_iter i2; + void *c; + size_t h = ht->rehash(p, ht->priv); + bool found = false; + + n++; + + /* Open-code htable_get to avoid CCAN_HTABLE_DEBUG */ + for (c = htable_firstval_(ht, &i2, h); + c; + c = htable_nextval_(ht, &i2, h)) { + if (c == p) { + found = true; + break; + } + } + + if (!found) { + if (abortstr) { + fprintf(stderr, + "%s: element %p in position %zu" + " cannot find itself\n", + abortstr, p, i.off); + abort(); + } + return NULL; + } + } + if (n != ht->elems) { + if (abortstr) { + fprintf(stderr, + "%s: found %zu elems, expected %zu\n", + abortstr, n, ht->elems); + abort(); + } + return NULL; + } + + return (struct htable *)ht; +} diff --git a/ccan/htable/htable.h b/ccan/htable/htable.h index 9845388e..53c447c0 100644 --- a/ccan/htable/htable.h +++ b/ccan/htable/htable.h @@ -2,10 +2,19 @@ #ifndef CCAN_HTABLE_H #define CCAN_HTABLE_H #include "config.h" +#include #include #include #include +/* Define CCAN_HTABLE_DEBUG for expensive debugging checks on each call. */ +#define HTABLE_LOC __FILE__ ":" stringify(__LINE__) +#ifdef CCAN_HTABLE_DEBUG +#define htable_debug(h, loc) htable_check((h), loc) +#else +#define htable_debug(h, loc) ((void)loc, h) +#endif + /** * struct htable - private definition of a htable. * @@ -75,6 +84,22 @@ bool htable_init_sized(struct htable *ht, */ void htable_clear(struct htable *ht); + +/** + * htable_check - check hash table for consistency + * @ht: the htable + * @abortstr: the location to print on aborting, or NULL. + * + * Because hash tables have redundant information, consistency checking that + * each element is in the correct location can be done. This is useful as a + * debugging check. If @abortstr is non-NULL, that will be printed in a + * diagnostic if the htable is inconsistent, and the function will abort. + * + * Returns the htable if it is consistent, NULL if not (it can never return + * NULL if @abortstr is set). + */ +struct htable *htable_check(const struct htable *ht, const char *abortstr); + /** * htable_copy - duplicate a hash table. * @dst: the hash table to overwrite @@ -92,14 +117,8 @@ void htable_clear(struct htable *ht); * } * return true; */ -bool htable_copy(struct htable *dst, const struct htable *src); - -/** - * htable_rehash - use a hashtree's rehash function - * @elem: the argument to rehash() - * - */ -size_t htable_rehash(const void *elem); +#define htable_copy(dst, src) htable_copy_(dst, htable_debug(src, HTABLE_LOC)) +bool htable_copy_(struct htable *dst, const struct htable *src); /** * htable_add - add a pointer into a hash table. @@ -110,7 +129,9 @@ size_t htable_rehash(const void *elem); * Also note that this can only fail due to allocation failure. Otherwise, it * returns true. */ -bool htable_add(struct htable *ht, size_t hash, const void *p); +#define htable_add(ht, hash, p) \ + htable_add_(htable_debug(ht, HTABLE_LOC), hash, p) +bool htable_add_(struct htable *ht, size_t hash, const void *p); /** * htable_del - remove a pointer from a hash table @@ -120,7 +141,9 @@ bool htable_add(struct htable *ht, size_t hash, const void *p); * * Returns true if the pointer was found (and deleted). */ -bool htable_del(struct htable *ht, size_t hash, const void *p); +#define htable_del(ht, hash, p) \ + htable_del_(htable_debug(ht, HTABLE_LOC), hash, p) +bool htable_del_(struct htable *ht, size_t hash, const void *p); /** * struct htable_iter - iterator or htable_first or htable_firstval etc. @@ -141,8 +164,11 @@ struct htable_iter { * See Also: * htable_delval() */ -void *htable_firstval(const struct htable *htable, - struct htable_iter *i, size_t hash); +#define htable_firstval(htable, i, hash) \ + htable_firstval_(htable_debug(htable, HTABLE_LOC), i, hash) + +void *htable_firstval_(const struct htable *htable, + struct htable_iter *i, size_t hash); /** * htable_nextval - find another candidate for a given hash value @@ -152,8 +178,10 @@ void *htable_firstval(const struct htable *htable, * * You'll need to check the value is what you want; returns NULL if no more. */ -void *htable_nextval(const struct htable *htable, - struct htable_iter *i, size_t hash); +#define htable_nextval(htable, i, hash) \ + htable_nextval_(htable_debug(htable, HTABLE_LOC), i, hash) +void *htable_nextval_(const struct htable *htable, + struct htable_iter *i, size_t hash); /** * htable_get - find an entry in the hash table @@ -186,7 +214,9 @@ static inline void *htable_get(const struct htable *ht, * * Get an entry in the hashtable; NULL if empty. */ -void *htable_first(const struct htable *htable, struct htable_iter *i); +#define htable_first(htable, i) \ + htable_first_(htable_debug(htable, HTABLE_LOC), i) +void *htable_first_(const struct htable *htable, struct htable_iter *i); /** * htable_next - find another entry in the hash table @@ -196,7 +226,9 @@ void *htable_first(const struct htable *htable, struct htable_iter *i); * Get another entry in the hashtable; NULL if all done. * This is usually used after htable_first or prior non-NULL htable_next. */ -void *htable_next(const struct htable *htable, struct htable_iter *i); +#define htable_next(htable, i) \ + htable_next_(htable_debug(htable, HTABLE_LOC), i) +void *htable_next_(const struct htable *htable, struct htable_iter *i); /** * htable_prev - find the previous entry in the hash table @@ -211,7 +243,9 @@ void *htable_next(const struct htable *htable, struct htable_iter *i); * This is usually used in the middle of (or after) a htable_next iteration and * to "unwind" actions taken. */ -void *htable_prev(const struct htable *htable, struct htable_iter *i); +#define htable_prev(htable, i) \ + htable_prev_(htable_debug(htable, HTABLE_LOC), i) +void *htable_prev_(const struct htable *htable, struct htable_iter *i); /** * htable_delval - remove an iterated pointer from a hash table @@ -221,6 +255,8 @@ void *htable_prev(const struct htable *htable, struct htable_iter *i); * Usually used to delete a hash entry after it has been found with * htable_firstval etc. */ -void htable_delval(struct htable *ht, struct htable_iter *i); +#define htable_delval(htable, i) \ + htable_delval_(htable_debug(htable, HTABLE_LOC), i) +void htable_delval_(struct htable *ht, struct htable_iter *i); #endif /* CCAN_HTABLE_H */