On Sun, Apr 25, 2021 at 01:13:01AM +0530, Anupama K Patil wrote:
isapnp_proc_init() does not look at the return value from isapnp_proc_attach_device(). Check for this return value in isapnp_proc_detach_device().
Cleanup in isapnp_proc_detach_device and isapnp_proc_detach_bus() for cleanup.
Changed sprintf() to the kernel-space function scnprintf() as it returns the actual number of bytes written.
Removed unnecessary variables de, e of type 'struct proc_dir_entry' to save memory.
What exactly do you fix for such an old code?
Suggested-by: Shuah Khan <skhan@linuxfoundation.org> Co-developed-by: B K Karthik <bkkarthik@pesu.pes.edu> Signed-off-by: B K Karthik <bkkarthik@pesu.pes.edu> Signed-off-by: Anupama K Patil <anupamakpatil123@gmail.com> --- drivers/pnp/isapnp/proc.c | 40 +++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..46ebc24175b7 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -54,34 +54,54 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { .proc_read = isapnp_proc_bus_read, };
+static int isapnp_proc_detach_device(struct pnp_dev *dev) +{ + proc_remove(dev->procent); + dev->procent = NULL; + return 0; +} + +static int isapnp_proc_detach_bus(struct pnp_card *bus) +{ + proc_remove(bus->procdir); + return 0; +}
Please don't add one line functions that are called only once and have return value that no one care about it. Thanks
+ static int isapnp_proc_attach_device(struct pnp_dev *dev) { struct pnp_card *bus = dev->card; - struct proc_dir_entry *de, *e; char name[16];
- if (!(de = bus->procdir)) { - sprintf(name, "%02x", bus->number); - de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + if (!bus->procdir) { + scnprintf(name, 16, "%02x", bus->number); + bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); + if (!bus->procdir) return -ENOMEM; } - sprintf(name, "%02x", dev->number); - e = dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, de, + scnprintf(name, 16, "%02x", dev->number); + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) { + isapnp_proc_detach_bus(bus); return -ENOMEM; - proc_set_size(e, 256); + } + proc_set_size(dev->procent, 256); return 0; }
int __init isapnp_proc_init(void) { struct pnp_dev *dev; + int dev_attach;
isapnp_proc_bus_dir = proc_mkdir("bus/isapnp", NULL); protocol_for_each_dev(&isapnp_protocol, dev) { - isapnp_proc_attach_device(dev); + dev_attach = isapnp_proc_attach_device(dev); + if (!dev_attach) { + pr_info("procfs: pnp: Unable to attach the device, not enough memory"); + isapnp_proc_detach_device(dev); + return -ENOMEM; + } } return 0; } -- 2.25.1
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies