linux-kernel - Re: [RFC 1/4] hashtable: introduce a small and naive hashtable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Jul 2012 11:23:30 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Sasha Levin <levinsasha928@...il.com>
Cc:	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	paul.gortmaker@...driver.com
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/