Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
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
The change is straight without any functionality modification.
Reviewed-by: Jaroslav Kysela
> ---
> 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
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
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
>> ---
>> 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
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky 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! ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
On 21/04/26 03:50PM, Leon Romanovsky wrote: > On Mon, Apr 26, 2021 at 02:00:58PM +0200, Rafael J. Wysocki wrote: > > On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky 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. If changes to ISA code aren't welcomed, should these be marked obsolete in the MAINTIANERS file? > > > > Real fixes and obvious cleanups are, not much more than that. > > While first part is easy to determine, the second one is more blurry. > > > > > > According to the Wikipedia, even Windows Vista disabled ISA PnP by > > > default. > > > https://en.wikipedia.org/wiki/Legacy_Plug_and_Play#Specifications I wasn't aware until after this reply. Sorry about that! thanks, karthik > > > > 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! ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
On Mon, Apr 26, 2021 at 11:22:54PM +0530, bkkarthik wrote: > On 21/04/26 03:50PM, Leon Romanovsky wrote: > > On Mon, Apr 26, 2021 at 02:00:58PM +0200, Rafael J. Wysocki wrote: > > > On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky 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. > > If changes to ISA code aren't welcomed, should these be marked obsolete in > the MAINTIANERS file? I think so, but think that "Odd Fixes" better describes that Rafael wrote. Thanks ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
On Mon, Apr 26, 2021 at 02:00:58PM +0200, Rafael J. Wysocki wrote: > On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky 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. While first part is easy to determine, the second one is more blurry. > > > 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! ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
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
> > ---
> > 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
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
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
---
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
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
[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
---
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
signature.asc
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
