lkml.org 
[lkml]   [2012]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2012-07-31 22:01    [W:0.023 / U:0.880 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site