[PATCH] x86/mm: Simplify expression in set_gpfn_from_mfn()

[PATCH] x86/mm: Simplify expression in set_gpfn_from_mfn()

[Andrew Cooper]

Coverity points out that the "d &&" is redundant.

Fixes: c9476c4ad72 ("x86: don't override INVALID_M2P_ENTRY with SHARED_M2P_ENTRY")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/x86_64/mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 0d1aadbfce..4c0a3a275c 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1336,7 +1336,7 @@ void set_gpfn_from_mfn(unsigned long mfn, unsigned long pfn)
     {
         const struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn)));
 
-        if ( d && (d == dom_cow) )
+        if ( d == dom_cow )
             entry = SHARED_M2P_ENTRY;
     }
 
-- 
2.11.0

Re: [PATCH] x86/mm: Simplify expression in set_gpfn_from_mfn()

[Jan Beulich]

On 15.09.2020 19:53, Andrew Cooper wrote:
> Coverity points out that the "d &&" is redundant.

How can it know that ...

> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1336,7 +1336,7 @@ void set_gpfn_from_mfn(unsigned long mfn, unsigned long pfn)
>      {
>          const struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn)));
>  
> -        if ( d && (d == dom_cow) )
> +        if ( d == dom_cow )

... dom_cow is not NULL (in which case the new expression is wrong
when d also is). I actually think Coverity has spotted the issue
when dom_cow is #define-d to NULL, in which case the original
expression degenerates to "d && !d".

If the purpose of the extra check is more obvious that way, we
could check dom_cow to be non-NULL (instead of d), but I'm afraid
this may not silence Coverity.

Jan