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

[Eric Blake]

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