* [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize()
@ 2010-01-21 21:29 Matthias Kaehlcke
2010-01-21 23:01 ` Mike Frysinger
0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2010-01-21 21:29 UTC (permalink / raw)
To: u-boot
smc911x_initialize(): remove unecessary call to free() and
return 0 in case of failure instead of -1
Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
---
drivers/net/smc911x.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 5d51406..f2b5895 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -242,8 +242,7 @@ int smc911x_initialize(u8 dev_num, int base_addr)
dev = malloc(sizeof(*dev));
if (!dev) {
- free(dev);
- return -1;
+ return 0;
}
memset(dev, 0, sizeof(*dev));
--
1.6.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize()
2010-01-21 21:29 [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize() Matthias Kaehlcke
@ 2010-01-21 23:01 ` Mike Frysinger
2010-01-22 6:18 ` Matthias Kaehlcke
0 siblings, 1 reply; 5+ messages in thread
From: Mike Frysinger @ 2010-01-21 23:01 UTC (permalink / raw)
To: u-boot
On Thursday 21 January 2010 16:29:24 Matthias Kaehlcke wrote:
> smc911x_initialize(): remove unecessary call to free() and
> return 0 in case of failure instead of -1
>
> Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
> ---
> drivers/net/smc911x.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 5d51406..f2b5895 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -242,8 +242,7 @@ int smc911x_initialize(u8 dev_num, int base_addr)
>
> dev = malloc(sizeof(*dev));
> if (!dev) {
> - free(dev);
OK
> - return -1;
> + return 0;
this is an error path, so i think -1 is correct. if you're out of memory,
increase your malloc region.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100121/1b4583b7/attachment.pgp
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize()
2010-01-21 23:01 ` Mike Frysinger
@ 2010-01-22 6:18 ` Matthias Kaehlcke
2010-01-22 7:36 ` Ben Warren
0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2010-01-22 6:18 UTC (permalink / raw)
To: u-boot
hi mike,
El Thu, Jan 21, 2010 at 06:01:47PM -0500 Mike Frysinger ha dit:
> On Thursday 21 January 2010 16:29:24 Matthias Kaehlcke wrote:
> > smc911x_initialize(): remove unecessary call to free() and
> > return 0 in case of failure instead of -1
> >
> > Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
> > ---
> > drivers/net/smc911x.c | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> > index 5d51406..f2b5895 100644
> > --- a/drivers/net/smc911x.c
> > +++ b/drivers/net/smc911x.c
> > @@ -242,8 +242,7 @@ int smc911x_initialize(u8 dev_num, int base_addr)
> >
> > dev = malloc(sizeof(*dev));
> > if (!dev) {
> > - free(dev);
>
> OK
>
> > - return -1;
> > + return 0;
>
> this is an error path, so i think -1 is correct. if you're out of memory,
> increase your malloc region.
that's what I thought in the first place, but Ben Warren told me that
in the initialize function the return value indicates the number of
devices that were initialized (see
http://lists.denx.de/pipermail/u-boot/2010-January/066859.html)
best regards
--
Matthias Kaehlcke
Embedded Linux Developer
Barcelona
Don't walk behind me, I may not lead
Don't walk in front of me, I may not follow
Just walk beside me and be my friend
(Albert Camus)
.''`.
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] smc911x driver: cleanup smc911x_initialize()
2010-01-22 6:18 ` Matthias Kaehlcke
@ 2010-01-22 7:36 ` Ben Warren
2010-01-22 9:02 ` Matthias Kaehlcke
0 siblings, 1 reply; 5+ messages in thread
From: Ben Warren @ 2010-01-22 7:36 UTC (permalink / raw)
To: u-boot
Hi Matthias,
On Thu, Jan 21, 2010 at 10:18 PM, Matthias Kaehlcke
<matthias@kaehlcke.net>wrote:
> hi mike,
>
> El Thu, Jan 21, 2010 at 06:01:47PM -0500 Mike Frysinger ha dit:
>
> > On Thursday 21 January 2010 16:29:24 Matthias Kaehlcke wrote:
> > > smc911x_initialize(): remove unecessary call to free() and
> > > return 0 in case of failure instead of -1
> > >
> > > Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
> > > ---
> > > drivers/net/smc911x.c | 3 +--
> > > 1 files changed, 1 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> > > index 5d51406..f2b5895 100644
> > > --- a/drivers/net/smc911x.c
> > > +++ b/drivers/net/smc911x.c
> > > @@ -242,8 +242,7 @@ int smc911x_initialize(u8 dev_num, int base_addr)
> > >
> > > dev = malloc(sizeof(*dev));
> > > if (!dev) {
> > > - free(dev);
> >
> > OK
> >
> > > - return -1;
> > > + return 0;
> >
> > this is an error path, so i think -1 is correct. if you're out of
> memory,
> > increase your malloc region.
>
> that's what I thought in the first place, but Ben Warren told me that
> in the initialize function the return value indicates the number of
> devices that were initialized (see
> http://lists.denx.de/pipermail/u-boot/2010-January/066859.html)
>
> I agree that this is confusing. If the following xxx_eth_initialize()
return codes seem reasonable, I'll send a patch tomorrow that applies it to
the logic in net/eth.c:
-2: reserved (magic value, used to return from __def_eth_init())
-1: error
0: no devices added, no error
1+: number of devices added
best regards
>
> --
> Matthias Kaehlcke
> Embedded Linux Developer
> Barcelona
>
> regards,
Ben
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize()
2010-01-22 7:36 ` Ben Warren
@ 2010-01-22 9:02 ` Matthias Kaehlcke
0 siblings, 0 replies; 5+ messages in thread
From: Matthias Kaehlcke @ 2010-01-22 9:02 UTC (permalink / raw)
To: u-boot
Hi Ben,
El Thu, Jan 21, 2010 at 11:36:41PM -0800 Ben Warren ha dit:
> On Thu, Jan 21, 2010 at 10:18 PM, Matthias Kaehlcke
> <matthias@kaehlcke.net> wrote:
>
> El Thu, Jan 21, 2010 at 06:01:47PM -0500 Mike Frysinger ha dit:
> > On Thursday 21 January 2010 16:29:24 Matthias Kaehlcke wrote:
> > > smc911x_initialize(): remove unecessary call to free() and
> > > return 0 in case of failure instead of -1
> > >
> > > Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
> > > ---
> > > drivers/net/smc911x.c | 3 +--
> > > 1 files changed, 1 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> > > index 5d51406..f2b5895 100644
> > > --- a/drivers/net/smc911x.c
> > > +++ b/drivers/net/smc911x.c
> > > @@ -242,8 +242,7 @@ int smc911x_initialize(u8 dev_num, int
> base_addr)
> > >
> > > dev = malloc(sizeof(*dev));
> > > if (!dev) {
> > > - free(dev);
> >
> > OK
> >
> > > - return -1;
> > > + return 0;
> >
> > this is an error path, so i think -1 is correct. if you're out of
> memory,
> > increase your malloc region.
>
> that's what I thought in the first place, but Ben Warren told me that
> in the initialize function the return value indicates the number of
> devices that were initialized (see
> http://lists.denx.de/pipermail/u-boot/2010-January/066859.html)
>
> I agree that this is confusing. If the following xxx_eth_initialize()
> return codes seem reasonable, I'll send a patch tomorrow that applies it
> to the logic in net/eth.c:
> -2: reserved (magic value, used to return from __def_eth_init())
> -1: error
> 0: no devices added, no error
> 1+: number of devices added
to me it seems reasonable and less confusing than the current logic.
i can collaborate in the task of applying it to the driver code
--
Matthias Kaehlcke
Embedded Linux Developer
Barcelona
Control over the use of one's ideas really constitutes control over other
people's lives; and it is usually used to make their lives more difficult.
(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
end of thread, other threads:[~2010-01-22 9: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 21:29 [U-Boot] [PATCH] smc911x driver: cleanup smc911x_initialize() Matthias Kaehlcke
2010-01-21 23:01 ` Mike Frysinger
2010-01-22 6:18 ` Matthias Kaehlcke
2010-01-22 7:36 ` Ben Warren
2010-01-22 9:02 ` Matthias Kaehlcke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox