Re: [PATCH] libxl: fix an error path that uses uninitialised rc in libxl_set_memory_target

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jun 22, 2016 at 06:58:28AM -0600, Jan Beulich wrote:
> >>> On 12.06.16 at 16:09, <wei.liu2@citrix.com> wrote:
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -4927,10 +4927,12 @@ retry_transaction:
> >  
> >      target = libxl__xs_read(gc, t, GCSPRINTF("%s/memory/target", dompath));
> >      if (!target && !domid) {
> > -        if (!xs_transaction_end(ctx->xsh, t, 1))
> > +        if (!xs_transaction_end(ctx->xsh, t, 1)) {
> > +            rc = ERROR_FAIL;
> 
> I'm sorry for noticing this only now - is ERROR_FAIL the right thing
> to use here, considering how things worked before the change that
> introduced the issue getting fixed here? I had intentionally decided
> to use ERROR_INVAL in the patch variant I did submit (as at that
> time I wasn't yet aware of the other fix floating around already).
> 

When I wrote this patch, I thought the return value should be tied to
xs_transaction_end.

The original implementation could return 1 -- that's not ERROR_INVAL,
what did I miss? And the original return value was mad enough anyway so
I don't see a need to retain that -- after all the purpose of
ecdc6fd8787 is to fix the return value of that function.

Wei.

> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel