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

Re: [PATCH 01/12] libxc/save: Shrink code volume where possible


  • To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • From: Ian Jackson <ian.jackson@xxxxxxxxxx>
  • Date: Mon, 27 Apr 2020 18:19:37 +0100
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=ian.jackson@xxxxxxxxxx; spf=Pass smtp.mailfrom=Ian.Jackson@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 27 Apr 2020 17:20:15 +0000
  • Ironport-sdr: RTfL2MK298dSewkDORuNIH88IT9ojSZwpeM2g+p0eEdXIpHDsNaIVwaxe7K6ruaSve6SKe1JMF eTvX7ZnTNPUbnlRTf1Wbj0amjcV9svFLXw9w1u0edx/36NqIMs6RAGqOwGM4QXZ7RqikSa1Bxt Ffb3EyGScmIYfbIDHPNJeNv4RMirrw2aRbOIO1WEfyj811rI9DsDOswGT6vYKH+exz1fskHEz3 mQ+nK0tzdNQhSFUk2TMFe/W1iR0Poq8I1+sgw8TT4oOk9+WI0f//Z4V6s4qRnJLXoJqXHNkbtP jFc=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Andrew Cooper writes ("Re: [PATCH 01/12] libxc/save: Shrink code volume where 
possible"):
> On 14/01/2020 16:48, Ian Jackson wrote:
> > Andrew Cooper writes ("[PATCH 01/12] libxc/save: Shrink code volume where 
> > possible"):
> >> A property of how the error handling (0 on success, nonzero otherwise)
> >> allows these calls to be chained together with the ternary operatior.
> > I'm quite surprised to find a suggestion like this coming from you in
> > particular.
> 
> What probably is relevant is that ?: is a common construct in the
> hypervisor, which I suppose does colour my expectation of people knowing
> exactly what it means and how it behaves.

I expect other C programmers to know what ?: does, too.  But I think
using it to implement the error monad is quite unusual.  I asked
around a bit and my feeling is still that this isn't an improvement.

> > Or just to permit
> >    rc = write_one_vcpu_basic(ctx, i);    if (rc) goto error;
> > (ie on a single line).
> 
> OTOH, it should come as no surprise that I'd rather drop this patch
> entirely than go with these alternatives, both of which detract from
> code clarity. The former for hiding control flow, and the latter for
> being atypical layout which unnecessary cognitive load to follow.

I think, then, that it would be best to drop this patch, unless Wei
(or someone else) disagrees with me.

Sorry,
Ian.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.