linux-kernel - Re: [PATCH v6] 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]

Message-ID: <20120927131118.GC30139@Krystal>
Date:	Thu, 27 Sep 2012 09:11:18 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:	Sasha Levin <levinsasha928@...il.com>
Cc:	David Laight <David.Laight@...LAB.COM>,
	Steven Rostedt <rostedt@...dmis.org>,
	torvalds@...ux-foundation.org, tj@...nel.org,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	ebiederm@...ssion.com, neilb@...e.de, bfields@...ldses.org,
	ejt@...hat.com, snitzer@...hat.com, edumazet@...gle.com,
	josh@...htriplett.org, rmallon@...il.com, palves@...hat.com
Subject: Re: [PATCH v6] hashtable: introduce a small and naive hashtable

* Sasha Levin (levinsasha928@...il.com) wrote:
> On 09/27/2012 10:25 AM, David Laight wrote:
> >>>> And even then, if we would do:
> >>>>
> >>>>  	for (i = 0; i < HASH_SIZE(hashtable); i++)
> >>>>  		if (!hlist_empty(&hashtable[i]))
> >>>>  			break;
> >>>>
> >>>>  	return i >= HASH_SIZE(hashtable);
> >>>>
> >>>> What happens if the last entry of the table is non-empty ?
> >>>
> >>> It still works, as 'i' is not incremented due to the break. And i will
> >>> still be less than HASH_SIZE(hashtable). Did you have *your* cup of
> >>> coffee today? ;-)
> >>
> >> Ahh, right! Actually I had it already ;-)
> > 
> > I tend to dislike the repeated test, gcc might be able to optimise
> > it away, but the code is cleaner written as:
> > 
> > 	for (i = 0; i < HASH_SIZE(hashtable); i++)
> > 		if (!hlist_empty(&hashtable[i]))
> > 			return false;
> > 	return true;
> 
> Right, the flag thing in the macro was there just to make it work
> properly as a macro.
> 
> >> Agreed that the flags should be removed. Moving to define + static
> >> inline is still important though.
> > 
> > Not sure I'd bother making the function inline.
> 
> I usually never make anything 'inline', I just let gcc do it's own
> thing when it compiles the code. If there are any objections
> please let me know before I send the new version.

AFAIK, gcc nowadays use "inline" only as a hint, because programmers
were using it everywhere, even where it should not have been used. This
is where the attribute always_inline becomes useful, if you really,
really want to inline. However, for kernel coding style consistency, it
might be better to use "static inline", because it is used everywhere
else in kernel headers. Or maybe are there some headers that do not use
"inline" I am not aware of ?

Moreover, if your thinking is that we do not need a static inline
function replicated at every caller, maybe we should introduce a
lib/hashtable.c that implements those 2 functions.

Thanks,

Mathieu

> 
> 
> Thanks,
> Sasha
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
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/