From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Lx7gf-0004uk-CO for qemu-devel@nongnu.org; Thu, 23 Apr 2009 18:46:33 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lx7ga-0004tB-MZ for qemu-devel@nongnu.org; Thu, 23 Apr 2009 18:46:33 -0400 Received: from [199.232.76.173] (port=52446 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lx7ga-0004t2-Dr for qemu-devel@nongnu.org; Thu, 23 Apr 2009 18:46:28 -0400 Received: from mail2.shareable.org ([80.68.89.115]:36067) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Lx7ga-0003Yu-0v for qemu-devel@nongnu.org; Thu, 23 Apr 2009 18:46:28 -0400 Date: Thu, 23 Apr 2009 23:46:25 +0100 From: Jamie Lokier Subject: Re: [Qemu-devel] [7234] Use a more natural order Message-ID: <20090423224625.GP13326@shareable.org> References: <20090423185308.GH3795@csclub.uwaterloo.ca> <20090423191040.GI3795@csclub.uwaterloo.ca> <20090423194447.GK3795@csclub.uwaterloo.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090423194447.GK3795@csclub.uwaterloo.ca> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Lennart Sorensen Cc: Blue Swirl , qemu-devel@nongnu.org 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