Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[ayush ruia]

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

Hi,
In the function libxl__fill_dom0_memory_info in the file libxl.c, here we
see that the function is called as follows:

4329
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4329>
static int libxl__fill_dom0_memory_info(libxl__gc *gc, uint32_t *target_memkb,
4330
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4330>
                                        uint32_t *max_memkb)


Here, we are passed the pointers to the variables *max_memkb and
*target_memkb. In the code below we see that we first read the string value
of target, staticmax, and freememslack from the files listed in the path in
the function. However, if the values are set, then we do not update the
value of the pointers passed via reference in the function call. Only if
the three variables are not set, then do we update the values of the
variable passed through reference. Otherwise these values are not updated
and the functions exits. This might be a bug, or it may be intentional. I
do not understand why are we passing the variables via reference in the
function if it is not even set in the function.

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD

4346
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4346>
    target = libxl__xs_read(gc, t, target_path);
4347
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4347>
    staticmax = libxl__xs_read(gc, t, max_path);
4348
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4348>
    freememslack = libxl__xs_read(gc, t, free_mem_slack_path);
4349
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4349>
    if (target && staticmax && freememslack) {
4350
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4350>
        rc = 0;
4351
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4351>
        goto out;
4352
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4352>
    }
4353
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4353>
4354
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4354>
    if (target) {
4355
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4355>
        *target_memkb = strtoul(target, &endptr, 10);
4356
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4356>
        if (*endptr != '\0') {
4357
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4357>
            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
4358
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4358>
                    "invalid memory target %s from %s\n", target, target_path);
4359
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4359>
            rc = ERROR_FAIL;
4360
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4360>
            goto out;
4361
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4361>
        }
4362
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4362>
    }
4363
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4363>
4364
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4364>
    if (staticmax) {
4365
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4365>
        *max_memkb = strtoul(staticmax, &endptr, 10);
4366
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4366>
        if (*endptr != '\0') {
4367
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4367>
            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
4368
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4368>
                             "invalid memory static-max %s from %s\n",
4369
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4369>
                             staticmax, max_path);
4370
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4370>
            rc = ERROR_FAIL;
4371
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4371>
            goto out;
4372
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4372>
        }
4373
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4373>
    }
4374
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4374>


Regards,
Ayush Ruia.

[-- Attachment #1.2: Type: text/html, Size: 11561 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: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[ayush ruia]

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

What I am trying to say is that shouldn't the code block highlighted in
yellow should come before the block marked in green. Then it would update
the value of *target_memkb and *max_memkb in all possible situations.

4346
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4346>
    target = libxl__xs_read(gc, t, target_path);
4347
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4347>
    staticmax = libxl__xs_read(gc, t, max_path);
4348
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4348>
    freememslack = libxl__xs_read(gc, t, free_mem_slack_path);



4349
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4349>
    if (target && staticmax && freememslack) {
4350
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4350>
        rc = 0;
4351
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4351>
        goto out;
4352
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4352>
    }
4353
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4353>
4354
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4354>
    if (target) {
4355
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4355>
        *target_memkb = strtoul(target, &endptr, 10);
4356
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4356>
        if (*endptr != '\0') {
4357
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4357>
            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
4358
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4358>
                    "invalid memory target %s from %s\n", target, target_path);
4359
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4359>
            rc = ERROR_FAIL;
4360
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4360>
            goto out;
4361
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4361>
        }
4362
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4362>
    }
4363
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4363>
4364
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4364>
    if (staticmax) {
4365
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4365>
        *max_memkb = strtoul(staticmax, &endptr, 10);
4366
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4366>
        if (*endptr != '\0') {
4367
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4367>
            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
4368
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4368>
                             "invalid memory static-max %s from %s\n",
4369
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4369>
                             staticmax, max_path);
4370
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4370>
            rc = ERROR_FAIL;
4371
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4371>
            goto out;
4372
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4372>
        }
4373
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4373>
    }
4374
<http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4374>

Regards,
Ayush Ruia.

On Thu, Oct 9, 2014 at 8:07 PM, ayush ruia <ayushruia@gmail.com> wrote:

> Hi,
> In the function libxl__fill_dom0_memory_info in the file libxl.c, here we
> see that the function is called as follows:
>
> 4329
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4329>
> static int libxl__fill_dom0_memory_info(libxl__gc *gc, uint32_t *target_memkb,
> 4330
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4330>
>                                         uint32_t *max_memkb)
>
>
> Here, we are passed the pointers to the variables *max_memkb and
> *target_memkb. In the code below we see that we first read the string value
> of target, staticmax, and freememslack from the files listed in the path in
> the function. However, if the values are set, then we do not update the
> value of the pointers passed via reference in the function call. Only if
> the three variables are not set, then do we update the values of the
> variable passed through reference. Otherwise these values are not updated
> and the functions exits. This might be a bug, or it may be intentional. I
> do not understand why are we passing the variables via reference in the
> function if it is not even set in the function.
>
>
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD
>
> 4346
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4346>
>     target = libxl__xs_read(gc, t, target_path);
> 4347
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4347>
>     staticmax = libxl__xs_read(gc, t, max_path);
> 4348
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4348>
>     freememslack = libxl__xs_read(gc, t, free_mem_slack_path);
> 4349
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4349>
>     if (target && staticmax && freememslack) {
> 4350
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4350>
>         rc = 0;
> 4351
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4351>
>         goto out;
> 4352
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4352>
>     }
> 4353
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4353>
> 4354
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4354>
>     if (target) {
> 4355
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4355>
>         *target_memkb = strtoul(target, &endptr, 10);
> 4356
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4356>
>         if (*endptr != '\0') {
> 4357
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4357>
>             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> 4358
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4358>
>                     "invalid memory target %s from %s\n", target, target_path);
> 4359
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4359>
>             rc = ERROR_FAIL;
> 4360
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4360>
>             goto out;
> 4361
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4361>
>         }
> 4362
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4362>
>     }
> 4363
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4363>
> 4364
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4364>
>     if (staticmax) {
> 4365
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4365>
>         *max_memkb = strtoul(staticmax, &endptr, 10);
> 4366
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4366>
>         if (*endptr != '\0') {
> 4367
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4367>
>             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> 4368
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4368>
>                              "invalid memory static-max %s from %s\n",
> 4369
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4369>
>                              staticmax, max_path);
> 4370
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4370>
>             rc = ERROR_FAIL;
> 4371
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4371>
>             goto out;
> 4372
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4372>
>         }
> 4373
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4373>
>     }
> 4374
> <http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl.c;h=9c72df27d988d7a4c8f6058708daa16d72be084d;hb=HEAD#l4374>
>
>
> Regards,
> Ayush Ruia.
>

[-- Attachment #1.2: Type: text/html, Size: 23068 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: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Wei Liu]

Please send plain text email in the future. Some (if not all) developers
only have very shabby text editor to read / reply to your email. ;-)

On Thu, Oct 09, 2014 at 08:28:53PM -0500, ayush ruia wrote:
> What I am trying to say is that shouldn't the code block highlighted in
> yellow should come before the block marked in green. Then it would update
> the value of *target_memkb and *max_memkb in all possible situations.
> 

I don't think so. This function means "if there's no such values in
xenstore then retrieve them from hypervisor and fill them in xenstore,
optionally return those values to the caller".

So if those values already are present in xenstore this function doesn't
need to do anything.

Probably looking at its call sites can help you understand better.

Wei.

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Ian Campbell]

On Fri, 2014-10-10 at 10:06 +0100, Wei Liu wrote:
> Please send plain text email in the future. Some (if not all) developers
> only have very shabby text editor to read / reply to your email. ;-)
> 
> On Thu, Oct 09, 2014 at 08:28:53PM -0500, ayush ruia wrote:
> > What I am trying to say is that shouldn't the code block highlighted in
> > yellow should come before the block marked in green. Then it would update
> > the value of *target_memkb and *max_memkb in all possible situations.
> > 
> 
> I don't think so. This function means "if there's no such values in
> xenstore then retrieve them from hypervisor and fill them in xenstore,
> optionally return those values to the caller".
> 
> So if those values already are present in xenstore this function doesn't
> need to do anything.

If I call this function and receive a return code of zero how can I tell
if the target_memkb pointer I passed has been initialised or not?

If all of target, staticmax and freememslack are already set the
function returns 0 without updating those pointers, so I don't think we
can tell.

It does seem like the 
 if (target && staticmax && freememslack) {
        rc = 0;
        goto out;
    }
belongs after the code which writes back the two variables.

Ian.

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Wei Liu]

On Fri, Oct 10, 2014 at 10:17:15AM +0100, Ian Campbell wrote:
> On Fri, 2014-10-10 at 10:06 +0100, Wei Liu wrote:
> > Please send plain text email in the future. Some (if not all) developers
> > only have very shabby text editor to read / reply to your email. ;-)
> > 
> > On Thu, Oct 09, 2014 at 08:28:53PM -0500, ayush ruia wrote:
> > > What I am trying to say is that shouldn't the code block highlighted in
> > > yellow should come before the block marked in green. Then it would update
> > > the value of *target_memkb and *max_memkb in all possible situations.
> > > 
> > 
> > I don't think so. This function means "if there's no such values in
> > xenstore then retrieve them from hypervisor and fill them in xenstore,
> > optionally return those values to the caller".
> > 
> > So if those values already are present in xenstore this function doesn't
> > need to do anything.
> 
> If I call this function and receive a return code of zero how can I tell
> if the target_memkb pointer I passed has been initialised or not?
> 
> If all of target, staticmax and freememslack are already set the
> function returns 0 without updating those pointers, so I don't think we
> can tell.
> 

The way the callers use it prevents the issue you described from
happening -- they only call this function when they can't read those
values from xenstore -- if those values are already in xenstore this
function won't get called.

> It does seem like the 
>  if (target && staticmax && freememslack) {
>         rc = 0;
>         goto out;
>     }
> belongs after the code which writes back the two variables.
> 

I agree this is not nice and there's room for refactoring.

Wei.

> Ian.

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Ian Campbell]

On Fri, 2014-10-10 at 10:24 +0100, Wei Liu wrote:
> On Fri, Oct 10, 2014 at 10:17:15AM +0100, Ian Campbell wrote:
> > On Fri, 2014-10-10 at 10:06 +0100, Wei Liu wrote:
> > > Please send plain text email in the future. Some (if not all) developers
> > > only have very shabby text editor to read / reply to your email. ;-)
> > > 
> > > On Thu, Oct 09, 2014 at 08:28:53PM -0500, ayush ruia wrote:
> > > > What I am trying to say is that shouldn't the code block highlighted in
> > > > yellow should come before the block marked in green. Then it would update
> > > > the value of *target_memkb and *max_memkb in all possible situations.
> > > > 
> > > 
> > > I don't think so. This function means "if there's no such values in
> > > xenstore then retrieve them from hypervisor and fill them in xenstore,
> > > optionally return those values to the caller".
> > > 
> > > So if those values already are present in xenstore this function doesn't
> > > need to do anything.
> > 
> > If I call this function and receive a return code of zero how can I tell
> > if the target_memkb pointer I passed has been initialised or not?
> > 
> > If all of target, staticmax and freememslack are already set the
> > function returns 0 without updating those pointers, so I don't think we
> > can tell.
> > 
> 
> The way the callers use it prevents the issue you described from
> happening -- they only call this function when they can't read those
> values from xenstore -- if those values are already in xenstore this
> function won't get called.

At least the callers in libxl__get_free_memory_slack and
libxl__get_memory_target look to be racy with someone else writing these
nodes to me.

libxl_set_memory_target is a bit unclear but the fact that it ends the
transaction right before it calls libxl__fill_dom0_memory_info seems
suspicious.

libxl__get_free_memory_slack is also separately suspicious because it
never uses the values anyway. I suppose just because
libxl__fill_dom0_memory_info doesn't tolerate NULL arguments like I
think it should.

Ian.

> 
> > It does seem like the 
> >  if (target && staticmax && freememslack) {
> >         rc = 0;
> >         goto out;
> >     }
> > belongs after the code which writes back the two variables.
> > 
> 
> I agree this is not nice and there's room for refactoring.
> 
> Wei.
> 
> > Ian.

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Wei Liu]

