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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.
     */

?

Jan