[U-Boot] [PATCH] 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 matthias@kaehlcke.net --- drivers/net/cs8900.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c index df36004..7895048 100644 --- a/drivers/net/cs8900.c +++ b/drivers/net/cs8900.c @@ -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;

Matthias Kaehlcke wrote:
cs8900_initialize(): remove unecessary calls to free(), fix memory leak and report errors in the return value
Signed-off-by: Matthias Kaehlcke matthias@kaehlcke.net
drivers/net/cs8900.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c index df36004..7895048 100644 --- a/drivers/net/cs8900.c +++ b/drivers/net/cs8900.c @@ -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.
} memset(dev, 0, sizeof(*dev));
priv = malloc(sizeof(*priv)); if (!priv) {
free(priv);
return 0;
free(dev);
} memset(priv, 0, sizeof(*priv)); priv->regs = (struct cs8900_regs *)base_addr;return -1;
regards, Ben

El Thu, Jan 21, 2010 at 01:10:45PM -0800 Ben Warren ha dit:
Matthias Kaehlcke wrote:
cs8900_initialize(): remove unecessary calls to free(), fix memory leak and report errors in the return value
Signed-off-by: Matthias Kaehlcke matthias@kaehlcke.net
drivers/net/cs8900.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c index df36004..7895048 100644 --- a/drivers/net/cs8900.c +++ b/drivers/net/cs8900.c @@ -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 :(
Removing the 'free' calls is good, though.
i'll send out a patch without the return value 'fix' then
best regards

Dear Matthias Kaehlcke,
In message 20100121211245.GA16182@darwin you wrote:
'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

hi wolfgang,
El Thu, Jan 21, 2010 at 11:02:17PM +0100 Wolfgang Denk ha dit:
Dear Matthias Kaehlcke,
In message 20100121211245.GA16182@darwin you wrote:
'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
participants (3)
-
Ben Warren
-
Matthias Kaehlcke
-
Wolfgang Denk