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

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

[Simran Singhal]

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.

Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
---
 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)
-- 
2.17.1

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

[Julien Grall]

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@gmail.com>
> ---
>   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

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

[Wei Liu]

On Sun, Mar 29, 2020 at 12:08:39PM +0100, Julien Grall wrote:
> 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.

I agree with Julien's assessment.

Wei.