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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to