* Sasha Levin ([email protected]) 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
