[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 06/15] block/mirror: conservative mirror_exit

From: Eric Blake
Subject: Re: [Qemu-devel] [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

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]