diff mbox series

[1/1] iommu/dma: fix trival coding style mistake

Message ID 1527055369-13192-1-git-send-email-thunder.leizhen@huawei.com
State New
Headers show
Series [1/1] iommu/dma: fix trival coding style mistake | expand

Commit Message

Zhen Lei May 23, 2018, 6:02 a.m. UTC
No functional changes.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

---
 drivers/iommu/dma-iommu.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--
1.8.3

Comments

Robin Murphy May 23, 2018, 12:51 p.m. UTC | #1
On 23/05/18 07:02, Zhen Lei wrote:
> No functional changes.


What's the mistake?

> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

> ---

>   drivers/iommu/dma-iommu.c | 12 +++++++-----

>   1 file changed, 7 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c

> index ddcbbdb..4e885f7 100644

> --- a/drivers/iommu/dma-iommu.c

> +++ b/drivers/iommu/dma-iommu.c

> @@ -231,6 +231,9 @@ static int iova_reserve_iommu_regions(struct device *dev,

>   	LIST_HEAD(resv_regions);

>   	int ret = 0;

> 

> +	if (!dev)

> +		return 0;


Logically, it makes no sense at all to call this function without a 
valid device; doing the check in init_domain was a deliberate decision 
to reflect that. This isn't a cleanup path shared by multiple callers 
where the "accept NULL for simplicity" argument might apply.

> +

>   	if (dev_is_pci(dev))

>   		iova_reserve_pci_windows(to_pci_dev(dev), iovad);

> 

> @@ -246,11 +249,12 @@ static int iova_reserve_iommu_regions(struct device *dev,

>   		hi = iova_pfn(iovad, region->start + region->length - 1);

>   		reserve_iova(iovad, lo, hi);

> 

> -		if (region->type == IOMMU_RESV_MSI)

> +		if (region->type == IOMMU_RESV_MSI) {

>   			ret = cookie_init_hw_msi_region(cookie, region->start,

>   					region->start + region->length);

> -		if (ret)

> -			break;

> +			if (ret)

> +				break;

> +		}


Why? ret is already initialised appropriately, and the coding style even 
says that going beyond 3 levels of indentation is undesirable...

Robin.

>   	}

>   	iommu_put_resv_regions(dev, &resv_regions);

> 

> @@ -308,8 +312,6 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,

>   	}

> 

>   	init_iova_domain(iovad, 1UL << order, base_pfn);

> -	if (!dev)

> -		return 0;

> 

>   	return iova_reserve_iommu_regions(dev, domain);

>   }

> --

> 1.8.3

> 

> 

> _______________________________________________

> iommu mailing list

> iommu@lists.linux-foundation.org

> https://lists.linuxfoundation.org/mailman/listinfo/iommu

>
diff mbox series

Patch

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index ddcbbdb..4e885f7 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -231,6 +231,9 @@  static int iova_reserve_iommu_regions(struct device *dev,
 	LIST_HEAD(resv_regions);
 	int ret = 0;

+	if (!dev)
+		return 0;
+
 	if (dev_is_pci(dev))
 		iova_reserve_pci_windows(to_pci_dev(dev), iovad);

@@ -246,11 +249,12 @@  static int iova_reserve_iommu_regions(struct device *dev,
 		hi = iova_pfn(iovad, region->start + region->length - 1);
 		reserve_iova(iovad, lo, hi);

-		if (region->type == IOMMU_RESV_MSI)
+		if (region->type == IOMMU_RESV_MSI) {
 			ret = cookie_init_hw_msi_region(cookie, region->start,
 					region->start + region->length);
-		if (ret)
-			break;
+			if (ret)
+				break;
+		}
 	}
 	iommu_put_resv_regions(dev, &resv_regions);

@@ -308,8 +312,6 @@  int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 	}

 	init_iova_domain(iovad, 1UL << order, base_pfn);
-	if (!dev)
-		return 0;

 	return iova_reserve_iommu_regions(dev, domain);
 }