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

[Jan Beulich <jbeulich@suse.com>]

On 12.03.2021 12:32, Andrew Cooper wrote:
> On 10/03/2021 10:13, Jan Beulich wrote:
>> Sadly I was wrong to suggest dropping vaddrs' initializer during review
>> of v2 of the patch introducing this code. gcc 4.3 can't cope.
>>
>> Fixes: 52531c734ea1 ("xen/gnttab: Rework resource acquisition")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -4026,7 +4026,7 @@ int gnttab_acquire_resource(
>>      struct grant_table *gt = d->grant_table;
>>      unsigned int i, final_frame;
>>      mfn_t tmp;
>> -    void **vaddrs;
>> +    void **vaddrs = NULL;
>>      int rc = -EINVAL;
>>  
>>      if ( !nr_frames )
> 
> in v1, there was a companion check.
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index f937c1d350..2bb07f129f 100644
> --- 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.

> With this reinstated, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Jan