public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Nicholas Mc Guire <hofrat@osadl.org>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Hartmut Knaack <knaack.h@gmx.de>,
	Peter Meerwald <pmeerw@pmeerw.net>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] iio: ad_sigma_delta: cleanup wait_for_completion return handling
Date: Mon, 02 Feb 2015 10:41:35 +0100	[thread overview]
Message-ID: <54CF464F.9000306@metafoo.de> (raw)
In-Reply-To: <1422866258-10517-1-git-send-email-hofrat@osadl.org>

On 02/02/2015 09:37 AM, Nicholas Mc Guire wrote:
> return type of wait_for_completion_timeout is unsigned long not int, rather
> than introducing a new variable the wait_for_completion_timeout is moved
> into the if condition as the return value is only used to detect timeout.

But the return value is bound by the timeout parameter and we know that fits 
into a int. And I really dont like moving the function call into the if 
condition, so unless there is some additional explanation why this is 
necessary and should be done I'm inclined to nack this.

>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
>
> Aside from the fixup of the wait_for_completion_timeout handling
> a minor coding style issue in the else branch was fixed - the {} is
> not needed for the one-line body.

If one of the branches of a conditional statement has braces the other 
branches should also have braces. This is documented in 
Documentation/CodingStyle.

  reply	other threads:[~2015-02-02  9:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-02  8:37 [PATCH 3/3] iio: ad_sigma_delta: cleanup wait_for_completion return handling Nicholas Mc Guire
2015-02-02  9:41 ` Lars-Peter Clausen [this message]
2015-02-02  9:50   ` Nicholas Mc Guire
2015-02-02 10:10     ` Lars-Peter Clausen

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=54CF464F.9000306@metafoo.de \
    --to=lars@metafoo.de \
    --cc=Michael.Hennerich@analog.com \
    --cc=hofrat@osadl.org \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    /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