All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: Daniel Walker <dwalker@fifo99.com>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Thomas Gleixner <tglx@linutronix.de>,
	john stultz <johnstul@us.ibm.com>
Subject: Re: [RFC][patch 02/12] remove clocksource inline functions
Date: Wed, 29 Jul 2009 18:37:56 +0200	[thread overview]
Message-ID: <20090729183756.7c1314ce@skybase> (raw)
In-Reply-To: <1248882770.28841.236.camel@desktop>

On Wed, 29 Jul 2009 08:52:50 -0700
Daniel Walker <dwalker@fifo99.com> wrote:

> On Wed, 2009-07-29 at 17:32 +0200, Martin Schwidefsky wrote:
> > Hmm, you have an object of type struct clocksource and you do
> > cs->read(cs). If that is not clear enough then I don't know what is.
> 
> It's not as clear as it could be .. In the case above you have to look
> in at least two places to know what's going on.. First to see the
> cs->read() , and second to see if "cs" is actually a clocksource or
> something else.. "cs" could be declared anyplace with any name.

Well you have something like that in the code:

	struct clocksource *clock;

	clock = timekeeper.clock;
	cycle_now = clock->read(clock);

If I read the function top to bottom I immediately see that clock is a
clocksource and that the code does a read on it. That is not the case
if I need to lookup the clocksource_read wrapper.

> If you see clocksource_read(cs) , you might need to once check what
> clocksource_read() is actually doing, but only once.. After that when
> you see that function you know that variable is a clocksource, and it's
> "read()" is getting called. So you only need to review one line in the
> simplest case.

After you learned (once) that timekeeper.clock is a clock source you
have no trouble to understand the 6 occurrences of clock->read(clock)
there are in the code.

Anyway this seems to be a matter of personal preference, in the end I
don't care too much about the inline functions.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


  reply	other threads:[~2009-07-29 16:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200907291457.n6TEvDAt003701@d06av06.portsmouth.uk.ibm.com>
2009-07-29 15:32 ` [RFC][patch 02/12] remove clocksource inline functions Martin Schwidefsky
2009-07-29 15:36   ` Will Newton
2009-07-29 16:27     ` Martin Schwidefsky
2009-07-29 16:44       ` Martin Schwidefsky
2009-07-30 12:21       ` Valdis.Kletnieks
2009-07-30 21:48         ` Christoph Hellwig
2009-07-31 11:50           ` Valdis.Kletnieks
2009-08-03  8:10             ` Martin Schwidefsky
2009-07-29 15:52   ` Daniel Walker
2009-07-29 16:37     ` Martin Schwidefsky [this message]
     [not found] <200907291415.n6TEFJkA019086@d06av05.portsmouth.uk.ibm.com>
2009-07-29 14:44 ` Martin Schwidefsky
2009-07-29 14:57   ` Daniel Walker
2009-07-29 13:41 [RFC][patch 00/12] clocksource / timekeeping rework V2 Martin Schwidefsky
2009-07-29 13:41 ` [RFC][patch 02/12] remove clocksource inline functions Martin Schwidefsky
2009-07-29 14:15   ` Daniel Walker
2009-07-30 21:46     ` Christoph Hellwig
2009-07-30 21:05   ` john stultz

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=20090729183756.7c1314ce@skybase \
    --to=schwidefsky@de.ibm.com \
    --cc=dwalker@fifo99.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.