public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Sasha Levin <levinsasha928@gmail.com>,
	torvalds@linux-foundation.org, tj@kernel.org,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	ebiederm@xmission.com, neilb@suse.de, bfields@fieldses.org,
	ejt@redhat.com, snitzer@redhat.com, edumazet@google.com,
	josh@joshtriplett.org, rmallon@gmail.com, palves@redhat.com
Subject: Re: [PATCH v6] hashtable: introduce a small and naive hashtable
Date: Thu, 27 Sep 2012 09:03:03 -0400	[thread overview]
Message-ID: <20120927130303.GB30139@Krystal> (raw)
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7009@saturn3.aculab.com>

* David Laight (David.Laight@ACULAB.COM) 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;
> 

Agreed, this looks like a good way to write it.

> > 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.

Do you mean you prefer to keep it as a macro, or that you don't think
the "inline" keyword is relevant anymore, and want to do a "static" only
function in the header file ?

In both cases, please explain the reasons for doing things that way.

Thanks,

Mathieu

> 
> 	David
> 
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

  parent reply	other threads:[~2012-09-27 13:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-26 12:48 [PATCH v6] hashtable: introduce a small and naive hashtable Sasha Levin
2012-09-26 13:45 ` David Laight
2012-09-26 13:59   ` Steven Rostedt
2012-09-26 14:26     ` Sasha Levin
2012-09-26 14:39       ` Mathieu Desnoyers
2012-09-26 16:09         ` Steven Rostedt
2012-09-26 16:19           ` Mathieu Desnoyers
2012-09-27  8:25             ` David Laight
2012-09-27  8:33               ` Sasha Levin
2012-09-27 12:09                 ` Steven Rostedt
2012-09-27 13:11                 ` Mathieu Desnoyers
2012-09-27 13:30                   ` Steven Rostedt
2012-09-27 14:36                   ` David Laight
2012-09-27 13:03               ` Mathieu Desnoyers [this message]
2012-09-26 14:31   ` Mathieu Desnoyers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120927130303.GB30139@Krystal \
    --to=mathieu.desnoyers@efficios.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=akpm@linux-foundation.org \
    --cc=bfields@fieldses.org \
    --cc=ebiederm@xmission.com \
    --cc=edumazet@google.com \
    --cc=ejt@redhat.com \
    --cc=josh@joshtriplett.org \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=palves@redhat.com \
    --cc=rmallon@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=snitzer@redhat.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox