public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize()
@ 2010-01-21 20:56 Matthias Kaehlcke
  2010-01-21 21:10 ` Ben Warren
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2010-01-21 20:56 UTC (permalink / raw)
  To: u-boot

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;
-- 
1.6.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize()
  2010-01-21 20:56 [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize() Matthias Kaehlcke
@ 2010-01-21 21:10 ` Ben Warren
  2010-01-21 21:12   ` Matthias Kaehlcke
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Warren @ 2010-01-21 21:10 UTC (permalink / raw)
  To: u-boot

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);
> +		return -1;
>  	}
>  	memset(priv, 0, sizeof(*priv));
>  	priv->regs = (struct cs8900_regs *)base_addr;
>   
regards,
Ben

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize()
  2010-01-21 21:10 ` Ben Warren
@ 2010-01-21 21:12   ` Matthias Kaehlcke
  2010-01-21 22:02     ` Wolfgang Denk
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2010-01-21 21:12 UTC (permalink / raw)
  To: u-boot

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

-- 
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                  `-

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize()
  2010-01-21 22:02     ` Wolfgang Denk
@ 2010-01-21 22:01       ` Matthias Kaehlcke
  0 siblings, 0 replies; 5+ messages in thread
From: Matthias Kaehlcke @ 2010-01-21 22:01 UTC (permalink / raw)
  To: u-boot

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

-- 
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                  `-

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize()
  2010-01-21 21:12   ` Matthias Kaehlcke
@ 2010-01-21 22:02     ` Wolfgang Denk
  2010-01-21 22:01       ` Matthias Kaehlcke
  0 siblings, 1 reply; 5+ messages in thread
From: Wolfgang Denk @ 2010-01-21 22:02 UTC (permalink / raw)
  To: u-boot

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

-- 
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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-01-21 22:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-21 20:56 [U-Boot] [PATCH] cs8900 driver: cleanup cs8900_initialize() Matthias Kaehlcke
2010-01-21 21:10 ` Ben Warren
2010-01-21 21:12   ` Matthias Kaehlcke
2010-01-21 22:02     ` Wolfgang Denk
2010-01-21 22:01       ` Matthias Kaehlcke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox