Re: [PATCH] drivers:vfio: make the logic cleaner with braket

[Alex Williamson]

On Tue,  8 Mar 2022 17:49:46 +0800
jianchunfu <jianchunfu@xxxxxxxxxxxxxxxxxxxx> wrote:

> Use braket to avoid identifying operators in function
> vfio_iova_dirty_bitmap() and vfio_dma_do_unmap()
> when there are too many field values.

s/braket/bracket/ but we're actually adding parenthesis.

"to avoid identifying operators", to avoid confusing operators?

s/function/functions/ but this only lists two of the three.

How many are too many field values?  Per this patch, apparently one?
These are not particularly confusing or unruly tests imo.  Thanks,

Alex


> Signed-off-by: jianchunfu <jianchunfu@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 9394aa944..199547012 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1251,7 +1251,7 @@ static int vfio_iova_dirty_bitmap(u64 __user *bitmap, struct vfio_iommu *iommu,
>  		return -EINVAL;
>  
>  	dma = vfio_find_dma(iommu, iova + size - 1, 0);
> -	if (dma && dma->iova + dma->size != iova + size)
> +	if (dma && (dma->iova + dma->size) != (iova + size))
>  		return -EINVAL;
>  
>  	for (n = rb_first(&iommu->dma_list); n; n = rb_next(n)) {
> @@ -1363,7 +1363,7 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
>  			goto unlock;
>  
>  		dma = vfio_find_dma(iommu, iova + size - 1, 0);
> -		if (dma && dma->iova + dma->size != iova + size)
> +		if (dma && (dma->iova + dma->size) != (iova + size))
>  			goto unlock;
>  	}
>  
> @@ -2958,7 +2958,7 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,
>  			ret = -EINVAL;
>  			goto out_unlock;
>  		}
> -		if (!range.size || range.size & (iommu_pgsize - 1)) {
> +		if (!range.size || (range.size & (iommu_pgsize - 1))) {
>  			ret = -EINVAL;
>  			goto out_unlock;
>  		}