On 21/04/28 03:21PM, Leon Romanovsky wrote:
On Wed, Apr 28, 2021 at 02:04:49PM +0200, Jaroslav Kysela wrote:
Dne 26. 04. 21 v 19:50 bkkarthik napsal(a):
On 21/04/26 08:04AM, Leon Romanovsky wrote:
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?
I was not aware that this code is so old. This fix was made after checkpatch reported assignment inside an if-statement. Please ignore this patch if th change is not necessary as the code is probably not being used anywhere :)
Maybe the code has to be marked as obsolete in the MAINTAINERS file to prevent patches being sent?
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.
These were only intended for a clean-up job, the idea of this function came from how PCI handles procfs. Maybe those should be changed?
Which code you refer? I see:
for_each_pci_dev(dev) pci_proc_attach_device(dev);
He talks about isapnp_proc_detach_*() functions.
Yes, pci_proc_detach_device() and pci_proc_detach_bus() are both one-line functions as well. I don't mean to question working code, we only tried to do something similar here for ISA. thanks, karthik
The error codes are ignored, too. It does not harm, if proc entries are not created (in this case - the system is unstable anyway). We should concentrate only to the wrong pointers usage.
Jaroslav
-- Jaroslav Kysela <perex@perex.cz> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.