On Fri, Oct 10, 2014 at 10:30:06AM +0100, Ian Campbell wrote:
[...]
> > 
> > The way the callers use it prevents the issue you described from
> > happening -- they only call this function when they can't read those
> > values from xenstore -- if those values are already in xenstore this
> > function won't get called.
> 
> At least the callers in libxl__get_free_memory_slack and
> libxl__get_memory_target look to be racy with someone else writing these
> nodes to me.
> 

 libxl__fill_dom0_memory_info uses transaction already, that should
 avoid race?

> libxl_set_memory_target is a bit unclear but the fact that it ends the
> transaction right before it calls libxl__fill_dom0_memory_info seems
> suspicious.
> 

To avoid having a transaction inside another transaction?

> libxl__get_free_memory_slack is also separately suspicious because it
> never uses the values anyway. I suppose just because
> libxl__fill_dom0_memory_info doesn't tolerate NULL arguments like I
> think it should.
> 

*free_mem_slask is populated with that value, after
libxl__fill_dom0_memory_info successfully writes that value into
xenstore (oddly it's not returned with an out parameter).

TBH I have no idea why libxl__fill_dom0_memory_info is used in such
convoluted way...

Wei.

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Ian Campbell]

On Fri, 2014-10-10 at 11:54 +0100, Wei Liu wrote:
> On Fri, Oct 10, 2014 at 10:30:06AM +0100, Ian Campbell wrote:
> [...]
> > > 
> > > The way the callers use it prevents the issue you described from
> > > happening -- they only call this function when they can't read those
> > > values from xenstore -- if those values are already in xenstore this
> > > function won't get called.
> > 
> > At least the callers in libxl__get_free_memory_slack and
> > libxl__get_memory_target look to be racy with someone else writing these
> > nodes to me.
> > 
> 
>  libxl__fill_dom0_memory_info uses transaction already, that should
>  avoid race?

I don't think so.

The caller is:
	read target (perhaps in a transaction)
	if (!target)	
		***
		if ( call fill_dom0_memory(&target) < 0 ) /* uses a transaction */
			return
		use target
	}

If another process dives in at *** and writes target then
fill_dom0_memory will return 0 but not initialise target, but the caller
will still (potentially) use it.

Even if it somehow transpires that through some careful coding in the
caller the use target never actually happens because of some other
reason it way to opaque as it is currently written.

I think the initialisation of current_target_memkb to zero in 
libxl_set_memory_target might be masking the issue here, preventing the
caller from (legitimately) complaining about a variable which is used
without initialisation

> > libxl_set_memory_target is a bit unclear but the fact that it ends the
> > transaction right before it calls libxl__fill_dom0_memory_info seems
> > suspicious.
> > 
> 
> To avoid having a transaction inside another transaction?

I suppose that might have been what the author was thinking, bit it
means that help if you are relying on that transaction to provide
atomicity you lose.

Ian

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[ayush ruia]

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

There is one issue which I could not understand -- notably do I want the
caller's of these functions to be one single transactions. As it works now,
in most of the cases the caller first tries to read the value  from the
xenstore, if it is not present then it will call the function
libxl__fill_dom0_memory. After the call is successful, the caller function
will again try to read the value from xenstore . Would it be an issue if
someone went inbetween and modified the value of target, static_max or
freemem_slack and there are different values in target during the
invocation of the function libxl__fill_dom0_memory and when re-reading the
value from xenstore? I am assuming that is not a problem.

There are mainly three callers of the function libxl__fill_dom0_memory_info
in the code.

1. libxl__get_free_memory_slack -- Here we read the value from the xenstore
again after the function successfully returns. This might be a little
redundant, because we are reading the value again from xenstore, which we
have already read once and "ideally" should have the value stored in the
pointer.
2. libxl_set_memory_target -- Pretty much the same as above. Here
re-reading the value from the xenstore might make sense as then it starts
the transaction afresh from reading the value from xenstore again. This
comes back to my question above -- it seems that the function possibly
wants to complete the the whole set of calculations and set the
memory_target in just one transaction, from when it read the value of
target from xenstore.

3. libxl__get_memory_target -- This is the only function that actually uses
the values of the variables passed by reference, and does not re-read it
from xenstore again.

4593 target = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
4594                     "%s/memory/target", dompath));
4595     static_max = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
4596                     "%s/memory/static-max", dompath));
4597
4598     rc = ERROR_FAIL;
4599     if ((!target || !static_max) && !domid) {
4600         rc = libxl__fill_dom0_memory_info(gc, &target_memkb,
4601                                           &max_memkb);
4602         if (rc < 0)
4603             goto out;
This seems to be plagued with the issue Ian described above. Someone could
update the value of target or static_max inbetween line 4593 and line 4599.
This could cause invalid values to be present in the pointers target_memkb
and max_memkb.


Out of the three callers of the function libxl__fill_dom0_memory_info(),
two re-read the value again from the xenstore, and only one uses the
variables passes by reference-- which might have a race condition.

Regards,
Ayush Ruia.

On Fri, Oct 10, 2014 at 6:30 AM, Ian Campbell <Ian.Campbell@citrix.com>
wrote:

> On Fri, 2014-10-10 at 11:54 +0100, Wei Liu wrote:
> > On Fri, Oct 10, 2014 at 10:30:06AM +0100, Ian Campbell wrote:
> > [...]
> > > >
> > > > The way the callers use it prevents the issue you described from
> > > > happening -- they only call this function when they can't read those
> > > > values from xenstore -- if those values are already in xenstore this
> > > > function won't get called.
> > >
> > > At least the callers in libxl__get_free_memory_slack and
> > > libxl__get_memory_target look to be racy with someone else writing
> these
> > > nodes to me.
> > >
> >
> >  libxl__fill_dom0_memory_info uses transaction already, that should
> >  avoid race?
>
> I don't think so.
>
> The caller is:
>         read target (perhaps in a transaction)
>         if (!target)
>                 ***
>                 if ( call fill_dom0_memory(&target) < 0 ) /* uses a
> transaction */
>                         return
>                 use target
>         }
>
> If another process dives in at *** and writes target then
> fill_dom0_memory will return 0 but not initialise target, but the caller
> will still (potentially) use it.
>
> Even if it somehow transpires that through some careful coding in the
> caller the use target never actually happens because of some other
> reason it way to opaque as it is currently written.
>
> I think the initialisation of current_target_memkb to zero in
> libxl_set_memory_target might be masking the issue here, preventing the
> caller from (legitimately) complaining about a variable which is used
> without initialisation
>
> > > libxl_set_memory_target is a bit unclear but the fact that it ends the
> > > transaction right before it calls libxl__fill_dom0_memory_info seems
> > > suspicious.
> > >
> >
> > To avoid having a transaction inside another transaction?
>
> I suppose that might have been what the author was thinking, bit it
> means that help if you are relying on that transaction to provide
> atomicity you lose.
>
> Ian
>
>

[-- Attachment #1.2: Type: text/html, Size: 7921 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: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Wei Liu]

On Fri, Oct 10, 2014 at 07:43:10AM -0500, ayush ruia wrote:
> There is one issue which I could not understand -- notably do I want the
> caller's of these functions to be one single transactions. 

Is this a question? It doesn't end with question mark but start with
"do". :-)

Currently they are using different transactions. I think it's incorrect,
and using one transaction is better.

> As it works now,
> in most of the cases the caller first tries to read the value  from the
> xenstore, if it is not present then it will call the function
> libxl__fill_dom0_memory. After the call is successful, the caller function
> will again try to read the value from xenstore . Would it be an issue if
> someone went inbetween and modified the value of target, static_max or
> freemem_slack and there are different values in target during the
> invocation of the function libxl__fill_dom0_memory and when re-reading the
> value from xenstore? I am assuming that is not a problem.
> 

It's similar to the race Ian described. It is a problem, but probably a
benign one, less harmful than the one Ian spotted. But it still needs to
get fixed.

> There are mainly three callers of the function libxl__fill_dom0_memory_info
> in the code.
> 
> 1. libxl__get_free_memory_slack -- Here we read the value from the xenstore
> again after the function successfully returns. This might be a little
> redundant, because we are reading the value again from xenstore, which we
> have already read once and "ideally" should have the value stored in the
> pointer.

Given the current implementation (that libxl__fill_dom0_memory_info has
its own transaction, by the time it returns the value is guaranteed to
be present in xenstore), it's a bit odd libxl__fill_dom0_memory_info
doesn't just return that value in out parameter and the caller just uses
it.

