Re: [PATCH] iommu/omap: Use helper function IS_ERR_OR_NULL()

Re: [PATCH] iommu/omap: Use helper function IS_ERR_OR_NULL()

[Jason Gunthorpe]

On Thu, Sep 07, 2023 at 03:14:09AM -0400, Bo Liu wrote:
> Use IS_ERR_OR_NULL() to detect an error pointer or a null pointer
> open-coding to simplify the code.
> 
> Signed-off-by: Bo Liu <liubo03@inspur.com>
> ---
>  drivers/iommu/omap-iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please no, IS_ERR_OR_NULL is an abomination.

There are only two callers:

	for (i = 0; i < omap_domain->num_iommus; i++, iommu--, arch_data--) {
		oiommu = iommu->iommu_dev;
		iopgtable_clear_entry_all(oiommu);

		omap_iommu_detach(oiommu);

Obviously oiommu is not NULL or ERR since we derefed it

The second:

attach_fail:
	while (i--) {
		iommu--;
		arch_data--;
		oiommu = iommu->iommu_dev;
		omap_iommu_detach(oiommu);
		iommu->iommu_dev = NULL;
		oiommu->domain = NULL;

And here I don't see how iomm->iommu_dev can ever be NULL or
ERR_PTR. The i-- follows this:

	iommu = omap_domain->iommus;
	for (i = 0; i < omap_domain->num_iommus; i++, iommu++, arch_data++) {
		/* configure and enable the omap iommu */
		oiommu = arch_data->iommu_dev;
		ret = omap_iommu_attach(oiommu, iommu->pgtable);
		if (ret) {
			dev_err(dev, "can't get omap iommu: %d\n", ret);
			goto attach_fail;
		}

		oiommu->domain = domain;
		iommu->iommu_dev = oiommu;

And again we have always deref'd iommu->iommu_dev.

It is just wrong defensive coding, remove it.

Jason