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

2018-05-23 Thread Robin Murphy

On 23/05/18 07:02, Zhen Lei wrote:

No functional changes.


What's the mistake?


Signed-off-by: Zhen Lei 
---
  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, _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


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2018-05-23 Thread Zhen Lei
No functional changes.

Signed-off-by: Zhen Lei 
---
 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;
+
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, _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