[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/5] libxl: tidy libxl_get_scheduler() according to CODING_STYLE
On Mon, 2015-12-28 at 00:16 -0500, Joshua Otto wrote: > To more closely follow the guidelines in CODING_STYLE, store the result > of the libxc call in the local variable r, and the check the result of > the call in a separate statement. I think a far more important aspect of this change is:Â don't store the int return value of xc_sched_id into a variable of type libxl_scheduler libxl_scheduler is an enum, and hence "int-like", but... still. I think this is worth mentioning in the commit message, mainly because I'm only 99% confident this is just a benign oddity rather than an actual latent bug. > Additionally, change the error log statement to more accurately reflect > the failure.ÂÂThis is the only functional change introduced by this > patch. Right. > Signed-off-by: Joshua Otto <jtotto@xxxxxxxxxxxx> > --- > Âtools/libxl/libxl.c | 7 ++++--- > Â1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 9207621..ca4679b 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -5585,10 +5585,11 @@ out: > Â > Âlibxl_scheduler libxl_get_scheduler(libxl_ctx *ctx) > Â{ > -ÂÂÂÂlibxl_scheduler sched, ret; > ÂÂÂÂÂGC_INIT(ctx); > -ÂÂÂÂif ((ret = xc_sched_id(ctx->xch, (int *)&sched)) != 0) { > -ÂÂÂÂÂÂÂÂLOGE(ERROR, "getting domain info list"); > +ÂÂÂÂlibxl_scheduler sched; > +ÂÂÂÂint r = xc_sched_id(ctx->xch, (int *)&sched); If you were minded to make a further cleanup (i.e. this is totally optional) then I'm not convinced this case is actually safe, since libxl_scheduler could potentially be smaller than sizeof(int), or at least IIRC the C standard give the compiler that option, although I don't know if gcc will make use of it or if something else (e.g. OS calling convention on Linux) would make it a non-issue (or I might be totally wrong...). Safer (and cleaner looking even if I'm wrong) would be to use a temporary int for the function call and turn it into an enum implicitly in the return. > +ÂÂÂÂif (r != 0) { > +ÂÂÂÂÂÂÂÂLOGE(ERROR, "getting current scheduler id"); > ÂÂÂÂÂÂÂÂÂreturn ERROR_FAIL; > ÂÂÂÂÂÂÂÂÂGC_FREE; > ÂÂÂÂÂ} _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |