| Message ID | 1456312061-2606-1-git-send-email-write.harmandeep@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-02-24 at 16:37 +0530, Harmandeep Kaur wrote: > Check the return value of xc_version() and return NULL if it > fails. libxl_get_version_info() can also return NULL now. > > Callers of the function libxl_get_version_info() are already > prepared to deal with a NULL return value. > > Coverity ID 1351217 > > Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com> Thanks and regards, Dario
Harmandeep Kaur writes ("[PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()"):
> Check the return value of xc_version() and return NULL if it
> fails. libxl_get_version_info() can also return NULL now.
>
> Callers of the function libxl_get_version_info() are already
> prepared to deal with a NULL return value.
Thanks for your submission.
> long xen_version;
> + long r = 0;
Why did you choose `long' for the type of `r' ? Normally it would be
an int and AFAICT xc_version returns int.
> v3: Group all calls to xc_version() , so that data copies in
> various info fields only if all calls to xc_version work.
This should be in the main part of the commit message, and also the
explanation of why it is necessary.
Thanks,
Ian.
On Tue, Mar 1, 2016 at 7:18 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > Harmandeep Kaur writes ("[PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()"): >> Check the return value of xc_version() and return NULL if it >> fails. libxl_get_version_info() can also return NULL now. >> >> Callers of the function libxl_get_version_info() are already >> prepared to deal with a NULL return value. > > Thanks for your submission. > >> long xen_version; >> + long r = 0; > > Why did you choose `long' for the type of `r' ? Normally it would be > an int and AFAICT xc_version returns int. "xen_version" is already using long. Thats the only reason I used long fot 'r' . We can make int r , no issues there. > >> v3: Group all calls to xc_version() , so that data copies in >> various info fields only if all calls to xc_version work. > > This should be in the main part of the commit message, and also the > explanation of why it is necessary. > > Thanks, > Ian. Thanks for the help.
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 2d18b8d..07f2667 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -5268,41 +5268,44 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx) xen_commandline_t xen_commandline; } u; long xen_version; + long r = 0; libxl_version_info *info = &ctx->version_info; if (info->xen_version_extra != NULL) goto out; xen_version = xc_version(ctx->xch, XENVER_version, NULL); + if (xen_version < 0) goto out; + r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); + if (r < 0) goto out; + r = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); + if (r < 0) goto out; + r = xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps); + if (r < 0) goto out; + r = xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset); + if (r < 0) goto out; + r = xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms); + if (r < 0) goto out; + r = info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL); + if (r < 0) goto out; + r = xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline); + if (r < 0) goto out; + info->xen_version_major = xen_version >> 16; info->xen_version_minor = xen_version & 0xFF; - - xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra); - - xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler); info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by); info->compile_domain = libxl__strdup(NOGC, u.xen_cc.compile_domain); info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date); - - xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps); info->capabilities = libxl__strdup(NOGC, u.xen_caps); - - xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset); info->changeset = libxl__strdup(NOGC, u.xen_chgset); - - xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms); info->virt_start = u.p_parms.virt_start; - - info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL); - - xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline); info->commandline = libxl__strdup(NOGC, u.xen_commandline); out: GC_FREE; - return info; + return r < 0 ? NULL : info; } libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
Check the return value of xc_version() and return NULL if it fails. libxl_get_version_info() can also return NULL now. Callers of the function libxl_get_version_info() are already prepared to deal with a NULL return value. Coverity ID 1351217 Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com> --- v2: Change local variable rc to r. Removes xen_version. Better readiblity of blocks of code. Group all calls to xc_version() , so that data copies in various info fields only if all calls to xc_version go error-free. v3: Group all calls to xc_version() , so that data copies in various info fields only if all calls to xc_version work. v4: Improve suboptimal subject. Readds xen_version to suit re-arrangement. --- tools/libxl/libxl.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-)