Re: [PATCH 04/14] vfio: factor out a vfio_group_find_or_alloc helper

From: Christoph Hellwig <hch@lst.de>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Christoph Hellwig <hch@lst.de>,
	Alex Williamson <alex.williamson@redhat.com>,
	Diana Craciun <diana.craciun@oss.nxp.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Eric Auger <eric.auger@redhat.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH 04/14] vfio: factor out a vfio_group_find_or_alloc helper
Date: Wed, 25 Aug 2021 07:30:13 +0200	[thread overview]
Message-ID: <20210825053013.GA26806@lst.de> (raw)
In-Reply-To: <20210824234050.GK543798@ziepe.ca>

On Tue, Aug 24, 2021 at 08:40:50PM -0300, Jason Gunthorpe wrote:
> I think the non-"success oriented flow" is less readable than what was
> before. It is jarring to see, and I was about to say this is not
> logically the same having missed the !...
> 
> How about:
> 
>         /* If found group already holds the iommu_group reference */
> 	group = vfio_group_get_from_iommu(iommu_group);
> 	if (group)
>             goto out_put;
> 
> 	group = vfio_create_group(iommu_group);
> 	if (IS_ERR(group))
> 	     goto out_put;
>         /* If created our iommu_group reference was moved to group, keep it */
> 	return group;
> 
> out_put:
> 	vfio_iommu_group_put(iommu_group, dev);
> 	return group;

Fine with me.  I actually repeatedly switched between the two flows
because I couldn't decide on one.