* [PATCH V4] char: pcmcia: scr24x_cs: Fix failure handling of device_create()
@ 2021-05-27 19:34 nizamhaider786
2021-05-28 7:26 ` Lubomir Rintel
0 siblings, 1 reply; 2+ messages in thread
From: nizamhaider786 @ 2021-05-27 19:34 UTC (permalink / raw)
To: lkundrak; +Cc: arnd, gregkh, linux-kernel, Nijam Haider
From: Nijam Haider <nizamhaider786@gmail.com>
Ignored error in device_create() and pcmcia_enable_device()
this patch implements proper error handling.
Signed-off-by: Nijam Haider <nizamhaider786@gmail.com>
---
V3 -> V4: Added label and moved the cleanup code
V2 -> V3: Added description, Changelog and removed whitespace error
V1 -> V2: Split the patch into two parts and addressed review comments
---
drivers/char/pcmcia/scr24x_cs.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/char/pcmcia/scr24x_cs.c b/drivers/char/pcmcia/scr24x_cs.c
index 47feb39af34c..ba84b4dd13d3 100644
--- a/drivers/char/pcmcia/scr24x_cs.c
+++ b/drivers/char/pcmcia/scr24x_cs.c
@@ -233,6 +233,7 @@ static int scr24x_probe(struct pcmcia_device *link)
{
struct scr24x_dev *dev;
int ret;
+ struct device *dev_ret;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
@@ -271,22 +272,28 @@ static int scr24x_probe(struct pcmcia_device *link)
goto err;
ret = pcmcia_enable_device(link);
- if (ret < 0) {
- pcmcia_disable_device(link);
- goto err;
- }
+ if (ret < 0)
+ goto err_device;
- device_create(scr24x_class, NULL, MKDEV(MAJOR(scr24x_devt), dev->devno),
+ dev_ret = device_create(scr24x_class, NULL, MKDEV(MAJOR(scr24x_devt), dev->devno),
NULL, "scr24x%d", dev->devno);
+ if (IS_ERR(dev_ret)) {
+ dev_err(&link->dev, "device_create failed for %d\n",
+ dev->devno);
+ goto err_device;
+ }
dev_info(&link->dev, "SCR24x Chip Card Interface\n");
return 0;
+err_device:
+ pcmcia_disable_device(link);
+ cdev_del(&dev->c_dev);
err:
if (dev->devno < SCR24X_DEVS)
clear_bit(dev->devno, scr24x_minors);
kfree (dev);
- return ret;
+ return -ENODEV;
}
static void scr24x_remove(struct pcmcia_device *link)
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH V4] char: pcmcia: scr24x_cs: Fix failure handling of device_create()
2021-05-27 19:34 [PATCH V4] char: pcmcia: scr24x_cs: Fix failure handling of device_create() nizamhaider786
@ 2021-05-28 7:26 ` Lubomir Rintel
0 siblings, 0 replies; 2+ messages in thread
From: Lubomir Rintel @ 2021-05-28 7:26 UTC (permalink / raw)
To: nizamhaider786; +Cc: arnd, gregkh, linux-kernel
Hi,
much better now, thank you.
Acked-by: Lubomir Rintel <lkundrak@v3.sk>
There's one small remark below.
On Fri, May 28, 2021 at 01:04:29AM +0530, nizamhaider786@gmail.com wrote:
> From: Nijam Haider <nizamhaider786@gmail.com>
>
> Ignored error in device_create() and pcmcia_enable_device()
> this patch implements proper error handling.
>
> Signed-off-by: Nijam Haider <nizamhaider786@gmail.com>
> ---
> V3 -> V4: Added label and moved the cleanup code
> V2 -> V3: Added description, Changelog and removed whitespace error
> V1 -> V2: Split the patch into two parts and addressed review comments
> ---
> drivers/char/pcmcia/scr24x_cs.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/pcmcia/scr24x_cs.c b/drivers/char/pcmcia/scr24x_cs.c
> index 47feb39af34c..ba84b4dd13d3 100644
> --- a/drivers/char/pcmcia/scr24x_cs.c
> +++ b/drivers/char/pcmcia/scr24x_cs.c
> @@ -233,6 +233,7 @@ static int scr24x_probe(struct pcmcia_device *link)
> {
> struct scr24x_dev *dev;
> int ret;
> + struct device *dev_ret;
>
> dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> if (!dev)
> @@ -271,22 +272,28 @@ static int scr24x_probe(struct pcmcia_device *link)
> goto err;
>
> ret = pcmcia_enable_device(link);
> - if (ret < 0) {
> - pcmcia_disable_device(link);
> - goto err;
> - }
> + if (ret < 0)
> + goto err_device;
>
> - device_create(scr24x_class, NULL, MKDEV(MAJOR(scr24x_devt), dev->devno),
> + dev_ret = device_create(scr24x_class, NULL, MKDEV(MAJOR(scr24x_devt), dev->devno),
> NULL, "scr24x%d", dev->devno);
> + if (IS_ERR(dev_ret)) {
> + dev_err(&link->dev, "device_create failed for %d\n",
> + dev->devno);
> + goto err_device;
> + }
>
> dev_info(&link->dev, "SCR24x Chip Card Interface\n");
> return 0;
>
> +err_device:
> + pcmcia_disable_device(link);
> + cdev_del(&dev->c_dev);
> err:
> if (dev->devno < SCR24X_DEVS)
> clear_bit(dev->devno, scr24x_minors);
> kfree (dev);
> - return ret;
> + return -ENODEV;
Why -ENODEV? Couldn't we propagate the original error with something
like "ret = PTR_ERR(dev_ret);" just before the "goto err_device"?
Thank you,
Lubo
> }
>
> static void scr24x_remove(struct pcmcia_device *link)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-05-28 7:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 19:34 [PATCH V4] char: pcmcia: scr24x_cs: Fix failure handling of device_create() nizamhaider786
2021-05-28 7:26 ` Lubomir Rintel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).