[PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

The result of calling kzalloc is never used or freed.

The semantic match that finds this problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// smpl
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x...@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
... when != x
 when != if (...) { +...x...+ }
(
x-f1 = E
|
 (x-f1 == NULL || ...)
|
 f(...,x-f1,...)
)
...
(
 return \(0\|+...x...+\|ptr\);
|
 ret...@p2 ...;
)

@script:python@
p1  r.p1;
p2  r.p2;
@@

print * file: %s kmalloc %s return %s % (p1[0].file,p1[0].line,p2[0].line)
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk
---
 drivers/pcmcia/sa_generic.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/pcmcia/sa_generic.c b/drivers/pcmcia/sa_generic.c
index deb5036..de6bc33 100644
--- a/drivers/pcmcia/sa_generic.c
+++ b/drivers/pcmcia/sa_generic.c
@@ -127,10 +127,6 @@ int sa_pcmcia_add(struct sa_dev *dev, struct 
pcmcia_low_level *ops,
ops-socket_state = sa_pcmcia_socket_state;
ops-socket_suspend = sa_pcmcia_socket_suspend;
 
-   s = kzalloc(sizeof(*s) * ops-nr, GFP_KERNEL);
-   if (!s)
-   return -ENODEV;
-
for (i = 0; i  ops-nr; i++) {
s = kzalloc(sizeof(*s), GFP_KERNEL);
if (!s)

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia

Re: [PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread walter harms 


Julia Lawall schrieb:
 From: Julia Lawall ju...@diku.dk
 
 The result of calling kzalloc is never used or freed.
 
 The semantic match that finds this problem is as follows:
 (http://www.emn.fr/x-info/coccinelle/)
 
 // smpl
 @r exists@
 local idexpression x;
 statement S;
 expression E;
 identifier f,f1,l;
 position p1,p2;
 expression *ptr != NULL;
 @@
 
 x...@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
 ...
 if (x == NULL) S
 ... when != x
  when != if (...) { +...x...+ }
 (
 x-f1 = E
 |
  (x-f1 == NULL || ...)
 |
  f(...,x-f1,...)
 )
 ...
 (
  return \(0\|+...x...+\|ptr\);
 |
  ret...@p2 ...;
 )
 
 @script:python@
 p1  r.p1;
 p2  r.p2;
 @@
 
 print * file: %s kmalloc %s return %s % (p1[0].file,p1[0].line,p2[0].line)
 // /smpl
 
 Signed-off-by: Julia Lawall ju...@diku.dk
 ---
  drivers/pcmcia/sa_generic.c |4 
  1 files changed, 0 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/pcmcia/sa_generic.c b/drivers/pcmcia/sa_generic.c
 index deb5036..de6bc33 100644
 --- a/drivers/pcmcia/sa_generic.c
 +++ b/drivers/pcmcia/sa_generic.c
 @@ -127,10 +127,6 @@ int sa_pcmcia_add(struct sa_dev *dev, struct 
 pcmcia_low_level *ops,
   ops-socket_state = sa_pcmcia_socket_state;
   ops-socket_suspend = sa_pcmcia_socket_suspend;
  
 - s = kzalloc(sizeof(*s) * ops-nr, GFP_KERNEL);
 - if (!s)
 - return -ENODEV;
 -
   for (i = 0; i  ops-nr; i++) {
   s = kzalloc(sizeof(*s), GFP_KERNEL);
   if (!s)



mmmh, perhaps the original idea was to have an array
and then he moved to a linked list ?

I thing the array idea is better (1. it fails on low mem propperly, 2. no need 
free()
3. less zmalloc stuff) but requieres that pcmcia_remove()
will release that array propperly

the bug is strange,

just my 2 cents,
   re,
wh

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia

Re: [PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread Julia Lawall 
On Sat, 21 Nov 2009, walter harms wrote:

 
 
 Julia Lawall schrieb:
  From: Julia Lawall ju...@diku.dk
  
  The result of calling kzalloc is never used or freed.
  
  The semantic match that finds this problem is as follows:
  (http://www.emn.fr/x-info/coccinelle/)
  
  // smpl
  @r exists@
  local idexpression x;
  statement S;
  expression E;
  identifier f,f1,l;
  position p1,p2;
  expression *ptr != NULL;
  @@
  
  x...@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
  ...
  if (x == NULL) S
  ... when != x
   when != if (...) { +...x...+ }
  (
  x-f1 = E
  |
   (x-f1 == NULL || ...)
  |
   f(...,x-f1,...)
  )
  ...
  (
   return \(0\|+...x...+\|ptr\);
  |
   ret...@p2 ...;
  )
  
  @script:python@
  p1  r.p1;
  p2  r.p2;
  @@
  
  print * file: %s kmalloc %s return %s % (p1[0].file,p1[0].line,p2[0].line)
  // /smpl
  
  Signed-off-by: Julia Lawall ju...@diku.dk
  ---
   drivers/pcmcia/sa_generic.c |4 
   1 files changed, 0 insertions(+), 4 deletions(-)
  
  diff --git a/drivers/pcmcia/sa_generic.c 
  b/drivers/pcmcia/sa_generic.c
  index deb5036..de6bc33 100644
  --- a/drivers/pcmcia/sa_generic.c
  +++ b/drivers/pcmcia/sa_generic.c
  @@ -127,10 +127,6 @@ int sa_pcmcia_add(struct sa_dev *dev, struct 
  pcmcia_low_level *ops,
  ops-socket_state = sa_pcmcia_socket_state;
  ops-socket_suspend = sa_pcmcia_socket_suspend;
   
  -   s = kzalloc(sizeof(*s) * ops-nr, GFP_KERNEL);
  -   if (!s)
  -   return -ENODEV;
  -
  for (i = 0; i  ops-nr; i++) {
  s = kzalloc(sizeof(*s), GFP_KERNEL);
  if (!s)
 
 
 
 mmmh, perhaps the original idea was to have an array
 and then he moved to a linked list ?
 
 I thing the array idea is better (1. it fails on low mem propperly, 2. no 
 need free()
 3. less zmalloc stuff) but requieres that pcmcia_remove()
 will release that array propperly
 
 the bug is strange,

Both kzallocs were added at the same time, when the function was added in 
commit 701a5dc05ad99a06958b3f97cb69d99b47cebee3.  I have added the author 
to the CC list.

julia

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia

Re: [PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread Russell King - ARM Linux 
On Sat, Nov 21, 2009 at 04:12:59PM +0100, Julia Lawall wrote:
 Both kzallocs were added at the same time, when the function was added in 
 commit 701a5dc05ad99a06958b3f97cb69d99b47cebee3.  I have added the author 
 to the CC list.

That commit id means nothing to me.

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia

Re: [PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread Julia Lawall 
On Sat, 21 Nov 2009, Russell King - ARM Linux wrote:

 On Sat, Nov 21, 2009 at 04:12:59PM +0100, Julia Lawall wrote:
  Both kzallocs were added at the same time, when the function was added in 
  commit 701a5dc05ad99a06958b3f97cb69d99b47cebee3.  I have added the author 
  to the CC list.
 
 That commit id means nothing to me.

It seems to only exist under linux-next, even though it dates from March.  
The relevant part of the patch is below (see the definition of 
sa_pcmcia_add).

julia

commit 701a5dc05ad99a06958b3f97cb69d99b47cebee3
Author: Russell King - ARM Linux li...@arm.linux.org.uk
Date:   Sun Mar 29 19:42:44 2009 +0100

PCMCIA: sa: wrap soc_pcmcia_socket to contain sa specific data

Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Dominik Brodowski li...@dominikbrodowski.net

diff --git a/drivers/pcmcia/sa_generic.c b/drivers/pcmcia/sa_generic.c
index a6793e3..98c7915 100644
--- a/drivers/pcmcia/sa_generic.c
+++ b/drivers/pcmcia/sa_generic.c
@@ -30,9 +30,6 @@ static struct pcmcia_irqs irqs[] = {
 
 int sa_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 {
-   if (skt-irq == NO_IRQ)
-   skt-irq = skt-nr ? IRQ_S1_READY_NINT : IRQ_S0_READY_NINT;
-
return soc_pcmcia_request_irqs(skt, irqs, ARRAY_SIZE(irqs));
 }
 
@@ -43,8 +40,8 @@ void sa_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt)
 
 void sa_pcmcia_socket_state(struct soc_pcmcia_socket *skt, struct 
pcmcia_state *state)
 {
-   struct sa_dev *sadev = SA_DEV(skt-dev);
-   unsigned long status = sa_readl(sadev-mapbase + SA_PCSR);
+   struct sa_pcmcia_socket *s = to_skt(skt);
+   unsigned long status = sa_readl(s-dev-mapbase + SA_PCSR);
 
switch (skt-nr) {
case 0:
@@ -71,7 +68,7 @@ void sa_pcmcia_socket_state(struct soc_pcmcia_socket 
*skt, struct pcmcia_sta
 
 int sa_pcmcia_configure_socket(struct soc_pcmcia_socket *skt, const 
socket_state_t *state)
 {
-   struct sa_dev *sadev = SA_DEV(skt-dev);
+   struct sa_pcmcia_socket *s = to_skt(skt);
unsigned int pccr_skt_mask, pccr_set_mask, val;
unsigned long flags;
 
@@ -100,10 +97,10 @@ int sa_pcmcia_configure_socket(struct 
soc_pcmcia_socket *skt, const socket_s
pccr_set_mask |= PCCR_S0_FLT|PCCR_S1_FLT;
 
local_irq_save(flags);
-   val = sa_readl(sadev-mapbase + SA_PCCR);
+   val = sa_readl(s-dev-mapbase + SA_PCCR);
val = ~pccr_skt_mask;
val |= pccr_set_mask  pccr_skt_mask;
-   sa_writel(val, sadev-mapbase + SA_PCCR);
+   sa_writel(val, s-dev-mapbase + SA_PCCR);
local_irq_restore(flags);
 
return 0;
@@ -119,10 +116,45 @@ void sa_pcmcia_socket_suspend(struct 
soc_pcmcia_socket *skt)
soc_pcmcia_disable_irqs(skt, irqs, ARRAY_SIZE(irqs));
 }
 
+int sa_pcmcia_add(struct sa_dev *dev, struct pcmcia_low_level *ops,
+   int (*add)(struct soc_pcmcia_socket *))
+{
+   struct sa_pcmcia_socket *s;
+   int i, ret = 0;
+
+   s = kzalloc(sizeof(*s) * ops-nr, GFP_KERNEL);
+   if (!s)
+   return -ENODEV;
+
+   for (i = 0; i  ops-nr; i++) {
+   s = kzalloc(sizeof(*s), GFP_KERNEL);
+   if (!s)
+   return -ENOMEM;
+
+   s-soc.nr = ops-first + i;
+   s-soc.irq = s-soc.nr ? IRQ_S1_READY_NINT : IRQ_S0_READY_NINT;
+   s-soc.ops = ops;
+   s-soc.socket.owner = ops-owner;
+   s-soc.socket.dev.parent = dev-dev;
+   s-dev = dev;
+
+   ret = add(s-soc);
+   if (ret == 0) {
+   s-next = dev_get_drvdata(dev-dev);
+   dev_set_drvdata(dev-dev, s);
+   } else
+   kfree(s);
+   }
+
+   return ret;
+}
+
 static int pcmcia_probe(struct sa_dev *dev)
 {
void __iomem *base;
 
+   dev_set_drvdata(dev-dev, NULL);
+
if (!request_mem_region(dev-res.start, 512,
SA_DRIVER_NAME(dev)))
return -EBUSY;
@@ -152,15 +184,15 @@ static int pcmcia_probe(struct sa_dev *dev)
 
 static int __devexit pcmcia_remove(struct sa_dev *dev)
 {
-   struct skt_dev_info *sinfo = dev_get_drvdata(dev-dev);
-   int i;
+   struct sa_pcmcia_socket *next, *s = dev_get_drvdata(dev-dev);
 
dev_set_drvdata(dev-dev, NULL);
 
-   for (i = 0; i  sinfo-nskt; i++)
-   soc_pcmcia_remove_one(sinfo-skt[i]);
+   for (; next = s-next, s; s = next) {
+   soc_pcmcia_remove_one(s-soc);
+   kfree(s);
+   }
 
-   kfree(sinfo);
release_mem_region(dev-res.start, 512);
return 0;
 }

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia

Re: [PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread Julia Lawall 
 Just remove the first kzalloc and don't convert it to an array; that's the
 safest option.  I don't remember if there's a reason why I switched to a
 linked list - however, what I will say is that the way all the sa11xx
 and pxa stuff interact is not plainly obvious.

OK, that is what my patch does.  Thanks for looking into it.

julia

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia

Re: [PATCH 3/8] drivers/pcmcia: remove unnecessary kzalloc

2009-11-21 Thread Dominik Brodowski 
Russell,

 Thanks for the additional info which allows me to track which patch it
 corresponds with.  As an aside, it's really not nice to git pull and
 then edit the commits afterwards - that's much worse than rebasing.
 When trees are pulled, the act of merging it conveys sufficent sign-off.

It was not a git pull and edit, as I told you in a mail sent on Nov 02:

 Thanks! I've applied the patches you sent me by mail, though, as your tree
 was based on something more recent from Linus than my tree, and I wanted to
 avoid a merge of Linus' tree into my tree.

Best,
Dominik

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia