Messages in this thread | | | Date | Mon, 19 Sep 2022 17:34:07 +0200 | From | Miquel Raynal <> | Subject | Re: [PATCH] mtd: rawnand: denali: Fix a possible resource leak in denali_pci_probe |
| |
Hi Ruiqi,
gongruiqi1@huawei.com wrote on Mon, 1 Aug 2022 16:03:15 +0800:
> Call pci_release_regions() to retrieve the allocated resource when > devm_ioremap() or denali_init() failed.
fail > > Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com> > --- > drivers/mtd/nand/raw/denali_pci.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/raw/denali_pci.c b/drivers/mtd/nand/raw/denali_pci.c > index de7e722d3826..40943cda0914 100644 > --- a/drivers/mtd/nand/raw/denali_pci.c > +++ b/drivers/mtd/nand/raw/denali_pci.c > @@ -74,21 +74,22 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > return ret; > } > > + ret = -ENOMEM;
I don't like so much this construction as later changes in the probe might just fail this logic.
Please just set ret = -ENOMEM; in the error path each time it's needed. I don't care about loosing 3 lines of code if it clarifies what the error path returns.
> denali->reg = devm_ioremap(denali->dev, csr_base, csr_len); > if (!denali->reg) { > dev_err(&dev->dev, "Spectra: Unable to remap memory region\n"); > - return -ENOMEM; > + goto out_release_pci; > } > > denali->host = devm_ioremap(denali->dev, mem_base, mem_len); > if (!denali->host) { > dev_err(&dev->dev, "Spectra: ioremap failed!"); > - return -ENOMEM; > + goto out_release_pci; > } > > ret = denali_init(denali); > if (ret) > - return ret; > + goto out_release_pci; > > nsels = denali->nbanks; > > @@ -116,6 +117,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > > out_remove_denali: > denali_remove(denali); > +out_release_pci: > + pci_release_regions(dev); > return ret; > } >
Thanks, Miquèl
| |