[PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()

[PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()

[Harmandeep Kaur]

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(-)

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,
-- 
2.5.0

Re: [PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 744 bytes --]

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
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()

[Ian Jackson]

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.

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

Re: [PATCH v4] libxl: handle failure of xc_version() in libxl_get_version_info()

[Harmandeep Kaur]

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.

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