All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org,
	mdroth@linux.vnet.ibm.com,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [Qemu-devel] [PATCH 07/10] pseries: Clean up error handling in spapr_rtas_register()
Date: Wed, 20 Jan 2016 19:15:26 +1100	[thread overview]
Message-ID: <569F421E.6000106@ozlabs.ru> (raw)
In-Reply-To: <87fuxs3ehz.fsf@blackfin.pond.sub.org>

On 01/20/2016 06:18 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> 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.


In order to educate myself - where/when was this wonderful rule 
established? What are the other rules then?


> 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.

Nice, now we are discussing taste :-/


-- 
Alexey

  reply	other threads:[~2016-01-20  8:15 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-15 12:00 [Qemu-devel] [PATCH 00/10] Cleanups to error reporting on ppc and spapr (v2) David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 01/10] ppc: Cleanup error handling in ppc_set_compat() David Gibson
2016-01-15 15:19   ` Markus Armbruster
2016-01-17  9:32     ` David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 02/10] pseries: Cleanup error handling of spapr_cpu_init() David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 03/10] pseries: Clean up hash page table allocation error handling David Gibson
2016-01-18  2:44   ` Alexey Kardashevskiy
2016-01-18  4:42     ` David Gibson
2016-01-18  5:17       ` Alexey Kardashevskiy
2016-01-18  5:35         ` David Gibson
2016-01-18  6:04           ` Alexey Kardashevskiy
2016-01-18  8:17             ` David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 04/10] pseries: Clean up error handling in spapr_validate_node_memory() David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 05/10] pseries: Cleanup error handling in spapr_vga_init() David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 06/10] pseries: Improve error handling in find_unknown_sysbus_device() David Gibson
2016-01-15 15:40   ` Markus Armbruster
2016-01-18  2:50     ` David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 07/10] pseries: Clean up error handling in spapr_rtas_register() David Gibson
2016-01-19 22:58   ` Eric Blake
2016-01-20  0:21     ` Alexey Kardashevskiy
2016-01-20  4:53       ` Eric Blake
2016-01-20  5:53         ` Alexey Kardashevskiy
2016-01-20 10:08           ` Thomas Huth
2016-01-20  7:18         ` Markus Armbruster
2016-01-20  8:15           ` Alexey Kardashevskiy [this message]
2016-01-20  9:27             ` Markus Armbruster
2016-01-15 12:00 ` [Qemu-devel] [PATCH 08/10] pseries: Clean up error handling in xics_system_init() David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 09/10] pseries: Clean up error reporting in ppc_spapr_init() David Gibson
2016-01-15 12:00 ` [Qemu-devel] [PATCH 10/10] pseries: Clean up error reporting in htab migration functions David Gibson
2016-01-15 15:47 ` [Qemu-devel] [PATCH 00/10] Cleanups to error reporting on ppc and spapr (v2) Markus Armbruster
2016-01-16 13:15   ` David Gibson
2016-01-17 23:56   ` Alexey Kardashevskiy

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=569F421E.6000106@ozlabs.ru \
    --to=aik@ozlabs.ru \
    --cc=armbru@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=eblake@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.