Messages in this thread |  | | | Date | Tue, 31 Jul 2012 11:23:30 -0700 | | From | Tejun Heo <> | | Subject | Re: [RFC 1/4] hashtable: introduce a small and naive hashtable |
| |
Hello, Sasha.
On Tue, Jul 31, 2012 at 08:05:17PM +0200, Sasha Levin wrote: > +#define HASH_INIT(name) \ > +({ \ > + int __i; \ > + for (__i = 0 ; __i < HASH_SIZE(name) ; __i++) \ > + INIT_HLIST_HEAD(&name[__i]); \ > +})
Why use macro?
> +#define HASH_ADD(name, obj, key) \ > + hlist_add_head(obj, &name[ \ > + hash_long((unsigned long)key, HASH_BITS(name))]);
Ditto.
> +#define HASH_GET(name, key, type, member, cmp_fn) \ > +({ \ > + struct hlist_node *__node; \ > + typeof(key) __key = key; \ > + type *__obj = NULL; \ > + hlist_for_each_entry(__obj, __node, &name[ \ > + hash_long((unsigned long) __key, \ > + HASH_BITS(name))], member) \ > + if (cmp_fn(__obj, __key)) \ > + break; \ > + __obj; \ > +})
Wouldn't it be simpler to have something like the following
hash_for_each_possible_match(pos, hash, key)
and let the caller handle the actual comparison? Callbacks often are painful to use and I don't think the above dancing buys much.
> +#define HASH_DEL(obj, member) \ > + hlist_del(&obj->member)
@obj is struct hlist_node in HASH_ADD and the containing type here? Most in-kernel generic data containers implement just the container itself and let the caller handle the conversions between container node and the containing object. I think it would better not to deviate from that.
> +#define HASH_FOR_EACH(bkt, node, name, obj, member) \ > + for (bkt = 0; bkt < HASH_SIZE(name); bkt++) \ > + hlist_for_each_entry(obj, node, &name[i], member)
Why in caps? Most for_each macros are in lower case.
Thanks.
-- tejun
|  |