Re: [Xen-devel] [PATCH] xen/x86: Use min macro instead of ternary operator

[Julien Grall <julien@xxxxxxx>]

Hi,

On 29/03/2020 08:22, Simran Singhal wrote:
> Replace ternary operator with macro min as it is shorter and
> thus increases code readability. Macro min return the minimum of the
> two compared values.
While I understand the ternary operator is doing exactly the same as a 
min(), I read the current code as "If there is an error then return the 
error code, otherwise return 0".
This is quite different from the meaning of "min" which is "I want the 
minimum of the two values". Therefore I am not convinced using min() is 
the right thing to do.
> Signed-off-by: Simran Singhal <singhalsimran0@xxxxxxxxx>
> ---
>   xen/arch/x86/bzimage.c    | 2 +-
>   xen/arch/x86/mm.c         | 2 +-
>   xen/arch/x86/mm/p2m-ept.c | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/bzimage.c b/xen/arch/x86/bzimage.c
> index ac4fd428be..f396aa3445 100644
> --- a/xen/arch/x86/bzimage.c
> +++ b/xen/arch/x86/bzimage.c
> @@ -136,7 +136,7 @@ int __init bzimage_parse(void *image_base, void 
> **image_start,
>           *image_len = output_len;
>       }
>   
> -    return err > 0 ? 0 : err;
> +    return min(0, err);
>   }
>   
>   /*
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 2fac67ad57..c7617f80e8 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1988,7 +1988,7 @@ static int demote_l3_table(struct page_info *page)
>           page->partial_flags = partial_flags;
>           rc = -ERESTART;
>       }
> -    return rc > 0 ? 0 : rc;
> +    return min(0, rc);
>   }
>   
>   static int demote_l4_table(struct page_info *page)
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index eb0f0edfef..38faa4df52 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1069,7 +1069,7 @@ static int ept_change_entry_type_range(struct p2m_domain 
> *p2m,
>       if ( sync )
>           ept_sync_domain(p2m);
>   
> -    return rc < 0 ? rc : 0;
> +    return min(rc, 0);
>   }
>   
>   static void ept_memory_type_changed(struct p2m_domain *p2m)
--
Julien Grall