Re: [PATCH][4.15] gnttab: work around "may be used uninitialized" warning

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/03/2021 13:18, Jan Beulich wrote:
> On 12.03.2021 14:08, Jan Beulich wrote:
>> On 12.03.2021 12:32, Andrew Cooper wrote:
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -4059,6 +4059,16 @@ int gnttab_acquire_resource(
>>>      if ( rc )
>>>          goto out;
>>>  
>>> +    /*
>>> +     * Some older toolchains can't spot that vaddrs is non-NULL on
>>> non-error
>>> +     * paths.  Leave some runtime safety.
>>> +     */
>>> +    if ( !vaddrs )
>>> +    {
>>> +        ASSERT_UNREACHABLE();
>>> +        goto out;
>>> +    }
>>> +
>>>      for ( i = 0; i < nr_frames; ++i )
>>>          mfn_list[i] = virt_to_mfn(vaddrs[frame + i]);
>> Oh, I didn't realize this. Will add, but did you really mean to
>> have the function return success in this case (on a release
>> build)? I'd be inclined to put it ahead of if "if ( rc )" and
>> set rc (to e.g. -ENODATA) in this case.
> But I think the comment isn't really correct - the problem isn't
> NULL or not, but uninitialized without setting it to NULL. How
> about
>
>     /*
>      * Some older toolchains can't spot that vaddrs won't remain uninitialized
>      * on non-error paths, and hence it needs setting to NULL at the top of the
>      * function.  Leave some runtime safety.
>      */
>
> ?

Yes - that's fine.

~Andrew