qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Blue Swirl <blauwirbel@gmail.com>,
	qemu-devel@nongnu.org, Jes Sorensen <Jes.Sorensen@redhat.com>
Subject: Re: [Qemu-devel] Re: [PATCH 2/5] CODING_STYLE: add C type rules
Date: Wed, 18 Aug 2010 19:55:14 +0300	[thread overview]
Message-ID: <4C6C1072.1000200@redhat.com> (raw)
In-Reply-To: <4C6BE6E1.9010108@redhat.com>

  On 08/18/2010 04:57 PM, Paolo Bonzini wrote:
> On 08/18/2010 12:30 PM, Kevin Wolf wrote:
>> Am 18.08.2010 10:35, schrieb Paolo Bonzini:
>>> On 08/17/2010 08:39 PM, Richard Henderson wrote:
>>>>>   On 08/12/10 19:50, Blue Swirl wrote:
>>>>>>   +While using "bool" is good for readability, it comes with 
>>>>>> minor caveats:
>>>>>>   + - Don't use "bool" in places where the type size must be 
>>>>>> constant across
>>>>>>   +   all systems, like public interfaces and on-the-wire protocols.
>>>>>>   + - Don't compare a bool variable against the literal, "true",
>>>>>>   +   since a value with a logical non-false value need not be "1".
>>>>>>   +   I.e., don't write "if (seen == true) ...".  Rather, write 
>>>>>> "if (seen)...".
>>>>>
>>>>>   I'd strongly discourage the use of bool in any code.
>>>>
>>>> I strongly disagree.  The use of "bool", even if you ignore stdbool.h
>>>> and do "typedef int bool", is valuable documentation in the code.
>>>
>>> I think "bool" is fine, but it should be either stdbool.h or a typedef.
>>>    Using stdbool.h only when it is present is going to introduce 
>>> bugs the
>>> day someone relies on the magic properties of the C99 bool.
>>
>> We rely on C99 anyway, so stdbool.h should always be present (and in
>> fact, it is used unconditionally today).
>
> Right.  However, this is wrong then:
>
> - Don't compare a bool variable against the literal, "true",
>   since a value with a logical non-false value need not be "1".
>   I.e., don't write "if (seen == true) ...".  Rather, write "if
>   (seen)...".
>
> I mean, I'm all for using "if (seen)" but bool *does* normalize 
> logical non-false values to "1".  I'd remove the second line (making 
> it just a coding style issue) and add something like this:
>
> - Do not rely on the fact that bool normalizes logical non-false values
>   to 1.  So, write "x = true" instead of "x++" and "x = !x" instead of
>   "x--".
> - Similarly, when x is a bool, it may be clearer to avoid
>   "x |= y".  Instead, use either "x = x || y" (if short circuiting
>   is acceptable or even desirable) or "x |= (y != 0)".
>
> Probably a bit too verbose, but you get the idea.

_Bool can have values of 0 or 1, so all of the above should actually work.

_Bool convert_to_bool(long long x)
{
     return x;
}


    0:   48 85 ff                test   %rdi,%rdi
    3:   0f 95 c0                setne  %al
    6:   c3                      retq

void incr(_Bool* b)
{
     ++*b;
}

   10:   c6 07 01                movb   $0x1,(%rdi)
   13:   c3                      retq


If _Bool were int, the first example would fail.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

  reply	other threads:[~2010-08-18 16:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-12 17:50 [Qemu-devel] [PATCH 2/5] CODING_STYLE: add C type rules Blue Swirl
2010-08-13 19:37 ` [Qemu-devel] " Blue Swirl
2010-08-17  8:09 ` [Qemu-devel] " Jes Sorensen
2010-08-17 17:56   ` Blue Swirl
2010-08-17 18:55     ` malc
2010-08-17 19:23       ` Jes Sorensen
2010-08-17 19:24         ` malc
2010-08-17 19:43           ` Jes Sorensen
2010-08-17 20:29             ` Anthony Liguori
2010-08-17 20:33             ` malc
2010-08-17 18:39   ` Richard Henderson
2010-08-17 19:15     ` Jes Sorensen
2010-08-18 16:46       ` Avi Kivity
2010-08-19  7:58         ` Jes Sorensen
2010-08-19  8:10           ` Avi Kivity
2010-08-19  8:17             ` Jes Sorensen
2010-08-19 12:24               ` Avi Kivity
2010-08-19 12:52                 ` malc
2010-08-19 12:59                   ` Avi Kivity
2010-08-18  8:35     ` [Qemu-devel] " Paolo Bonzini
2010-08-18  8:58       ` Jes Sorensen
2010-08-18 10:30       ` Kevin Wolf
2010-08-18 13:57         ` Paolo Bonzini
2010-08-18 16:55           ` Avi Kivity [this message]
2010-08-19  7:51             ` Paolo Bonzini
2010-08-19  8:12               ` Avi Kivity
2010-08-18 16:44   ` [Qemu-devel] " Avi Kivity

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=4C6C1072.1000200@redhat.com \
    --to=avi@redhat.com \
    --cc=Jes.Sorensen@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.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;
as well as URLs for NNTP newsgroup(s).