Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()

[Harmandeep Kaur <write.harmandeep@xxxxxxxxx>]

On Fri, Feb 12, 2016 at 4:30 PM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
> On Fri, 2016-02-12 at 16:22 +0530, Harmandeep Kaur wrote:
>> On Thu, Feb 11, 2016 at 3:18 PM, Dario Faggioli
>> <dario.faggioli@xxxxxxxxxx> wrote:
>> >
>> > Another thing that you should check, and probably quickly mention
>> > too,
>> > is whether or not the callers of these functions --the ones inside
>> > xen.git of course-- are prepared to deal with this. I mean,
>> > returning
>> > NULL on failure of xc_version() is, IMO, the right thing to do here
>> > anyway, but it is something important to know whether more work is
>> > needed to fix the callers as well, or if we're just fine.
>> >
>> > To do so, search (e.g., with grep or cscope) for uses of
>> > libxl_get_version_info inside the tools/ dir of xen.git. For
>> > instance,
>> > there is one such use in xl_cmdimpl.c, in the output_xeninfo()
>> > function, which looks like it would interact well with this
>> > patch...
>> > Can you confirm this is the case also for all the other instances
>> > and
>> > note it down here?
>>
>> I think NULL may not be handled properly in auto_autoballoon() in
>> tools/libxl/xl.c Other instances seems okay to me.
>>
> So, here:
>
>     info = libxl_get_version_info(ctx);
>     if (!info)
>         return 1; /* default to on */
>
> Why do you think this is a problem? It looks like this call site is
> actually prepared to see and handle a NULL return value...
>

Sorry I actually meant,
tools/libxl/xl_cmdimpl.c:    vinfo = libxl_get_version_info(ctx);

vinfo = libxl_get_version_info(ctx);
    if (vinfo) {
        i = (1 << 20) / vinfo->pagesize;
        printf("total_memory           : %"PRIu64"\n", info.total_pages / i);
        printf("free_memory            : %"PRIu64"\n",
(info.free_pages - info.outstanding_pages) / i);
        printf("sharing_freed_memory   : %"PRIu64"\n",
info.sharing_freed_pages / i);
        printf("sharing_used_memory    : %"PRIu64"\n",
info.sharing_used_frames / i);
        printf("outstanding_claims     : %"PRIu64"\n",
info.outstanding_pages / i);
    }

Regards.

>> Should I fix the caller, too?
>>
> If there are issues with them, yes. If there aren't just put something
> like "xxx callers are already fine xxx" in the changelog.
>
> 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)
>

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