[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] libxl/x86: check return value of SHADOW_OP_SET_ALLOCATION domctl
On 02.07.2021 17:12, Jan Beulich wrote: > On 02.07.2021 16:46, Anthony PERARD wrote: >> On Fri, Jul 02, 2021 at 02:29:31PM +0200, Jan Beulich wrote: >>> On 01.07.2021 11:36, Anthony PERARD wrote: >>>> On Mon, Jun 28, 2021 at 01:47:03PM +0200, Jan Beulich wrote: >>>>> --- a/tools/libs/light/libxl_x86.c >>>>> +++ b/tools/libs/light/libxl_x86.c >>>>> @@ -531,8 +531,18 @@ int libxl__arch_domain_create(libxl__gc >>>>> if (d_config->b_info.type != LIBXL_DOMAIN_TYPE_PV) { >>>>> unsigned long shadow = DIV_ROUNDUP(d_config->b_info.shadow_memkb, >>>>> 1024); >>>>> - xc_shadow_control(ctx->xch, domid, >>>>> XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, >>>>> - NULL, 0, &shadow, 0, NULL); >>>>> + int rc = xc_shadow_control(ctx->xch, domid, >>>> >>>> Could you use 'r' instead of 'rc' ? The later is reserved for libxl >>>> error codes while the former is for system and libxc calls. >>> >>> Of course I can, but I did look at the rest of the function and >>> found that it uses "ret" for the purpose of what you now say >>> "rc" ought to be used for. Seeing "ret", I decided to avoid it >>> (knowing you use different names for different kinds of return >>> values). While I've switched to "r" for now, I'd be rather >>> inclined to re-use "ret" instead. (Or actually, as per the >>> remark further down, I can get away without any local variable >>> then.) >> >> I know there's quite a few (many?) coding style issue in libxl. I'm >> trying to prevent new issue without asking to fix the existing one. >> The use of "ret" is an already existing issue, so I'm fine with it been >> use in this patch for libxl error code in the function. >> >> BTW, you still need to store the return value of xc_shadow_control() >> into a "r" variable before checking it for error. > > Are you saying that > > if (xc_shadow_control(ctx->xch, domid, > XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, > NULL, 0, &shadow_mb, 0, NULL)) { > > is not acceptable, style-wise? Oh, there is indeed such a rule under "ERROR HANDLING". Which means ... > If indeed you are, please disambiguate > your statement above regarding the use of "ret": May I or may I not > use it? IOW do I need to introduce "r", or can I get away with the > existing local variables. ... I need this to be clarified. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |