From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: Re: [PATCH 11/29] libxl: events: Make
 libxl__async_exec_* pass caller an rc
Date: Tue, 31 Mar 2015 19:12:00 +0100
Message-ID: <21786.58224.721010.592110@mariner.uk.xensource.com>
References: <1423599016-32639-1-git-send-email-ian.jackson@eu.citrix.com>
	<1423599016-32639-12-git-send-email-ian.jackson@eu.citrix.com>
	<1427196013.21742.346.camel@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1427196013.21742.346.camel@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <ian.campbell@citrix.com>
Cc: xen-devel@lists.xensource.com, Euan Harris <euan.harris@citrix.com>
List-Id: xen-devel@lists.xenproject.org

Ian Campbell writes ("Re: [Xen-devel] [PATCH 11/29] libxl: events: Make libxl__async_exec_* pass caller an rc"):
> On Tue, 2015-02-10 at 20:09 +0000, Ian Jackson wrote:
> > diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> > index 754e2d1..891cdb8 100644
> > --- a/tools/libxl/libxl_aoutils.c
> > +++ b/tools/libxl/libxl_aoutils.c
> > @@ -483,11 +483,12 @@ static void async_exec_done(libxl__egc *egc,
> >      libxl__ev_time_deregister(gc, &aes->time);
> >  
> >      if (status) {
> > -        libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
> > -                                      aes->what, pid, status);
> > +        if (!aes->rc)
> 
> Could be one "if (status && !aes->rc)", unless perhaps there is more
> code to come in this block?

No, there is no more to come.  I find it clearer this way but I don't
mind changing it.

> > +                        libxl__async_exec_state *aes, int rc, int status);
> > +/*
> > + * Meaning of status and rc:
> > + *  rc==0, status==0    all went well
> > + *  rc==0, status!=0    everything OK except child exited nonzero (logged)
> > + *  rc!=0               something else went wrong (status is real
> > + *                       exit status, maybe reflecting SIGKILL if aes
> > + *                       code killed the child).  Logged unless CANCELLED.
> 
> I'm unclear on whether status is valid in this third case or not. I
> think you are saying that it is (probably?) valid but if rc!=0 the
> caller likely doesn't actually care what it is?

status is definitely valid but maybe uninteresting, as stated in the
comment.

Would it help to add something about status to the third row of the
little table bit at the left ?

Ian.