qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 1/8] CODING_STYLE: Section about conditional


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v3 1/8] CODING_STYLE: Section about conditional statement
Date: Fri, 01 Aug 2014 10:01:28 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0

On 08/01/2014 01:46 AM, address@hidden wrote:
> From: Gonglei <address@hidden>
> 
> Yoda conditions lack readability, and QEMU has a
> strict compiler configuration for checking a common
> mistake like "if (dev = NULL)". Make it a written rule.
> 
> Signed-off-by: Gonglei <address@hidden>
> ---
>  CODING_STYLE | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 4280945..b08bfb4 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -91,3 +91,17 @@ Mixed declarations (interleaving statements and 
> declarations within blocks)
>  are not allowed; declarations should be at the beginning of blocks.  In other
>  words, the code should not generate warnings if using GCC's
>  -Wdeclaration-after-statement option.
> +
> +6. Conditional statements
> +
> +When comparing a variable for (in)equality with a constant, list the
> +constant on the right, as in:
> +
> +if (a == 0) {
> +    /* Reads like: "If a is equal to 0" */

I actually tend to read it as 'if a equals 0'.

> +    do_something();
> +}
> +
> +Rationale: Yoda conditions (as in 'if (0 == a)') are awkward to read.

I know this is my suggested text, but now that I'm re-reading it, I'd
recommend s/0/1/ in all three places, since comparison to 0 is one of
those special cases where '!a' is faster to write than 'a == 0'.

> +Besides, good compilers already warn users when '==' is mis-typed as '=',
> +even when the constant is on the right.

With those changes,
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]