cs8900 driver: cleanup cs8900_initialize()

PATCH New Matthias Kaehlcke 3 participants 5 messages Jan 21, 2010 Network drivers

lore

Share & tools

lore

Matthias Kaehlcke

author

2010-01-21 20:56 UTC #0 lore raw

PATCH 1/1 cs8900 driver: cleanup cs8900_initialize()

cs8900_initialize(): remove unecessary calls to free(), fix memory leak and report errors in the return value

Signed-off-by Matthias Kaehlcke <[email protected]>

---
 drivers/net/cs8900.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

drivers/net/cs8900.c +3 -4

index df36004..7895048 100644
@@ -308,15 +308,14 @@ int cs8900_initialize(u8 dev_num, int base_addr)
 
 	dev = malloc(sizeof(*dev));
 	if (!dev) {
-		free(dev);
-		return 0;
+		return -1;
 	}
 	memset(dev, 0, sizeof(*dev));
 
 	priv = malloc(sizeof(*priv));
 	if (!priv) {
-		free(priv);
-		return 0;
+		free(dev);
+		return -1;
 	}
 	memset(priv, 0, sizeof(*priv));
 	priv->regs = (struct cs8900_regs *)base_addr;

Signature · 1 line

1.6.3.1

Ben Warren

2010-01-21 21:10 UTC #1 lore raw

PATCH 1/1 cs8900 driver: cleanup cs8900_initialize()

Quoted 13 lines · Matthias Kaehlcke

cs8900_initialize(): remove unecessary calls to free(), fix memory leak and report errors in the return value
Signed-off-by Matthias Kaehlcke <[email protected]>
---
 drivers/net/cs8900.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
drivers/net/cs8900.c +0 -0
index df36004..7895048 100644

drivers/net/cs8900.c int cs8900_initialize(u8 dev_num, int base_addr) · line 312

Quoted 7 lines

drivers/net/cs8900.c +1 -2

@@ -308,15 +308,14 @@ int cs8900_initialize(u8 dev_num, int base_addr)
 
 	dev = malloc(sizeof(*dev));
 	if (!dev) {
-		free(dev);
-		return 0;
+		return -1;

'return 0' is actually correct. It refers to the number of devices that were initialized. Removing the 'free' calls is good, though.

Quoted 11 lines

drivers/net/cs8900.c +2 -2

 	}
 	memset(dev, 0, sizeof(*dev));
 
 	priv = malloc(sizeof(*priv));
 	if (!priv) {
-		free(priv);
-		return 0;
+		free(dev);
+		return -1;
 	}
 	memset(priv, 0, sizeof(*priv));

Quoted 2 lines

priv->regs = (struct cs8900_regs *)base_addr;

regards,
Ben

Matthias Kaehlcke

author

2010-01-21 21:12 UTC #2 lore raw

PATCH 1/1 cs8900 driver: cleanup cs8900_initialize()

El Thu, Jan 21, 2010 at 01:10:45PM -0800 Ben Warren ha dit:

Quoted 9 lines

Quoted 8 lines · Matthias Kaehlcke
cs8900_initialize(): remove unecessary calls to free(), fix memory leak and report errors in the return value
Signed-off-by Matthias Kaehlcke <[email protected]>
---
 drivers/net/cs8900.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

Quoted 13 lines

drivers/net/cs8900.c int cs8900_initialize(u8 dev_num, int base_addr) · line 311
Quoted 11 lines
drivers/net/cs8900.c +1 -2
index df36004..7895048 100644
@@ -308,15 +308,14 @@ int cs8900_initialize(u8 dev_num, int base_addr)
  	dev = malloc(sizeof(*dev));
 	if (!dev) {
-		free(dev);
-		return 0;
+		return -1;
 
'return 0' is actually correct. It refers to the number of devices that were initialized.

ok, i was in doubt and had a look at another driver, which returns a negative value and followed the bad example :(

Quoted 1 line

Removing the 'free' calls is good, though.

i'll send out a patch without the return value 'fix' then

best regards

Signature · 10 lines

Matthias Kaehlcke
Embedded Linux Developer
Barcelona

  Si deseas mantener tu libertad, debes estar preparado para defenderla
                          (Richard Stallman)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

Wolfgang Denk

2010-01-21 22:02 UTC #3 lore raw

PATCH 1/1 cs8900 driver: cleanup cs8900_initialize()

Dear Matthias Kaehlcke,

Quoted 6 lines · In message <20100121211245.GA16182@darwin> you wrote:

Quoted 3 lines
'return 0' is actually correct. It refers to the number of devices that were initialized.
ok, i was in doubt and had a look at another driver, which returns a negative value and followed the bad example :(

Please send a fix for that other driver, too...

...or at least point us to the erroneous spot.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Comparing information  and  knowledge  is  like  asking  whether  the
fatness  of  a  pig  is more or less green than the designated hitter
rule."                                               - David Guaspari

Matthias Kaehlcke

author

2010-01-21 22:01 UTC #4 lore raw

PATCH 1/1 cs8900 driver: cleanup cs8900_initialize()

hi wolfgang,

El Thu, Jan 21, 2010 at 11:02:17PM +0100 Wolfgang Denk ha dit:

Quoted 11 lines

Dear Matthias Kaehlcke,
Quoted 6 lines · In message <20100121211245.GA16182@darwin> you wrote:
Quoted 3 lines
'return 0' is actually correct. It refers to the number of devices that were initialized.
ok, i was in doubt and had a look at another driver, which returns a negative value and followed the bad example :(
Please send a fix for that other driver, too...

http://lists.denx.de/pipermail/u-boot/2010-January/066871.html

regards

Signature · 10 lines

Matthias Kaehlcke
Embedded Linux Developer
Barcelona

              We build too many walls and not enough bridges
                             (Isaac Newton)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-