linux-kernel - Re: [PATCH] KVM: x86: fix cocci warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87o86leo34.fsf@redhat.com>
Date:   Mon, 15 Nov 2021 10:59:43 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Vihas Mak <makvihas@...il.com>, pbonzini@...hat.com
Cc:     seanjc@...gle.com, wanpengli@...cent.com, jmattson@...gle.com,
        joro@...tes.org, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: fix cocci warnings

Vihas Mak <makvihas@...il.com> writes:

> change 0 to false and 1 to true to fix following cocci warnings:
>
>         arch/x86/kvm/mmu/mmu.c:1485:9-10: WARNING: return of 0/1 in function 'kvm_set_pte_rmapp' with return type bool
>         arch/x86/kvm/mmu/mmu.c:1636:10-11: WARNING: return of 0/1 in function 'kvm_test_age_rmapp' with return type bool
>
> Signed-off-by: Vihas Mak <makvihas@...il.com>
> Cc: Sean Christopherson <seanjc@...gle.com>
> Cc: Vitaly Kuznetsov <vkuznets@...hat.com>
> Cc: Wanpeng Li <wanpengli@...cent.com>
> Cc: Jim Mattson <jmattson@...gle.com>
> Cc: Joerg Roedel <joro@...tes.org>
> ---
>  arch/x86/kvm/mmu/mmu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 337943799..2fcea4a78 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1454,7 +1454,7 @@ static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
>  {
>  	u64 *sptep;
>  	struct rmap_iterator iter;
> -	int need_flush = 0;
> +	bool need_flush = false;
>  	u64 new_spte;
>  	kvm_pfn_t new_pfn;
>  
> @@ -1466,7 +1466,7 @@ static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
>  		rmap_printk("spte %p %llx gfn %llx (%d)\n",
>  			    sptep, *sptep, gfn, level);
>  
> -		need_flush = 1;
> +		need_flush = true;
>  
>  		if (pte_write(pte)) {
>  			pte_list_remove(kvm, rmap_head, sptep);
> @@ -1482,7 +1482,7 @@ static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
>  
>  	if (need_flush && kvm_available_flush_tlb_with_range()) {
>  		kvm_flush_remote_tlbs_with_address(kvm, gfn, 1);
> -		return 0;
> +		return false;
>  	}
>  
>  	return need_flush;
> @@ -1623,8 +1623,8 @@ static bool kvm_test_age_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
>  
>  	for_each_rmap_spte(rmap_head, &iter, sptep)
>  		if (is_accessed_spte(*sptep))
> -			return 1;
> -	return 0;
> +			return true;
> +	return false;
>  }
>  
>  #define RMAP_RECYCLE_THRESHOLD 1000

Reviewed-by: Vitaly Kuznetsov <vkuznets@...hat.com>

One minor remark: 'kvm_set_pte_rmapp()' handler is passed to
'kvm_handle_gfn_range()' which does

        bool ret = false;

        for_each_slot_rmap_range(...)
                ret |= handler(...);

and I find '|=' to not be very natural with booleans. I'm not sure it's
worth changing though.

-- 
Vitaly