Re: [PATCH iommufd v3 2/9] iommu: Add iommu_group_has_isolated_msi()

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Jan 11, 2023 at 07:19:34PM +0100, Thomas Gleixner wrote:
> On Thu, Jan 05 2023 at 15:33, Jason Gunthorpe wrote:
> > +
> > +	mutex_lock(&group->mutex);
> > +	list_for_each_entry(group_dev, &group->devices, list)
> > +		ret &= msi_device_has_isolated_msi(group_dev->dev) ||
> > +		       device_iommu_capable(group_dev->dev,
> > +					    IOMMU_CAP_INTR_REMAP);
> 
> Nit. This really wants brackets even if they are not required by the
> language. Why?
> 
> Brackets can be omitted for a single line statement in the loop/if path,
> but this
> 
> > +		ret &= msi_device_has_isolated_msi(group_dev->dev) ||
> > +		       device_iommu_capable(group_dev->dev,
> > +					    IOMMU_CAP_INTR_REMAP);
> 
> is visually a multi line statement. So having brackets makes visual
> parsing of this construct way clearer:

Sure, though this is all undone by the last patch which does make it
visually a single statement:

 	list_for_each_entry(group_dev, &group->devices, list)
-		ret &= msi_device_has_isolated_msi(group_dev->dev) ||
-		       device_iommu_capable(group_dev->dev,
-					    IOMMU_CAP_INTR_REMAP);
+		ret &= msi_device_has_isolated_msi(group_dev->dev);

> Also get rid of that extra line break. We lifted the 80 characters line
> length quite some while ago and made it 100.

Ah, there are so many different opinions on that!

I got everything else, I'll stick this in linux-next via the iommufd
tree so more bots can check it - if anyone else has remarks I can make
a v4

Thanks,
Jason