From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47380) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLn2h-0006Wc-4N for qemu-devel@nongnu.org; Wed, 20 Jan 2016 02:19:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLn2c-0001Ld-5o for qemu-devel@nongnu.org; Wed, 20 Jan 2016 02:18:59 -0500 From: Markus Armbruster References: <1452859244-9500-1-git-send-email-david@gibson.dropbear.id.au> <1452859244-9500-8-git-send-email-david@gibson.dropbear.id.au> <569EBFA6.6090709@redhat.com> <569ED31D.1020307@ozlabs.ru> <569F12D2.60307@redhat.com> Date: Wed, 20 Jan 2016 08:18:48 +0100 In-Reply-To: <569F12D2.60307@redhat.com> (Eric Blake's message of "Tue, 19 Jan 2016 21:53:38 -0700") Message-ID: <87fuxs3ehz.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 07/10] pseries: Clean up error handling in spapr_rtas_register() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Alexey Kardashevskiy , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, mdroth@linux.vnet.ibm.com, David Gibson Eric Blake writes: > On 01/19/2016 05:21 PM, Alexey Kardashevskiy wrote: > >>> You could drop the redundant () while touching this, as in: >> >> >> Seriously? Why? I personally find it really annoying (but I stay silent) >> when people omit braces in cases like this. >> >> >>> assert(token >= RTAS_TOKEN_BASE && token < RTAS_TOKEN_MAX); > > Because it's the prevailing style. I estimate that less than 10% of qemu > over-parenthesizes, mostly because && and || are well-known C operator > precedence: > > $ git grep ' && ' | wc > 6462 57034 482477 > $ git grep ') && (' | wc > 578 6151 48655 > > Of course, that's a rough estimate, as it has false positives on 'if > (foo() && (b || c))', and false negatives on conditionals where there is > a unary rather than binary operator on either side of &&; but I'm sure > you could write a Coccinelle script if you wanted more accurate counting. > > But you are equally right that as long as HACKING doesn't document it, > and checkpatch.pl doesn't flag it, then you can over-parenthesize binary > arguments to the short-circuiting operators to your aesthetic tastes. HACKING doesn't document everything. Trying to document everything would drown the interesting parts in a sea of platitudes, and still leave innumerable loopholes. checkpatch.pl doesn't flag everything. It checks for *common* unwanted patterns. When HACKING and checkpatch.pl are silent, make your change blend in with the existing code. Since the existing code overwhelmingly eschews this kind of superfluous parenthesis, the general rule is to knock them off unless *local* code overwhelmingly uses them. Just because HACKING doesn't explicitly prohibit your personal preferences doesn't mean you get to do leave your stylistic mark on the code. Show some taste and make yourself invisible. [...]