|
From: | Eric Blake |
Subject: | Re: [Qemu-block] [PATCH v4 06/15] block/mirror: conservative mirror_exit refactor |
Date: | Wed, 5 Sep 2018 10:50:03 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 09/05/2018 08:09 AM, John Snow wrote:
On 09/05/2018 06:43 AM, Max Reitz wrote:On 2018-09-04 19:09, John Snow wrote:For purposes of minimum code movement, refactor the mirror_exit callback to use the post-finalization callbacks in a trivial way. Signed-off-by: John Snow <address@hidden> --- block/mirror.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-)Reviewed-by: Max Reitz <address@hidden> (Although I believe the ?: hunk from the previous patch should be here. Also note that we have a couple of places that make use of the GNU extension for "?:" as a binary operator (as in "x ?: y" returns x if x != 0). Just in case you find "s->to_replace ?: src" as appealing as I do.)Ah, I wasn't sure that was OK to use. Meh, since I goofed up the last patch I'll use that version.
My minor reason for liking 'a ?: b' - it's less typing, and makes avoiding long lines easier. My major reason for liking it: 'a() ?: b' only evaluates a() once, unlike 'a() ? a() : b' when taking the true branch, which can be particularly important if a() has side effects, but is still an efficiency issue even when side effects are not in play. Yes, you can always rewrite things to use a temporary variable to avoid the ?: operator (which in turn can inflate a single-line expression into multiple lines), but there are indeed enough places in the code base where we rely on this extension that it doesn't hurt to add more uses.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |