Re: [XEN PATCH] xen/arm: p2m: refactor 'p2m_get_entry'

[Julien Grall <julien@xxxxxxx>]

Hi Nicola,

I would add "to please ECLAIR" in the commit title.

On 21/07/2023 07:49, Nicola Vetrini wrote:
> This function is refactored to avoid using a
> local dummy variable that served as a fallback
> if the parameter 't' is NULL.
>
> Storing the address of that variable into 't' caused
> static analysis tools not to be able to recognize the
Can you mention which static analysis tools is not happy and the 
version? This could help us in the future if we decided to revert the patch.
> validity of the initialization, and to help with automatic
> checking the two usages of 't' have been slightly refactored.
>
> No functional changes.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> Suggested-by: Julien Grall <julien@xxxxxxx>
> ---
>   xen/arch/arm/p2m.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index de32a2d638..05d65db01b 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -496,16 +496,13 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
>       lpae_t entry, *table;
>       int rc;
>       mfn_t mfn = INVALID_MFN;
> -    p2m_type_t _t;
>       DECLARE_OFFSETS(offsets, addr);
>   
>       ASSERT(p2m_is_locked(p2m));
>       BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
>   
> -    /* Allow t to be NULL */
> -    t = t ?: &_t;
> -
> -    *t = p2m_invalid;
> +    if ( t )
> +        *t = p2m_invalid;
>   
>       if ( valid )
>           *valid = false;
> @@ -549,7 +546,8 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
>   
>       if ( p2m_is_valid(entry) )
>       {
> -        *t = entry.p2m.type;
> +        if ( t )
> +            *t = entry.p2m.type;
>   
>           if ( a )
>               *a = p2m_mem_access_radix_get(p2m, gfn);
Cheers,

--
Julien Grall