Received-SPF: pass (google.com: domain of linux-kernel+bounces-214921-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Fri, 14 Jun 2024 15:57:49 +0300 (EEST)
To: Dan Carpenter <dan.carpenter@linaro.org>
cc: Frank Li <Frank.Li@nxp.com>, Jon Mason <jdmason@kudzu.us>, 
    Dave Jiang <dave.jiang@intel.com>, Allen Hubbe <allenbh@gmail.com>, 
    Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>, 
    =?ISO-8859-2?Q?Krzysztof_Wilczy=F1ski?= <kw@linux.com>, 
    Kishon Vijay Abraham I <kishon@kernel.org>, 
    Bjorn Helgaas <bhelgaas@google.com>, ntb@lists.linux.dev, 
    linux-pci@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, 
    kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 1/2] PCI: endpoint: Clean up error handling in
 vpci_scan_bus()
In-Reply-To: <68e0f6a4-fd57-45d0-945b-0876f2c8cb86@moroto.mountain>
Message-ID: <d8f510d1-db05-66a8-b379-464fdc817143@linux.intel.com>
References: <68e0f6a4-fd57-45d0-945b-0876f2c8cb86@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1335945687-1718369869=:1013"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1335945687-1718369869=:1013
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Mon, 10 Jun 2024, Dan Carpenter wrote:

> Smatch complains about inconsistent NULL checking in vpci_scan_bus():
>=20
>     drivers/pci/endpoint/functions/pci-epf-vntb.c:1024 vpci_scan_bus()
>     error: we previously assumed 'vpci_bus' could be null (see line 1021)
>=20
> Instead of printing an error message and then crashing we should return
> an error code and clean up.  Also the NULL check is reversed so it
> prints an error for success instead of failure.
>=20
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and =
EP")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/=
endpoint/functions/pci-epf-vntb.c
> index 8e779eecd62d..7f05a44e9a9f 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -1018,8 +1018,10 @@ static int vpci_scan_bus(void *sysdata)
>  =09struct epf_ntb *ndev =3D sysdata;
> =20
>  =09vpci_bus =3D pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
> -=09if (vpci_bus)
> -=09=09pr_err("create pci bus\n");
> +=09if (!vpci_bus) {
> +=09=09pr_err("create pci bus failed\n");
> +=09=09return -EINVAL;
> +=09}
> =20
>  =09pci_bus_add_devices(vpci_bus);
> =20
> @@ -1338,10 +1340,14 @@ static int epf_ntb_bind(struct pci_epf *epf)
>  =09=09goto err_bar_alloc;
>  =09}
> =20
> -=09vpci_scan_bus(ntb);
> +=09ret =3D vpci_scan_bus(ntb);
> +=09if (ret)
> +=09=09goto err_unregister;
> =20
>  =09return 0;
> =20
> +err_unregister:
> +=09pci_unregister_driver(&vntb_pci_driver);
>  err_bar_alloc:
>  =09epf_ntb_config_spad_bar_free(ntb);

Reviewed-by: Ilpo J=E4rvinen <ilpo.jarvinen@linux.intel.com>


--=20
 i.

--8323328-1335945687-1718369869=:1013--