But if you want to make libxl__fill_dom0_memory_info share the same
transaction with its caller this pattern might be still valid.

> 2. libxl_set_memory_target -- Pretty much the same as above. Here
> re-reading the value from the xenstore might make sense as then it starts
> the transaction afresh from reading the value from xenstore again. This
> comes back to my question above -- it seems that the function possibly
> wants to complete the the whole set of calculations and set the
> memory_target in just one transaction, from when it read the value of
> target from xenstore.
> 
> 3. libxl__get_memory_target -- This is the only function that actually uses
> the values of the variables passed by reference, and does not re-read it
> from xenstore again.
> 
> 4593 target = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
> 4594                     "%s/memory/target", dompath));
> 4595     static_max = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
> 4596                     "%s/memory/static-max", dompath));
> 4597
> 4598     rc = ERROR_FAIL;
> 4599     if ((!target || !static_max) && !domid) {
> 4600         rc = libxl__fill_dom0_memory_info(gc, &target_memkb,
> 4601                                           &max_memkb);
> 4602         if (rc < 0)
> 4603             goto out;
> This seems to be plagued with the issue Ian described above. Someone could
> update the value of target or static_max inbetween line 4593 and line 4599.
> This could cause invalid values to be present in the pointers target_memkb
> and max_memkb.
> 
> 
> Out of the three callers of the function libxl__fill_dom0_memory_info(),
> two re-read the value again from the xenstore, and only one uses the
> variables passes by reference-- which might have a race condition.
> 

This is bad.  The way I see to fix this:

1. Refacotr libxl__fill_dom0_memory_info so that it returns
   freemem_slack in an out parameter.
2. Make libxl__fill_dom0_memory_info share the same transaction with its
   caller.
3. Fix all callers to adapt to the new libxl__fill_dom0_memory_info,
   which includes but not limits to change that "goto" style loop to a
   proper loop.

caller ()
{
   for (;;) {
     start transaction
     if domid == 0 {
       if those values don't exist in xenstore {
          fill in dom0 info
       }
     }
     domU path
     commit transaction
     break if success or fatal error
   }
}

It's going to be a small patch series. If you're up for writing any
code, feel free to ask more questions before actually committing serious
efforts to tackle the problem.

Wei.

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[ayush ruia]

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

That was a question. Thanks for answering it :). I think I am going to
wait, if you are thinking of changing the function definition. :)

Regards,
Ayush Ruia.

On Fri, Oct 10, 2014 at 8:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:

