[PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement. Signed-off-by: Anupama K Patil <anupamakpatil123@gmail.com> --- drivers/pnp/isapnp/proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..1ae458c02656 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -57,21 +57,20 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { 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)) { + if (!bus->procdir) { sprintf(name, "%02x", bus->number); - de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + 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, + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) return -ENOMEM; - proc_set_size(e, 256); + proc_set_size(dev->procent, 256); return 0; } -- 2.25.1
On 4/22/21 12:03 PM, Anupama K Patil wrote:
de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement.
Sounds like a reasonable change.
Signed-off-by: Anupama K Patil <anupamakpatil123@gmail.com> --- drivers/pnp/isapnp/proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..1ae458c02656 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -57,21 +57,20 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { 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)) { + if (!bus->procdir) { sprintf(name, "%02x", bus->number);
It would make sense to change this to scnprintf()
- de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); + if (!bus->procdir) return -ENOMEM; } sprintf(name, "%02x", dev->number);
It would make sense to change this to scnprintf()
- e = dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, de, + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) return -ENOMEM;
Shouldn't the procdir be deleted when proc_create_data() fails?
- proc_set_size(e, 256); + proc_set_size(dev->procent, 256); return 0; }
Note that isapnp_proc_init() doesn't check isapnp_proc_attach_device() return and handle errors and cleanup. thanks, -- Shuah
On Fri, Apr 23, 2021 at 03:08:03PM -0600, Shuah Khan wrote:
On 4/22/21 12:03 PM, Anupama K Patil wrote:
de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement.
Sounds like a reasonable change.
It is unclear how much changes to ISA code are welcomed. According to the Wikipedia, even Windows Vista disabled ISA PnP by default. https://en.wikipedia.org/wiki/Legacy_Plug_and_Play#Specifications
Signed-off-by: Anupama K Patil <anupamakpatil123@gmail.com> --- drivers/pnp/isapnp/proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..1ae458c02656 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -57,21 +57,20 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { 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)) { + if (!bus->procdir) { sprintf(name, "%02x", bus->number);
It would make sense to change this to scnprintf()
- de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); + if (!bus->procdir) return -ENOMEM; } sprintf(name, "%02x", dev->number);
It would make sense to change this to scnprintf()
- e = dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, de, + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) return -ENOMEM;
Shouldn't the procdir be deleted when proc_create_data() fails?
It needs but only if proc_mkdir() was called in this function. Thanks
- proc_set_size(e, 256); + proc_set_size(dev->procent, 256); return 0; }
Note that isapnp_proc_init() doesn't check isapnp_proc_attach_device() return and handle errors and cleanup.
thanks, -- Shuah
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky <leon@kernel.org> wrote:
On Fri, Apr 23, 2021 at 03:08:03PM -0600, Shuah Khan wrote:
On 4/22/21 12:03 PM, Anupama K Patil wrote:
de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement.
Sounds like a reasonable change.
It is unclear how much changes to ISA code are welcomed.
Real fixes and obvious cleanups are, not much more than that.
According to the Wikipedia, even Windows Vista disabled ISA PnP by default. https://en.wikipedia.org/wiki/Legacy_Plug_and_Play#Specifications
It is indeed unclear how many systems with this interface still run Linux, but as long as the code is in the tree, there's nothing wrong with attempting to improve it. There's no assurance that all such patches will be applied, though. Thanks!
Dne 23. 04. 21 v 23:08 Shuah Khan napsal(a):
On 4/22/21 12:03 PM, Anupama K Patil wrote:
de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement.
Sounds like a reasonable change.
Signed-off-by: Anupama K Patil <anupamakpatil123@gmail.com> --- drivers/pnp/isapnp/proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..1ae458c02656 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -57,21 +57,20 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { 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)) { + if (!bus->procdir) { sprintf(name, "%02x", bus->number);
It would make sense to change this to scnprintf()
The name is 16 bytes, so sprintf is safe here. Jaroslav -- Jaroslav Kysela <perex@perex.cz> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
Dne 22. 04. 21 v 20:03 Anupama K Patil napsal(a):
de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement.
Signed-off-by: Anupama K Patil <anupamakpatil123@gmail.com>
The change is straight without any functionality modification. Reviewed-by: Jaroslav Kysela <perex@perex.cz>
--- drivers/pnp/isapnp/proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..1ae458c02656 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -57,21 +57,20 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { 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)) { + if (!bus->procdir) { sprintf(name, "%02x", bus->number); - de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + 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, + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) return -ENOMEM; - proc_set_size(e, 256); + proc_set_size(dev->procent, 256); return 0; }
-- Jaroslav Kysela <perex@perex.cz> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
participants (5)
-
Anupama K Patil -
Jaroslav Kysela -
Leon Romanovsky -
Rafael J. Wysocki -
Shuah Khan