Re: [Xen-devel] [PATCH 5/9] libxl: introduce libxl_cpupool_cpu{add, remove}_cpumap()

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-03-11 at 16:50 +0000, Dario Faggioli wrote:
> On Wed, 2015-03-11 at 16:42 +0000, Ian Campbell wrote:
> > On Mon, 2015-03-09 at 10:39 +0000, Wei Liu wrote:
> > >      rc = xc_cpupool_addcpu(ctx->xch, poolid, cpu);
> > > >      if (rc) {
> > > > -        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
> > > > -            "Error moving cpu to cpupool");
> > > > +        LOGE(ERROR, "Error moving cpu %d to cpupool", cpu);
> > > > +        GC_FREE;
> > > >          return ERROR_FAIL;
> > > >      }
> > > > +    GC_FREE;
> > > 
> > > Please use "goto" idiom. Same applies to libxl_cpupool_cpuremove.
> > 
> > TBH I think using LIBXL__LOG_ERRNOVAL in functions where the GC is
> > otherwise unneeded in the entire call-chain, like here, is a reasonable
> > exception to the usual rule.
> > 
> And I distinctly remember someone of you tools people saying, about
> another patch, that "using the new macro is well worth adding a
> GC_INIT/FREE" :-D

Could have even be me on another day ;-)

> Of course, I'm fine with leaving this alone... just let me know what you
> prefer. :-)

Either use GC+goto style error paths or leave it alone, either is fine
by me.

Ian.


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