> On Fri, Oct 10, 2014 at 07:43:10AM -0500, ayush ruia wrote:
> > There is one issue which I could not understand -- notably do I want the
> > caller's of these functions to be one single transactions.
>
> Is this a question? It doesn't end with question mark but start with
> "do". :-)
>
> Currently they are using different transactions. I think it's incorrect,
> and using one transaction is better.
>
> > As it works now,
> > in most of the cases the caller first tries to read the value  from the
> > xenstore, if it is not present then it will call the function
> > libxl__fill_dom0_memory. After the call is successful, the caller
> function
> > will again try to read the value from xenstore . Would it be an issue if
> > someone went inbetween and modified the value of target, static_max or
> > freemem_slack and there are different values in target during the
> > invocation of the function libxl__fill_dom0_memory and when re-reading
> the
> > value from xenstore? I am assuming that is not a problem.
> >
>
> It's similar to the race Ian described. It is a problem, but probably a
> benign one, less harmful than the one Ian spotted. But it still needs to
> get fixed.
>
> > There are mainly three callers of the function
> libxl__fill_dom0_memory_info
> > in the code.
> >
> > 1. libxl__get_free_memory_slack -- Here we read the value from the
> xenstore
> > again after the function successfully returns. This might be a little
> > redundant, because we are reading the value again from xenstore, which we
> > have already read once and "ideally" should have the value stored in the
> > pointer.
>
> Given the current implementation (that libxl__fill_dom0_memory_info has
> its own transaction, by the time it returns the value is guaranteed to
> be present in xenstore), it's a bit odd libxl__fill_dom0_memory_info
> doesn't just return that value in out parameter and the caller just uses
> it.
>
> But if you want to make libxl__fill_dom0_memory_info share the same
> transaction with its caller this pattern might be still valid.
>
> > 2. libxl_set_memory_target -- Pretty much the same as above. Here
> > re-reading the value from the xenstore might make sense as then it starts
> > the transaction afresh from reading the value from xenstore again. This
> > comes back to my question above -- it seems that the function possibly
> > wants to complete the the whole set of calculations and set the
> > memory_target in just one transaction, from when it read the value of
> > target from xenstore.
> >
> > 3. libxl__get_memory_target -- This is the only function that actually
> uses
> > the values of the variables passed by reference, and does not re-read it
> > from xenstore again.
> >
> > 4593 target = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
> > 4594                     "%s/memory/target", dompath));
> > 4595     static_max = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
> > 4596                     "%s/memory/static-max", dompath));
> > 4597
> > 4598     rc = ERROR_FAIL;
> > 4599     if ((!target || !static_max) && !domid) {
> > 4600         rc = libxl__fill_dom0_memory_info(gc, &target_memkb,
> > 4601                                           &max_memkb);
> > 4602         if (rc < 0)
> > 4603             goto out;
> > This seems to be plagued with the issue Ian described above. Someone
> could
> > update the value of target or static_max inbetween line 4593 and line
> 4599.
> > This could cause invalid values to be present in the pointers
> target_memkb
> > and max_memkb.
> >
> >
> > Out of the three callers of the function libxl__fill_dom0_memory_info(),
> > two re-read the value again from the xenstore, and only one uses the
> > variables passes by reference-- which might have a race condition.
> >
>
> This is bad.  The way I see to fix this:
>
> 1. Refacotr libxl__fill_dom0_memory_info so that it returns
>    freemem_slack in an out parameter.
> 2. Make libxl__fill_dom0_memory_info share the same transaction with its
>    caller.
> 3. Fix all callers to adapt to the new libxl__fill_dom0_memory_info,
>    which includes but not limits to change that "goto" style loop to a
>    proper loop.
>
> caller ()
> {
>    for (;;) {
>      start transaction
>      if domid == 0 {
>        if those values don't exist in xenstore {
>           fill in dom0 info
>        }
>      }
>      domU path
>      commit transaction
>      break if success or fatal error
>    }
> }
>
> It's going to be a small patch series. If you're up for writing any
> code, feel free to ask more questions before actually committing serious
> efforts to tackle the problem.
>
> Wei.
>

[-- Attachment #1.2: Type: text/html, Size: 6041 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: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Ian Campbell]

On Fri, 2014-10-10 at 09:47 -0500, ayush ruia wrote:
> That was a question. Thanks for answering it :). I think I am going to
> wait, if you are thinking of changing the function definition. :)

I think Wei was hoping you might be interested in taking a run at a fix
and offering to mentor...

Re: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[ayush ruia]

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

Yes sure, I am interested in a fix. :)

Regards,
Ayush Ruia.

On Fri, Oct 10, 2014 at 10:27 AM, Ian Campbell <Ian.Campbell@citrix.com>
wrote:

> On Fri, 2014-10-10 at 09:47 -0500, ayush ruia wrote:
> > That was a question. Thanks for answering it :). I think I am going to
> > wait, if you are thinking of changing the function definition. :)
>
> I think Wei was hoping you might be interested in taking a run at a fix
> and offering to mentor...
>
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 851 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: Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Ian Campbell]

On Fri, 2014-10-10 at 11:41 -0500, ayush ruia wrote:
> Yes sure, I am interested in a fix. :)

Excellent, looking forward to your patches.

Ian

> 
> Regards,
> Ayush Ruia.
> 
> 
> On Fri, Oct 10, 2014 at 10:27 AM, Ian Campbell
> <Ian.Campbell@citrix.com> wrote:
>         On Fri, 2014-10-10 at 09:47 -0500, ayush ruia wrote:
>         > That was a question. Thanks for answering it :). I think I
>         am going to
>         > wait, if you are thinking of changing the function
>         definition. :)
>         
>         I think Wei was hoping you might be interested in taking a run
>         at a fix
>         and offering to mentor...
>         
>         
>         
> 
>