[Qemu-devel] [7234] Use a more natural order

[Qemu-devel] [7234] Use a more natural order

[Blue Swirl]

Revision: 7234
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
Author:   blueswir1
Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
Log Message:
-----------
Use a more natural order

Modified Paths:
--------------
    trunk/hw/xen_console.c
    trunk/hw/xen_disk.c
    trunk/hw/xen_domainbuild.c

Modified: trunk/hw/xen_console.c
===================================================================
--- trunk/hw/xen_console.c	2009-04-23 13:16:56 UTC (rev 7233)
+++ trunk/hw/xen_console.c	2009-04-23 18:29:47 UTC (rev 7234)
@@ -189,7 +189,7 @@
     free(dom);
 
     type = xenstore_read_str(con->console, "type");
-    if (!type || 0 != strcmp(type, "ioemu")) {
+    if (!type || strcmp(type, "ioemu" != 0)) {
 	xen_be_printf(xendev, 1, "not for me (type=%s)\n", type);
 	return -1;
     }

Modified: trunk/hw/xen_disk.c
===================================================================
--- trunk/hw/xen_disk.c	2009-04-23 13:16:56 UTC (rev 7233)
+++ trunk/hw/xen_disk.c	2009-04-23 18:29:47 UTC (rev 7234)
@@ -179,7 +179,7 @@
     switch (ioreq->req.operation) {
     case BLKIF_OP_READ:
 	ioreq->prot = PROT_WRITE; /* to memory */
-	if (BLKIF_OP_READ != ioreq->req.operation && blkdev->mode[0] != 'w') {
+        if (ioreq->req.operation != BLKIF_OP_READ && blkdev->mode[0] != 'w') {
 	    xen_be_printf(&blkdev->xendev, 0, "error: write req for ro device\n");
 	    goto err;
 	}
@@ -513,7 +513,7 @@
 
     if (use_aio)
         blk_send_response_all(blkdev);
-    while ((rc != rp)) {
+    while (rc != rp) {
         /* pull request from ring */
         if (RING_REQUEST_CONS_OVERFLOW(&blkdev->rings.common, rc))
             break;

Modified: trunk/hw/xen_domainbuild.c
===================================================================
--- trunk/hw/xen_domainbuild.c	2009-04-23 13:16:56 UTC (rev 7233)
+++ trunk/hw/xen_domainbuild.c	2009-04-23 18:29:47 UTC (rev 7234)
@@ -137,7 +137,7 @@
     int rc;
 
     rc = xc_domain_getinfo(xen_xc, xen_domid, 1, &info);
-    if ((1 != rc) || (info.domid != xen_domid)) {
+    if ((rc != 1) || (info.domid != xen_domid)) {
         qemu_log("xen: domain %d is gone\n", xen_domid);
         goto quit;
     }
@@ -186,7 +186,7 @@
         rc = read(fd[0], &byte, 1);
         switch (rc) {
         case -1:
-            if (EINTR == errno)
+            if (errno == EINTR)
                 continue;
             qemu_log("%s: Huh? read error: %s\n", __FUNCTION__, strerror(errno));
             qemu_running = 0;

Re: [Qemu-devel] [7234] Use a more natural order

[Andreas Färber]

Am 23.04.2009 um 20:29 schrieb Blue Swirl:

> Revision: 7234
>          http://svn.sv.gnu.org/viewvc/? 
> view=rev&root=qemu&revision=7234
> Author:   blueswir1
> Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
> Log Message:
> -----------
> Use a more natural order
>
> Modified Paths:
> --------------
>    trunk/hw/xen_console.c
>    trunk/hw/xen_disk.c
>    trunk/hw/xen_domainbuild.c
>
> Modified: trunk/hw/xen_console.c
> ===================================================================
> --- trunk/hw/xen_console.c	2009-04-23 13:16:56 UTC (rev 7233)
> +++ trunk/hw/xen_console.c	2009-04-23 18:29:47 UTC (rev 7234)
> @@ -189,7 +189,7 @@
>     free(dom);
>
>     type = xenstore_read_str(con->console, "type");
> -    if (!type || 0 != strcmp(type, "ioemu")) {
> +    if (!type || strcmp(type, "ioemu" != 0)) {

Wrong bracket position.

>
> 	xen_be_printf(xendev, 1, "not for me (type=%s)\n", type);
> 	return -1;
>     }
>
> Modified: trunk/hw/xen_disk.c
> ===================================================================
> --- trunk/hw/xen_disk.c	2009-04-23 13:16:56 UTC (rev 7233)
> +++ trunk/hw/xen_disk.c	2009-04-23 18:29:47 UTC (rev 7234)
> @@ -179,7 +179,7 @@
>     switch (ioreq->req.operation) {
>     case BLKIF_OP_READ:
> 	ioreq->prot = PROT_WRITE; /* to memory */
> -	if (BLKIF_OP_READ != ioreq->req.operation && blkdev->mode[0] !=  
> 'w') {
> +        if (ioreq->req.operation != BLKIF_OP_READ && blkdev- 
> >mode[0] != 'w') {
> 	    xen_be_printf(&blkdev->xendev, 0, "error: write req for ro  
> device\n");
> 	    goto err;
> 	}
> @@ -513,7 +513,7 @@
>
>     if (use_aio)
>         blk_send_response_all(blkdev);
> -    while ((rc != rp)) {
> +    while (rc != rp) {
>         /* pull request from ring */
>         if (RING_REQUEST_CONS_OVERFLOW(&blkdev->rings.common, rc))
>             break;
>
> Modified: trunk/hw/xen_domainbuild.c
> ===================================================================
> --- trunk/hw/xen_domainbuild.c	2009-04-23 13:16:56 UTC (rev 7233)
> +++ trunk/hw/xen_domainbuild.c	2009-04-23 18:29:47 UTC (rev 7234)
> @@ -137,7 +137,7 @@
>     int rc;
>
>     rc = xc_domain_getinfo(xen_xc, xen_domid, 1, &info);
> -    if ((1 != rc) || (info.domid != xen_domid)) {
> +    if ((rc != 1) || (info.domid != xen_domid)) {
>         qemu_log("xen: domain %d is gone\n", xen_domid);
>         goto quit;
>     }
> @@ -186,7 +186,7 @@
>         rc = read(fd[0], &byte, 1);
>         switch (rc) {
>         case -1:
> -            if (EINTR == errno)
> +            if (errno == EINTR)
>                 continue;
>             qemu_log("%s: Huh? read error: %s\n", __FUNCTION__,  
> strerror(errno));
>             qemu_running = 0;
>
>
>

Re: [Qemu-devel] [7234] Use a more natural order

[Blue Swirl]

On 4/23/09, Andreas Färber <andreas.faerber@web.de> wrote:
>
>  Am 23.04.2009 um 20:29 schrieb Blue Swirl:
>
>
> > Revision: 7234
> >
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
> > Author:   blueswir1
> > Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
> > Log Message:
> > -----------
> > Use a more natural order
> >
> > Modified Paths:
> > --------------
> >   trunk/hw/xen_console.c
> >   trunk/hw/xen_disk.c
> >   trunk/hw/xen_domainbuild.c
> >
> > Modified: trunk/hw/xen_console.c
> >
> ===================================================================
> > --- trunk/hw/xen_console.c      2009-04-23 13:16:56 UTC (rev 7233)
> > +++ trunk/hw/xen_console.c      2009-04-23 18:29:47 UTC (rev 7234)
> > @@ -189,7 +189,7 @@
> >    free(dom);
> >
> >    type = xenstore_read_str(con->console, "type");
> > -    if (!type || 0 != strcmp(type, "ioemu")) {
> > +    if (!type || strcmp(type, "ioemu" != 0)) {
> >
>
>  Wrong bracket position.

Thanks, fixed.

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 06:29:47PM +0000, Blue Swirl wrote:
> Revision: 7234
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
> Author:   blueswir1
> Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
> Log Message:
> -----------
> Use a more natural order

It may be more natural, but it is also less safe.

After all

if (0 = x) {

fails compile, while

if (x = 0) {

compiles silently even when you didn't mean that.

Some people also think month/day/year is more natural as a date format,
but it is confusing and impractical to actually use.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Blue Swirl]

On 4/23/09, Lennart Sorensen <lsorense@csclub.uwaterloo.ca> wrote:
> On Thu, Apr 23, 2009 at 06:29:47PM +0000, Blue Swirl wrote:
>
> > Revision: 7234
>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
>  > Author:   blueswir1
>  > Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
>  > Log Message:
>  > -----------
>  > Use a more natural order
>
>
> It may be more natural, but it is also less safe.
>
>  After all
>
>  if (0 = x) {
>
>  fails compile, while
>
>  if (x = 0) {
>
>  compiles silently even when you didn't mean that.

True, but it's not the style that is used here. You are of course free
to argue for using this version and even submit patches.

>  Some people also think month/day/year is more natural as a date format,
>  but it is confusing and impractical to actually use.

We humans also tend to use base 10 arithmetic and infix notation
despite their well known shortcomings.

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 10:01:43PM +0300, Blue Swirl wrote:
> True, but it's not the style that is used here. You are of course free
> to argue for using this version and even submit patches.

I didn't check if it is covered by the codestyle or not for qemu.

If your patch comment had said "Fix to follow code style" rather than
"make natural order" I wouldn't have said anything.  Natural order in
this case is a bad reason for the change.  Following code style is a
good reason (even if the code style may then be questionable).

If the code style doesn't cover it at all, I would say the change is
horrible and shouldn't be done at all.  The change even caused a compile
failure briefly (and hence breaks git bisect) by being commited completely
untested I suspect.  If it didn't cause a compile failure it at least
should have caused a serious warning.

> We humans also tend to use base 10 arithmetic and infix notation
> despite their well known shortcomings.

Well only a small part of humans still use broken date formats.  We do
still seem to be atracted to base 10 though.  And of course our time
measuring system is just weird.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[M. Warner Losh]

In message: <20090423185308.GH3795@csclub.uwaterloo.ca>
            lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes:
: On Thu, Apr 23, 2009 at 06:29:47PM +0000, Blue Swirl wrote:
: > Revision: 7234
: >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
: > Author:   blueswir1
: > Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
: > Log Message:
: > -----------
: > Use a more natural order
: 
: It may be more natural, but it is also less safe.
: 
: After all
: 
: if (0 = x) {
: 
: fails compile, while
: 
: if (x = 0) {
: 
: compiles silently even when you didn't mean that.

This style is evil and must die.  I don't know any nice way to put
it.  It encourages sloppiness.  Also, it breaks down when you add
inequality:

if (x < 1)

becomes

if (1 >= x)

which is also error prone.

The compiler will warn about your example, but won't warn if I
transcribe things wrongly as

if (1 < x)

: Some people also think month/day/year is more natural as a date format,
: but it is confusing and impractical to actually use.

True, but irrelevant.

Warner

Re: [Qemu-devel] [7234] Use a more natural order

[Glauber Costa]

On Thu, Apr 23, 2009 at 4:10 PM, Lennart Sorensen
<lsorense@csclub.uwaterloo.ca> wrote:
> On Thu, Apr 23, 2009 at 10:01:43PM +0300, Blue Swirl wrote:
>> True, but it's not the style that is used here. You are of course free
>> to argue for using this version and even submit patches.
>
> I didn't check if it is covered by the codestyle or not for qemu.
>
> If your patch comment had said "Fix to follow code style" rather than
> "make natural order" I wouldn't have said anything.  Natural order in
> this case is a bad reason for the change.  Following code style is a
> good reason (even if the code style may then be questionable).
>
> If the code style doesn't cover it at all, I would say the change is
> horrible and shouldn't be done at all.  The change even caused a compile
> failure briefly (and hence breaks git bisect) by being commited completely
> untested I suspect.  If it didn't cause a compile failure it at least
> should have caused a serious warning.

Note that this could be avoided by not commiting the code in the first place,
but rather, sending it to the mailing list. Everybody should be sending code
to the mailing list, even maintainers.

Anthony is already doing that, and it greatly reduces the probability of getting
a broken bisect for broken patches.

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 01:12:50PM -0600, M. Warner Losh wrote:
> In message: <20090423185308.GH3795@csclub.uwaterloo.ca>
>             lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes:
> : On Thu, Apr 23, 2009 at 06:29:47PM +0000, Blue Swirl wrote:
> : > Revision: 7234
> : >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
> : > Author:   blueswir1
> : > Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
> : > Log Message:
> : > -----------
> : > Use a more natural order
> : 
> : It may be more natural, but it is also less safe.
> : 
> : After all
> : 
> : if (0 = x) {
> : 
> : fails compile, while
> : 
> : if (x = 0) {
> : 
> : compiles silently even when you didn't mean that.
> 
> This style is evil and must die.  I don't know any nice way to put
> it.  It encourages sloppiness.  Also, it breaks down when you add
> inequality:
> 
> if (x < 1)
> 
> becomes
> 
> if (1 >= x)

No it doesn't.  It becomes:

if (1 > x)

Why would it be anything else?

> which is also error prone.
> 
> The compiler will warn about your example, but won't warn if I
> transcribe things wrongly as
> 
> if (1 < x)

Nothing wrong with that.  That's perfectly valid, if you want to check
that x is greater than 1.

Putting constants first means that you can't accidentally use assignment
when you meant equality.  You can't fix all the stupidies possible in C,
but you can at least try to avoid some of them when possible.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Blue Swirl]

On 4/23/09, Lennart Sorensen <lsorense@csclub.uwaterloo.ca> wrote:
> On Thu, Apr 23, 2009 at 10:01:43PM +0300, Blue Swirl wrote:
>  > True, but it's not the style that is used here. You are of course free
>  > to argue for using this version and even submit patches.
>
>
> I didn't check if it is covered by the codestyle or not for qemu.
>
>  If your patch comment had said "Fix to follow code style" rather than
>  "make natural order" I wouldn't have said anything.  Natural order in
>  this case is a bad reason for the change.  Following code style is a
>  good reason (even if the code style may then be questionable).
>
>  If the code style doesn't cover it at all, I would say the change is
>  horrible and shouldn't be done at all.  The change even caused a compile
>  failure briefly (and hence breaks git bisect) by being commited completely
>  untested I suspect.  If it didn't cause a compile failure it at least
>  should have caused a serious warning.

I don't think any code style document can cover all possible cases.
But another approach can be used: you could try to find a precedent
case where this style has been used in QEMU.

>  > We humans also tend to use base 10 arithmetic and infix notation
>  > despite their well known shortcomings.
>
>
> Well only a small part of humans still use broken date formats.  We do
>  still seem to be atracted to base 10 though.  And of course our time
>  measuring system is just weird.

The formats and systems with varying level of brokenness reflect the
centuries of weird history behind them. Only from a purely engineering
standpoint that is not a valid reason for still using them.

[Qemu-devel] Re: [7234] Use a more natural order

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

Lennart Sorensen wrote:
> On Thu, Apr 23, 2009 at 06:29:47PM +0000, Blue Swirl wrote:
>> Revision: 7234
>>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
>> Author:   blueswir1
>> Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
>> Log Message:
>> -----------
>> Use a more natural order
> 
> It may be more natural, but it is also less safe.
> 
> After all
> 
> if (0 = x) {
> 
> fails compile, while
> 
> if (x = 0) {
> 
> compiles silently even when you didn't mean that.

Today gcc will warn you as well. Of course this means you have to look
at warnings.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [7234] Use a more natural order

[Blue Swirl]

On 4/23/09, Glauber Costa <glommer@gmail.com> wrote:
> On Thu, Apr 23, 2009 at 4:10 PM, Lennart Sorensen
>
> <lsorense@csclub.uwaterloo.ca> wrote:
>
> > On Thu, Apr 23, 2009 at 10:01:43PM +0300, Blue Swirl wrote:
>  >> True, but it's not the style that is used here. You are of course free
>  >> to argue for using this version and even submit patches.
>  >
>  > I didn't check if it is covered by the codestyle or not for qemu.
>  >
>  > If your patch comment had said "Fix to follow code style" rather than
>  > "make natural order" I wouldn't have said anything.  Natural order in
>  > this case is a bad reason for the change.  Following code style is a
>  > good reason (even if the code style may then be questionable).
>  >
>  > If the code style doesn't cover it at all, I would say the change is
>  > horrible and shouldn't be done at all.  The change even caused a compile
>  > failure briefly (and hence breaks git bisect) by being commited completely
>  > untested I suspect.  If it didn't cause a compile failure it at least
>  > should have caused a serious warning.
>
>
> Note that this could be avoided by not commiting the code in the first place,
>  but rather, sending it to the mailing list. Everybody should be sending code
>  to the mailing list, even maintainers.
>
>  Anthony is already doing that, and it greatly reduces the probability of getting
>  a broken bisect for broken patches.

In this case he committed the patches even though I had pointed out
these few remaining instances of this style. Gerd had fixed most of
the issues on previous rounds but some were still left.

Re: [Qemu-devel] [7234] Use a more natural order

[M. Warner Losh]

In message: <20090423192844.GJ3795@csclub.uwaterloo.ca>
            lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes:
: On Thu, Apr 23, 2009 at 01:12:50PM -0600, M. Warner Losh wrote:
: > In message: <20090423185308.GH3795@csclub.uwaterloo.ca>
: >             lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes:
: > : On Thu, Apr 23, 2009 at 06:29:47PM +0000, Blue Swirl wrote:
: > : > Revision: 7234
: > : >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7234
: > : > Author:   blueswir1
: > : > Date:     2009-04-23 18:29:47 +0000 (Thu, 23 Apr 2009)
: > : > Log Message:
: > : > -----------
: > : > Use a more natural order
: > : 
: > : It may be more natural, but it is also less safe.
: > : 
: > : After all
: > : 
: > : if (0 = x) {
: > : 
: > : fails compile, while
: > : 
: > : if (x = 0) {
: > : 
: > : compiles silently even when you didn't mean that.
: > 
: > This style is evil and must die.  I don't know any nice way to put
: > it.  It encourages sloppiness.  Also, it breaks down when you add
: > inequality:
: > 
: > if (x < 1)
: > 
: > becomes
: > 
: > if (1 >= x)
: 
: No it doesn't.  It becomes:
: 
: if (1 > x)
: 
: Why would it be anything else?

Hmmm, see how tricky this style is?  It is confusing computing the
contrapositive to the expression you want to express.  Or rather you
aren't computing the contrapositive here, which is what got me into
trouble.  I usually don't make mistakes like this, and I made it in
coming up with the example.

: > which is also error prone.
: > 
: > The compiler will warn about your example, but won't warn if I
: > transcribe things wrongly as
: > 
: > if (1 < x)
: 
: Nothing wrong with that.  That's perfectly valid, if you want to check
: that x is greater than 1.

Correct.  The compiler doesn't warn you that you've gotten your
backwards flipping around wrong.  Which is the argument for this style
when it comes to equality.  So you've traded one class of problems for
another.  And this class of problem is just as hard to find.

I've fixed several bugs like this over the years from coders that
thought this was a good way to program.

if (1 < x)

rather than 

if (x < 1)

is the most common pattern I've had to fix.

: Putting constants first means that you can't accidentally use assignment
: when you meant equality.  You can't fix all the stupidies possible in C,
: but you can at least try to avoid some of them when possible.

I find this argument unpersuasive when the compiler will already warn
me about if (x = 0).

Warner

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 10:31:05PM +0300, Blue Swirl wrote:
> I don't think any code style document can cover all possible cases.
> But another approach can be used: you could try to find a precedent
> case where this style has been used in QEMU.

# grep -r '([0-9] =' .
./net.c:    if (0 == errno && '\0' == *last_char &&
./hw/gus.c:    if (0 == ((mode >> 4) & 1)) {
./hw/dma.c:            if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) {
./hw/sb16.c:        if (0 == s->needed_bytes) {
./hw/sb16.c:            if (0 == s->needed_bytes) {
./hw/sb16.c:        if (0 == s->dma_auto) {
./hw/sb16.c:        if (0 == s->dma_auto) {

That was just one quick search.  Looks like whoever wrote a bunch of
the audio hardware emulation liked less buggy code.

> The formats and systems with varying level of brokenness reflect the
> centuries of weird history behind them. Only from a purely engineering
> standpoint that is not a valid reason for still using them.

Well to me software development is a kind of engineering and hence using
anything but the safest practice that is at all practical makes no sense.

That means:
Constants before variables in all comparisons.
Braces are never optional for blocks.

The second one is especially hard to get some people to understand.

-- 
Len Sorensen

Re: [Qemu-devel] Re: [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 09:37:05PM +0200, Jan Kiszka wrote:
> Lennart Sorensen wrote:
> > It may be more natural, but it is also less safe.
> > 
> > After all
> > 
> > if (0 = x) {
> > 
> > fails compile, while
> > 
> > if (x = 0) {
> > 
> > compiles silently even when you didn't mean that.
> 
> Today gcc will warn you as well. Of course this means you have to look
> at warnings.

True.  You can tell it off by doing if ((x = 0)) it seems.

Always safer to treat warnings as errors it seems.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 01:41:36PM -0600, M. Warner Losh wrote:
> Hmmm, see how tricky this style is?  It is confusing computing the
> contrapositive to the expression you want to express.  Or rather you
> aren't computing the contrapositive here, which is what got me into
> trouble.  I usually don't make mistakes like this, and I made it in
> coming up with the example.

I wouldn't be surprised if there were languages other than english where
the reverse would be the natural order, so I don't think your argument
is worth much there.

> Correct.  The compiler doesn't warn you that you've gotten your
> backwards flipping around wrong.  Which is the argument for this style
> when it comes to equality.  So you've traded one class of problems for
> another.  And this class of problem is just as hard to find.

The compiler can't guess what your logic should be.  It can tell that
you tried to assign a value to a constant.

> I've fixed several bugs like this over the years from coders that
> thought this was a good way to program.
> 
> if (1 < x)
> 
> rather than 
> 
> if (x < 1)
> 
> is the most common pattern I've had to fix.

Then you were fixing the wrong problem.  The problem isn't the order,
but simply that sometimes people get their logic backwards.  The correct
fix would have been:

if (1 > x)

if in fact the logic was backwards.

> I find this argument unpersuasive when the compiler will already warn
> me about if (x = 0).

Some compilers warn you.  Not all do.  An error from all compilers is
also far better than a warning from some compilers.

This is why people writing safety critical code in C require this order.
It is simply the safest choice.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Anthony Liguori]

Glauber Costa wrote:
> On Thu, Apr 23, 2009 at 4:10 PM, Lennart Sorensen
> <lsorense@csclub.uwaterloo.ca> wrote:
>   
> Anthony is already doing that, and it greatly reduces the probability of getting
> a broken bisect for broken patches.
>   

In all fairness, I usually only do that when it's something I think 
people would comment on.  I probably would do a fix up like this by just 
committing.

People miss things, it happens.  No process is going to prevent it 
happening 100% of the time.

Regards,

Anthony Liguori

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 02:57:26PM -0500, Anthony Liguori wrote:
> In all fairness, I usually only do that when it's something I think  
> people would comment on.  I probably would do a fix up like this by just  
> committing.
>
> People miss things, it happens.  No process is going to prevent it  
> happening 100% of the time.

If you can't bother to compile test it, you shouldn't be commiting.
Your time isn't worth that much more than everyone else that gets
inconvinienced by the sloppy commit.

That's a procedure that can certainly help.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Anthony Liguori]

Blue Swirl wrote:
> On 4/23/09, Glauber Costa <glommer@gmail.com> wrote:
>   
> In this case he committed the patches even though I had pointed out
> these few remaining instances of this style. Gerd had fixed most of
> the issues on previous rounds but some were still left.
>   

With the expectation that they were going to get fixed after commit.  
It's very hard to maintain large patch series outside of the tree.  
Since they tend to sit on the list, merging often requires fix ups.  In 
svn, this is a total pain.

Regards,

Anthony Liguori

Re: [Qemu-devel] [7234] Use a more natural order

[Anthony Liguori]

Lennart Sorensen wrote:
> On Thu, Apr 23, 2009 at 02:57:26PM -0500, Anthony Liguori wrote:
>   
>> In all fairness, I usually only do that when it's something I think  
>> people would comment on.  I probably would do a fix up like this by just  
>> committing.
>>
>> People miss things, it happens.  No process is going to prevent it  
>> happening 100% of the time.
>>     
>
> If you can't bother to compile test it, you shouldn't be commiting.
> Your time isn't worth that much more than everyone else that gets
> inconvinienced by the sloppy commit.
>
> That's a procedure that can certainly help.
>   

I was obviously referring to posting to the mailing list.  I have a set 
of compilation and unit tests that get run before every commit.

There are a lot of possible configuration options for QEMU.  Sometimes 
the build breaks for some combination of configuration options and this 
is a difficult thing to address in a robust way.  With a patch series, 
my scripts run through what I consider the most common set of 
configurations for each patch.  Right now, it usually takes a good 30 
minutes to go through a large series just to compile test with my 
default configurations.

Regards,

Anthony Liguori

Re: [Qemu-devel] [7234] Use a more natural order

[M. Warner Losh]

In message: <20090423195553.GM3795@csclub.uwaterloo.ca>
            lsorense@csclub.uwaterloo.ca (Lennart Sorensen) writes:
: > I've fixed several bugs like this over the years from coders that
: > thought this was a good way to program.
: > 
: > if (1 < x)
: > 
: > rather than 
: > 
: > if (x < 1)
: > 
: > is the most common pattern I've had to fix.
: 
: Then you were fixing the wrong problem.  The problem isn't the order,
: but simply that sometimes people get their logic backwards.  The correct
: fix would have been:
: 
: if (1 > x)
: 
: if in fact the logic was backwards.

No.  My fix was the correct one. :)

: > I find this argument unpersuasive when the compiler will already warn
: > me about if (x = 0).
: 
: Some compilers warn you.  Not all do.  An error from all compilers is
: also far better than a warning from some compilers.

In the absence of other side effects, yes.

: This is why people writing safety critical code in C require this order.
: It is simply the safest choice.

It isn't the safest order.  It is safer for one class of expressions,
more dangerous for the others.

I've also never seen that requirement in any of the works that I've
done, which has had safety implications...

Warner

Re: [Qemu-devel] [7234] Use a more natural order

[Blue Swirl]

On 4/23/09, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Blue Swirl wrote:
>
> > On 4/23/09, Glauber Costa <glommer@gmail.com> wrote:
> >  In this case he committed the patches even though I had pointed out
> > these few remaining instances of this style. Gerd had fixed most of
> > the issues on previous rounds but some were still left.
> >
> >
>
>  With the expectation that they were going to get fixed after commit.  It's
> very hard to maintain large patch series outside of the tree.  Since they
> tend to sit on the list, merging often requires fix ups.  In svn, this is a
> total pain.

Also in my view the remaining issues were not blocking the commit or
worth a new round of patches, I was referring to Glauber's "all
patches to list" proposal.

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 03:03:54PM -0500, Anthony Liguori wrote:
> I was obviously referring to posting to the mailing list.  I have a set  
> of compilation and unit tests that get run before every commit.
>
> There are a lot of possible configuration options for QEMU.  Sometimes  
> the build breaks for some combination of configuration options and this  
> is a difficult thing to address in a robust way.  With a patch series,  
> my scripts run through what I consider the most common set of  
> configurations for each patch.  Right now, it usually takes a good 30  
> minutes to go through a large series just to compile test with my  
> default configurations.

Isn't there a configure option for building for all available targets?

It might take 30 minutes, but it saves large amounts of time for everyone
else later.  Also I would hope make is smart enough to only recompile
the bits that changed, so that shouldn't take as long.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 02:07:35PM -0600, M. Warner Losh wrote:
> No.  My fix was the correct one. :)

Well certainly not.  You changed 3 characters where 1 character would
have solved the problem, and not messed with the intent of the original
programmer for no good reason.

> In the absence of other side effects, yes.

There are no side effects, hence it is better.

> It isn't the safest order.  It is safer for one class of expressions,
> more dangerous for the others.

It is safer for the one that is generally much worse to get wrong
(assignment versus comparison).  It is arguable less safe for some people
who can only thing in words rather than logic for greater than and less
than comparisons.

> I've also never seen that requirement in any of the works that I've
> done, which has had safety implications...

Well there are certainly some places that shouldn't be doing such things.

Of course one could also argue that C shouldn't be permitted for such
type of work.  It simply permits too much sloppy code.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Anthony Liguori]

Lennart Sorensen wrote:
> On Thu, Apr 23, 2009 at 03:03:54PM -0500, Anthony Liguori wrote:
>   
>> I was obviously referring to posting to the mailing list.  I have a set  
>> of compilation and unit tests that get run before every commit.
>>
>> There are a lot of possible configuration options for QEMU.  Sometimes  
>> the build breaks for some combination of configuration options and this  
>> is a difficult thing to address in a robust way.  With a patch series,  
>> my scripts run through what I consider the most common set of  
>> configurations for each patch.  Right now, it usually takes a good 30  
>> minutes to go through a large series just to compile test with my  
>> default configurations.
>>     
>
> Isn't there a configure option for building for all available targets?
>   

Building all targets is easy.  It's the various combinations of other 
options that get you.  It's building for Linux, win32, x86, PPC, with 
AIO, without AIO, with KVM, without KVM, etc.

> It might take 30 minutes, but it saves large amounts of time for everyone
> else later.  Also I would hope make is smart enough to only recompile
> the bits that changed, so that shouldn't take as long.
>   

A great deal of our objects are target specific.  Almost everything 
depends on CONFIG_USER_ONLY too.  This means that full compilation takes 
a very long time.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [7234] Use a more natural order

[malc]

On Thu, 23 Apr 2009, Lennart Sorensen wrote:

> On Thu, Apr 23, 2009 at 09:37:05PM +0200, Jan Kiszka wrote:
> > Lennart Sorensen wrote:
> > > It may be more natural, but it is also less safe.
> > > 
> > > After all
> > > 
> > > if (0 = x) {
> > > 
> > > fails compile, while
> > > 
> > > if (x = 0) {
> > > 
> > > compiles silently even when you didn't mean that.
> > 
> > Today gcc will warn you as well. Of course this means you have to look
> > at warnings.
> 
> True.  You can tell it off by doing if ((x = 0)) it seems.

Bings back warm and fuzzy memories of: http://kerneltrap.org/node/1584

> 
> Always safer to treat warnings as errors it seems.
> 
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] Re: [7234] Use a more natural order

[Jamie Lokier]

Lennart Sorensen wrote:
> > > if (x = 0) {
> > > 
> > > compiles silently even when you didn't mean that.
> > 
> > Today gcc will warn you as well. Of course this means you have to look
> > at warnings.
> 
> True.  You can tell it off by doing if ((x = 0)) it seems.
> 
> Always safer to treat warnings as errors it seems.

If you want, you can force specific warnings to become errors in current GCC.

-- Jamie

Re: [Qemu-devel] [7234] Use a more natural order

[Jamie Lokier]

Anthony Liguori wrote:
> >It might take 30 minutes, but it saves large amounts of time for everyone
> >else later.  Also I would hope make is smart enough to only recompile
> >the bits that changed, so that shouldn't take as long.
> 
> A great deal of our objects are target specific.  Almost everything 
> depends on CONFIG_USER_ONLY too.  This means that full compilation takes 
> a very long time.

You might find ccache helps.  For this kind of testing, nearly every
compile should be a cache hit, so the slow part is limited running
configure, make and preprocessing, and the final link.

Just remember to clear out the cache when upgrading GCC :-)

-- Jamie

Re: [Qemu-devel] [7234] Use a more natural order

[Jamie Lokier]

Lennart Sorensen wrote:
> On Thu, Apr 23, 2009 at 10:31:05PM +0300, Blue Swirl wrote:
> > I don't think any code style document can cover all possible cases.
> > But another approach can be used: you could try to find a precedent
> > case where this style has been used in QEMU.
> 
> # grep -r '([0-9] =' .
> ./net.c:    if (0 == errno && '\0' == *last_char &&
> ./hw/gus.c:    if (0 == ((mode >> 4) & 1)) {
> ./hw/dma.c:            if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) {
> ./hw/sb16.c:        if (0 == s->needed_bytes) {
> ./hw/sb16.c:            if (0 == s->needed_bytes) {
> ./hw/sb16.c:        if (0 == s->dma_auto) {
> ./hw/sb16.c:        if (0 == s->dma_auto) {
> 
> That was just one quick search.  Looks like whoever wrote a bunch of
> the audio hardware emulation liked less buggy code.

No, it means they believe that way around makes them less likely to
write bugs.  They may be right for themselves.  It doesn't mean the
code is less buggy, and it doesn't mean someone else following the
same style would write less buggy code because of the style.

Same for the redundant parentheses around every comparison next to &&.
Really, there is no need: if (condition && condition) is so common
that there's no need to clarify the grouping.  Unlike say "x & 1 << 2"
where it's a good idea to add parentheses.

> Well to me software development is a kind of engineering and hence using
> anything but the safest practice that is at all practical makes no sense.

But what you call safest isn't safest for everyone.  A lot depends on
what people are familiar with and trained to spot deviations from.

> That means:
> Constants before variables in all comparisons.
> Braces are never optional for blocks.

I disagree with both, but especially the first.  You call it safe
practice.  I call it likely to cause more bugs because it's unfamiliar
compared with common practice.  And because it's the opposite to the
way we say it mathematically or in English.

It's opposite to the "Object Verb Subject" grammar that English
speakers think in: asking "is zero equal to x?"  is really weird
English, because that is asking a question _about zero_, which is not
right: zero is not meant to be something about which there is any question.
It's a constant!

That cognitive dissonance adds a translation step between thinking (in
English) and reading/writing, which means more chance of not seeing
other bugs - limited brainpower and all.

"if (0 < x)" is even worse: it's begging to be accidentally written
with ">".  More importantly, if it _is_ accidentally written with ">",
the mistake is less likely to be noticed than if it were written the
other way around.  But if you write "if (0 == x)" and "if (x > 0)"
just because of "=" C syntax, that's an inconsistent mess.

Do you also write things like "for (x = 0; NUMBER > x; x++)"?  No, of
course not.

Sure, it was ok practice back in the days when compilers would let you
write "if (x = 0)", but those days are long gone.  Now all compilers
warn, and you can ask them to make it an error.  A compiler check is
much safer than depending on writing style, especially a weird one
that's inconsistent with other writing in the same code, and
inconsistent with English thinking.

> The second one is especially hard to get some people to understand.

Accidentally mistaking the scope of a block is unlikely using proper
tools.  We're not using NOTEPAD.EXE any more.  Editors can indent code
automatically these days (last 20 years or so).  Again, using tools is
better than depending on writing style because it catches more mistakes.

Actually I'm sympathetic to people who prefer braces around every
block, and I happily follow such a style when requested.  It doesn't
look too painful or weird, unlike the "unnatural" comparison order. :-)
But I don't think it's necessary on projects where everyone uses other
equally good practices instead to catch the same errors.

-- Jamie

Re: [Qemu-devel] [7234] Use a more natural order

[Jamie Lokier]

Lennart Sorensen wrote:
> > if (x < 1)
> > 
> > becomes
> > 
> > if (1 >= x)
> 
> No it doesn't.  It becomes:
> 
> if (1 > x)

Exactly.

You just proved that it's error prone to someone unfamiliar with it.

We all see the mistake now.  But it was too easy to make in the first
place.  Maybe you never do that, but you have to choose practices
which don't trigger mistakes from others too.

> > The compiler will warn about your example, but won't warn if I
> > transcribe things wrongly as
> > 
> > if (1 < x)
> 
> Nothing wrong with that.  That's perfectly valid, if you want to check
> that x is greater than 1.

By transcribing he meant that one is prone to writing "if (1 < x)" _by
accident_ when one means to check that x is less than 1.  Even worse,
the mistake is prone to being missed when reading it too.

> Putting constants first means that you can't accidentally use assignment
> when you meant equality.  You can't fix all the stupidies possible in C,
> but you can at least try to avoid some of them when possible.

You can't do that anyway, without a warning or error, with any
compiler released in the last 10 years.  Unless you're so stupid as to
turn that warning off.  Now that _would_ be a stupidity possible in C :-)

-- Jame

Re: [Qemu-devel] [7234] Use a more natural order

[Jamie Lokier]

Lennart Sorensen wrote:
> On Thu, Apr 23, 2009 at 01:41:36PM -0600, M. Warner Losh wrote:
> > Hmmm, see how tricky this style is?  It is confusing computing the
> > contrapositive to the expression you want to express.  Or rather you
> > aren't computing the contrapositive here, which is what got me into
> > trouble.  I usually don't make mistakes like this, and I made it in
> > coming up with the example.
> 
> I wouldn't be surprised if there were languages other than english where
> the reverse would be the natural order, so I don't think your argument
> is worth much there.

I wouldn't be surprised either.  But C is based on English, and most
programmers probably use a lot of English thinking.

Anyway, QEMU is definitely oriented around English as its main human
language :-)

> > I've fixed several bugs like this over the years from coders that
> > thought this was a good way to program.
> > 
> > if (1 < x)
> > 
> > rather than 
> > 
> > if (x < 1)
> > 
> > is the most common pattern I've had to fix.
> 
> Then you were fixing the wrong problem.  The problem isn't the order,
> but simply that sometimes people get their logic backwards.  The correct
> fix would have been:
> 
> if (1 > x)
> 
> if in fact the logic was backwards.

You missed the point entirely.  *whoosh*

The point is that when

    if (1 < x)

where "1 > x" was would have been correct, is a _more likely mistake
for most people to make_ than

    if (x < 1)

because the latter is more familiar to most people.  There is less
cognitive translation getting in the way of seeing what it means and
writing fluently.

The actual bug fix is irrelevant.  The only thing relevant is which
style causes more mistakes.

> > I find this argument unpersuasive when the compiler will already warn
> > me about if (x = 0).
> 
> Some compilers warn you.  Not all do.  An error from all compilers is
> also far better than a warning from some compilers.

You can make it an error with the only compiler that can build QEMU if
you want.  I haven't seen any compiler which doesn't warn about it in
the last 10 years, except on machines which are more than 10 years old.

> This is why people writing safety critical code in C require this order.
> It is simply the safest choice.

I have a coding style which forbids this "safest choice" on the
grounds that it causes more mistakes than it avoids.

Of course I never use compilers which don't warn about it.  That would
be really stupid - *especially* when writing safety critical code.  If
you ignore _any_ warning in such code you are in big trouble.

Actually, these days for safety critical code it would be stupid not
to use the more advanced code checking tools such as Sparse...

-- Jamie

Re: [Qemu-devel] [7234] Use a more natural order

[Anthony Liguori]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>   
>>> It might take 30 minutes, but it saves large amounts of time for everyone
>>> else later.  Also I would hope make is smart enough to only recompile
>>> the bits that changed, so that shouldn't take as long.
>>>       
>> A great deal of our objects are target specific.  Almost everything 
>> depends on CONFIG_USER_ONLY too.  This means that full compilation takes 
>> a very long time.
>>     
>
> You might find ccache helps.  For this kind of testing, nearly every
> compile should be a cache hit, so the slow part is limited running
> configure, make and preprocessing, and the final link.
>   

If it wasn't for ccache, my current testing setup would probably take 
days to complete :-)

For each patch, I do an rm -rf $(builddir) to also catch any possible 
bugs in configure and avoid false positives from old config-* files.

Regards,

Anthony Liguori

Re: [Qemu-devel] [7234] Use a more natural order

[Gerd Hoffmann]

On 04/23/09 20:53, Lennart Sorensen wrote:
> fails compile, while
>
> if (x = 0) {
>
> compiles silently even when you didn't mean that.

gcc warns about that these days ...

cheers
   Gerd

Re: [Qemu-devel] [7234] Use a more natural order

[Gerd Hoffmann]

On 04/23/09 22:54, Lennart Sorensen wrote:
> Isn't there a configure option for building for all available targets?
>
> It might take 30 minutes, but it saves large amounts of time for everyone
> else later.  Also I would hope make is smart enough to only recompile
> the bits that changed, so that shouldn't take as long.

Even picking a reasonable subset catches most of these.

/me builds i386 + x86_64 softmmu (that's what I'm working on)
also usermode variants (to catch build bugs there)
and sparc (because it has neither xen nor kvm and also is bigendian)

btw: Does qemu support compiling with separate src/obj trees?  I think 
it doesn't, but would be useful because one could easily add a mingw32 
windows cross build to the picture ...

cheers,
   Gerd

Re: [Qemu-devel] [7234] Use a more natural order

[Anthony Liguori]

Gerd Hoffmann wrote:
> btw: Does qemu support compiling with separate src/obj trees?  I think 
> it doesn't, but would be useful because one could easily add a mingw32 
> windows cross build to the picture ...

It does and this is how I build for Windows and PPC.

Regards,

Anthony Liguori
> cheers,
>   Gerd

Re: [Qemu-devel] [7234] Use a more natural order

[Stefan Weil]

Gerd Hoffmann schrieb:
> On 04/23/09 22:54, Lennart Sorensen wrote:
>> Isn't there a configure option for building for all available targets?
>>
>> It might take 30 minutes, but it saves large amounts of time for
>> everyone
>> else later.  Also I would hope make is smart enough to only recompile
>> the bits that changed, so that shouldn't take as long.
>
> Even picking a reasonable subset catches most of these.
>
> /me builds i386 + x86_64 softmmu (that's what I'm working on)
> also usermode variants (to catch build bugs there)
> and sparc (because it has neither xen nor kvm and also is bigendian)
>
> btw: Does qemu support compiling with separate src/obj trees?  I think
> it doesn't, but would be useful because one could easily add a mingw32
> windows cross build to the picture ...

It does, and I use separate obj trees since years for this purpose
(native build and cross builds for windows and ppc).

>
> cheers,
>   Gerd
>
>
>

Re: [Qemu-devel] [7234] Use a more natural order

[Lennart Sorensen]

On Thu, Apr 23, 2009 at 11:46:25PM +0100, Jamie Lokier wrote:
> Lennart Sorensen wrote:
> > On Thu, Apr 23, 2009 at 10:31:05PM +0300, Blue Swirl wrote:
> > > I don't think any code style document can cover all possible cases.
> > > But another approach can be used: you could try to find a precedent
> > > case where this style has been used in QEMU.
> > 
> > # grep -r '([0-9] =' .
> > ./net.c:    if (0 == errno && '\0' == *last_char &&
> > ./hw/gus.c:    if (0 == ((mode >> 4) & 1)) {
> > ./hw/dma.c:            if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) {
> > ./hw/sb16.c:        if (0 == s->needed_bytes) {
> > ./hw/sb16.c:            if (0 == s->needed_bytes) {
> > ./hw/sb16.c:        if (0 == s->dma_auto) {
> > ./hw/sb16.c:        if (0 == s->dma_auto) {
> > 
> > That was just one quick search.  Looks like whoever wrote a bunch of
> > the audio hardware emulation liked less buggy code.
> 
> No, it means they believe that way around makes them less likely to
> write bugs.  They may be right for themselves.  It doesn't mean the
> code is less buggy, and it doesn't mean someone else following the
> same style would write less buggy code because of the style.
> 
> Same for the redundant parentheses around every comparison next to &&.
> Really, there is no need: if (condition && condition) is so common
> that there's no need to clarify the grouping.  Unlike say "x & 1 << 2"
> where it's a good idea to add parentheses.
> 
> > Well to me software development is a kind of engineering and hence using
> > anything but the safest practice that is at all practical makes no sense.
> 
> But what you call safest isn't safest for everyone.  A lot depends on
> what people are familiar with and trained to spot deviations from.
> 
> > That means:
> > Constants before variables in all comparisons.
> > Braces are never optional for blocks.
> 
> I disagree with both, but especially the first.  You call it safe
> practice.  I call it likely to cause more bugs because it's unfamiliar
> compared with common practice.  And because it's the opposite to the
> way we say it mathematically or in English.

Not everyone on the planet speaks english.  And common practice
(fortuantely) changes over time.

> It's opposite to the "Object Verb Subject" grammar that English
> speakers think in: asking "is zero equal to x?"  is really weird
> English, because that is asking a question _about zero_, which is not
> right: zero is not meant to be something about which there is any question.
> It's a constant!

Again, that may be true in english, but not many other languages.

> That cognitive dissonance adds a translation step between thinking (in
> English) and reading/writing, which means more chance of not seeing
> other bugs - limited brainpower and all.
> 
> "if (0 < x)" is even worse: it's begging to be accidentally written
> with ">".  More importantly, if it _is_ accidentally written with ">",
> the mistake is less likely to be noticed than if it were written the
> other way around.  But if you write "if (0 == x)" and "if (x > 0)"
> just because of "=" C syntax, that's an inconsistent mess.

Well preferably someone would invent a time machine, and go back and
smack whoever thought using = for assignment in C was a good idea.  :=
or something else that isn't just one character away from a completely
different operation would have been much better.

> Do you also write things like "for (x = 0; NUMBER > x; x++)"?  No, of
> course not.

Well to some extent, when doing < and >, you are not a single character
typo away from assigment.  It is == versus = that is the problem.

> Sure, it was ok practice back in the days when compilers would let you
> write "if (x = 0)", but those days are long gone.  Now all compilers
> warn, and you can ask them to make it an error.  A compiler check is
> much safer than depending on writing style, especially a weird one
> that's inconsistent with other writing in the same code, and
> inconsistent with English thinking.

I highly doubt all compilers warn.  Current gcc versions do, but not
everyone uses current gcc.

> Accidentally mistaking the scope of a block is unlikely using proper
> tools.  We're not using NOTEPAD.EXE any more.  Editors can indent code
> automatically these days (last 20 years or so).  Again, using tools is
> better than depending on writing style because it catches more mistakes.

The problem is when one wants to add a printf or something for debuging
in a condition and accidentally made the conditional code now happen
all the time.  It also means adding something to the condition now
requires changing 3 lines, to add one, while removing a line will
sometimes involve changing 3 lines when you drop down to a single
expression.

if (x > 0)
	printf("Found an x\n");
	y = x;

is a really annoying bug to find caused by trying to debug things when
braces are not used.  Now adding the debug line requires changing the
if line and adding another line afterwards.

> Actually I'm sympathetic to people who prefer braces around every
> block, and I happily follow such a style when requested.  It doesn't
> look too painful or weird, unlike the "unnatural" comparison order. :-)
> But I don't think it's necessary on projects where everyone uses other
> equally good practices instead to catch the same errors.

-- 
Len Sorensen

Re: [Qemu-devel] [7234] Use a more natural order

[Nathan Froyd]

On Fri, Apr 24, 2009 at 02:07:06PM -0400, Lennart Sorensen wrote:
> On Thu, Apr 23, 2009 at 11:46:25PM +0100, Jamie Lokier wrote:
> > Sure, it was ok practice back in the days when compilers would let you
> > write "if (x = 0)", but those days are long gone.  Now all compilers
> > warn, and you can ask them to make it an error.  A compiler check is
> > much safer than depending on writing style, especially a weird one
> > that's inconsistent with other writing in the same code, and
> > inconsistent with English thinking.
> 
> I highly doubt all compilers warn.  Current gcc versions do, but not
> everyone uses current gcc.

Documentation indicates that the relevant warning (-Wparatheses) has
been supported since at least 2.95.3 (released early 2001); I imagine it
has been supported since the corresponding major release of 2.95.0
(released mid-1999).

-Nathan