[PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

np is initialized to the result of calling a function that calls
of_node_get, so of_node_put should be called before the pointer is dropped.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e,e1,e2;
@@

* e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
  ... when != of_node_put(e)
      when != true e == NULL
      when != e2 = e
  e = e1
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index 96580b1..970ea1d 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
 		return -1;
 
 	np = of_find_node_by_name(NULL, "valkyrie");
-	if (np)
+	if (np) {
 		of_platform_device_create(np, "valkyrie", NULL);
+		of_node_put(np);
+	}
 	np = of_find_node_by_name(NULL, "platinum");
-	if (np)
+	if (np) {
 		of_platform_device_create(np, "platinum", NULL);
+		of_node_put(np);
+	}
         np = of_find_node_by_type(NULL, "smu");
         if (np) {
 		of_platform_device_create(np, "smu", NULL);

[PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

np is initialized to the result of calling a function that calls
of_node_get, so of_node_put should be called before the pointer is dropped.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e,e1,e2;
@@

* e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
  ... when != of_node_put(e)
      when != true e = NULL
      when != e2 = e
  e = e1
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index 96580b1..970ea1d 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
 		return -1;
 
 	np = of_find_node_by_name(NULL, "valkyrie");
-	if (np)
+	if (np) {
 		of_platform_device_create(np, "valkyrie", NULL);
+		of_node_put(np);
+	}
 	np = of_find_node_by_name(NULL, "platinum");
-	if (np)
+	if (np) {
 		of_platform_device_create(np, "platinum", NULL);
+		of_node_put(np);
+	}
         np = of_find_node_by_type(NULL, "smu");
         if (np) {
 		of_platform_device_create(np, "smu", NULL);

[PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

np is initialized to the result of calling a function that calls
of_node_get, so of_node_put should be called before the pointer is dropped.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e,e1,e2;
@@

* e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
  ... when != of_node_put(e)
      when != true e == NULL
      when != e2 = e
  e = e1
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index 96580b1..970ea1d 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
 		return -1;
 
 	np = of_find_node_by_name(NULL, "valkyrie");
-	if (np)
+	if (np) {
 		of_platform_device_create(np, "valkyrie", NULL);
+		of_node_put(np);
+	}
 	np = of_find_node_by_name(NULL, "platinum");
-	if (np)
+	if (np) {
 		of_platform_device_create(np, "platinum", NULL);
+		of_node_put(np);
+	}
         np = of_find_node_by_type(NULL, "smu");
         if (np) {
 		of_platform_device_create(np, "smu", NULL);

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[walter harms]

Am 21.08.2011 18:10, schrieb Julia Lawall:
> From: Julia Lawall <julia@diku.dk>
> 
> np is initialized to the result of calling a function that calls
> of_node_get, so of_node_put should be called before the pointer is dropped.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e,e1,e2;
> @@
> 
> * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
>   ... when != of_node_put(e)
>       when != true e == NULL
>       when != e2 = e
>   e = e1
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> index 96580b1..970ea1d 100644
> --- a/arch/powerpc/platforms/powermac/setup.c
> +++ b/arch/powerpc/platforms/powermac/setup.c
> @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
>  		return -1;
>  
>  	np = of_find_node_by_name(NULL, "valkyrie");
> -	if (np)
> +	if (np) {
>  		of_platform_device_create(np, "valkyrie", NULL);
> +		of_node_put(np);
> +	}
>  	np = of_find_node_by_name(NULL, "platinum");
> -	if (np)
> +	if (np) {
>  		of_platform_device_create(np, "platinum", NULL);
> +		of_node_put(np);
> +	}
>          np = of_find_node_by_type(NULL, "smu");
>          if (np) {
>  		of_platform_device_create(np, "smu", NULL);
> 


hi,
it seems save to call of_node_put(np) with np==NULL, i assume the same is true
for of_platform_device_create().

so the code collapses to:

_test_node(char *name)
{
 struct device_node *np;
 np = of_find_node_by_name(NULL, name);
 of_platform_device_create(np, name, NULL);
 of_node_put(np);
 return NULL?:0:1;
}

maybe there is already something like find_node() ?

re,
 wh

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing

[walter harms]

Am 21.08.2011 18:10, schrieb Julia Lawall:
> From: Julia Lawall <julia@diku.dk>
> 
> np is initialized to the result of calling a function that calls
> of_node_get, so of_node_put should be called before the pointer is dropped.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e,e1,e2;
> @@
> 
> * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
>   ... when != of_node_put(e)
>       when != true e = NULL
>       when != e2 = e
>   e = e1
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> index 96580b1..970ea1d 100644
> --- a/arch/powerpc/platforms/powermac/setup.c
> +++ b/arch/powerpc/platforms/powermac/setup.c
> @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
>  		return -1;
>  
>  	np = of_find_node_by_name(NULL, "valkyrie");
> -	if (np)
> +	if (np) {
>  		of_platform_device_create(np, "valkyrie", NULL);
> +		of_node_put(np);
> +	}
>  	np = of_find_node_by_name(NULL, "platinum");
> -	if (np)
> +	if (np) {
>  		of_platform_device_create(np, "platinum", NULL);
> +		of_node_put(np);
> +	}
>          np = of_find_node_by_type(NULL, "smu");
>          if (np) {
>  		of_platform_device_create(np, "smu", NULL);
> 


hi,
it seems save to call of_node_put(np) with np=NULL, i assume the same is true
for of_platform_device_create().

so the code collapses to:

_test_node(char *name)
{
 struct device_node *np;
 np = of_find_node_by_name(NULL, name);
 of_platform_device_create(np, name, NULL);
 of_node_put(np);
 return NULL?:0:1;
}

maybe there is already something like find_node() ?

re,
 wh

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[walter harms]

Am 21.08.2011 18:10, schrieb Julia Lawall:
> From: Julia Lawall <julia@diku.dk>
> 
> np is initialized to the result of calling a function that calls
> of_node_get, so of_node_put should be called before the pointer is dropped.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e,e1,e2;
> @@
> 
> * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
>   ... when != of_node_put(e)
>       when != true e == NULL
>       when != e2 = e
>   e = e1
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> index 96580b1..970ea1d 100644
> --- a/arch/powerpc/platforms/powermac/setup.c
> +++ b/arch/powerpc/platforms/powermac/setup.c
> @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
>  		return -1;
>  
>  	np = of_find_node_by_name(NULL, "valkyrie");
> -	if (np)
> +	if (np) {
>  		of_platform_device_create(np, "valkyrie", NULL);
> +		of_node_put(np);
> +	}
>  	np = of_find_node_by_name(NULL, "platinum");
> -	if (np)
> +	if (np) {
>  		of_platform_device_create(np, "platinum", NULL);
> +		of_node_put(np);
> +	}
>          np = of_find_node_by_type(NULL, "smu");
>          if (np) {
>  		of_platform_device_create(np, "smu", NULL);
> 


hi,
it seems save to call of_node_put(np) with np==NULL, i assume the same is true
for of_platform_device_create().

so the code collapses to:

_test_node(char *name)
{
 struct device_node *np;
 np = of_find_node_by_name(NULL, name);
 of_platform_device_create(np, name, NULL);
 of_node_put(np);
 return NULL?:0:1;
}

maybe there is already something like find_node() ?

re,
 wh

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e == NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np==NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);

of_platform_device_create seems to do a lot of work if np is NULL.  It 
returns NULL in the end, as far as I can see, but a lot of function calls 
are required.

julia

>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?
> 
> re,
>  wh
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e = NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np=NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);

of_platform_device_create seems to do a lot of work if np is NULL.  It 
returns NULL in the end, as far as I can see, but a lot of function calls 
are required.

julia

>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?
> 
> re,
>  wh
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e == NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np==NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);

of_platform_device_create seems to do a lot of work if np is NULL.  It 
returns NULL in the end, as far as I can see, but a lot of function calls 
are required.

julia

>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?
> 
> re,
>  wh
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing

[walter harms]

>>>  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
>>> index 96580b1..970ea1d 100644
>>> --- a/arch/powerpc/platforms/powermac/setup.c
>>> +++ b/arch/powerpc/platforms/powermac/setup.c
>>> @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
>>>  		return -1;
>>>  
>>>  	np = of_find_node_by_name(NULL, "valkyrie");
>>> -	if (np)
>>> +	if (np) {
>>>  		of_platform_device_create(np, "valkyrie", NULL);
>>> +		of_node_put(np);
>>> +	}
>>>  	np = of_find_node_by_name(NULL, "platinum");
>>> -	if (np)
>>> +	if (np) {
>>>  		of_platform_device_create(np, "platinum", NULL);
>>> +		of_node_put(np);
>>> +	}
>>>          np = of_find_node_by_type(NULL, "smu");
>>>          if (np) {
>>>  		of_platform_device_create(np, "smu", NULL);
>>>
>>
>>
>> hi,
>> it seems save to call of_node_put(np) with np=NULL, i assume the same is true
>> for of_platform_device_create().
>>
>> so the code collapses to:
>>
>> _test_node(char *name)
>> {
>>  struct device_node *np;
>>  np = of_find_node_by_name(NULL, name);
>>  of_platform_device_create(np, name, NULL);
> 
> of_platform_device_create seems to do a lot of work if np is NULL.  It 
> returns NULL in the end, as far as I can see, but a lot of function calls 
> are required.
> 

Yes but it is called only once at init.
but the whole code makes me burp, even if something is found he continues
to search.
How do the other driver handle this ?

re,
 wh



> julia
> 
>>  of_node_put(np);
>>  return NULL?:0:1;
>> }
>>
>> maybe there is already something like find_node() ?
>>
>> re,
>>  wh
>>

> 

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> >>>  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> >>> index 96580b1..970ea1d 100644
> >>> --- a/arch/powerpc/platforms/powermac/setup.c
> >>> +++ b/arch/powerpc/platforms/powermac/setup.c
> >>> @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >>>  		return -1;
> >>>  
> >>>  	np = of_find_node_by_name(NULL, "valkyrie");
> >>> -	if (np)
> >>> +	if (np) {
> >>>  		of_platform_device_create(np, "valkyrie", NULL);
> >>> +		of_node_put(np);
> >>> +	}
> >>>  	np = of_find_node_by_name(NULL, "platinum");
> >>> -	if (np)
> >>> +	if (np) {
> >>>  		of_platform_device_create(np, "platinum", NULL);
> >>> +		of_node_put(np);
> >>> +	}
> >>>          np = of_find_node_by_type(NULL, "smu");
> >>>          if (np) {
> >>>  		of_platform_device_create(np, "smu", NULL);
> >>>
> >>
> >>
> >> hi,
> >> it seems save to call of_node_put(np) with np=NULL, i assume the same is true
> >> for of_platform_device_create().
> >>
> >> so the code collapses to:
> >>
> >> _test_node(char *name)
> >> {
> >>  struct device_node *np;
> >>  np = of_find_node_by_name(NULL, name);
> >>  of_platform_device_create(np, name, NULL);
> > 
> > of_platform_device_create seems to do a lot of work if np is NULL.  It 
> > returns NULL in the end, as far as I can see, but a lot of function calls 
> > are required.
> > 
> 
> Yes but it is called only once at init.
> but the whole code makes me burp, even if something is found he continues
> to search.

Good point.

> How do the other driver handle this ?

I will take a look.

julia

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e == NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np==NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);
>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?

of_platform_device_create is not much used, and one might want to use it 
with other functions than of_find_node_by_name, as indeed is done here.  
Is the problem just a missing else between the calls?  Is it certain that 
all of the cases are disjoint?

julia

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e == NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np==NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);
>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?

of_platform_device_create is not much used, and one might want to use it 
with other functions than of_find_node_by_name, as indeed is done here.  
Is the problem just a missing else between the calls?  Is it certain that 
all of the cases are disjoint?

julia

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e = NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np=NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);
>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?

of_platform_device_create is not much used, and one might want to use it 
with other functions than of_find_node_by_name, as indeed is done here.  
Is the problem just a missing else between the calls?  Is it certain that 
all of the cases are disjoint?

julia

Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put

[Julia Lawall]

On Mon, 22 Aug 2011, walter harms wrote:

> 
> 
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > np is initialized to the result of calling a function that calls
> > of_node_get, so of_node_put should be called before the pointer is dropped.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression e,e1,e2;
> > @@
> > 
> > * e = \(of_find_node_by_type\|of_find_node_by_name\)(...)
> >   ... when != of_node_put(e)
> >       when != true e == NULL
> >       when != e2 = e
> >   e = e1
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  arch/powerpc/platforms/powermac/setup.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> > index 96580b1..970ea1d 100644
> > --- a/arch/powerpc/platforms/powermac/setup.c
> > +++ b/arch/powerpc/platforms/powermac/setup.c
> > @@ -494,11 +494,15 @@ static int __init pmac_declare_of_platform_devices(void)
> >  		return -1;
> >  
> >  	np = of_find_node_by_name(NULL, "valkyrie");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "valkyrie", NULL);
> > +		of_node_put(np);
> > +	}
> >  	np = of_find_node_by_name(NULL, "platinum");
> > -	if (np)
> > +	if (np) {
> >  		of_platform_device_create(np, "platinum", NULL);
> > +		of_node_put(np);
> > +	}
> >          np = of_find_node_by_type(NULL, "smu");
> >          if (np) {
> >  		of_platform_device_create(np, "smu", NULL);
> > 
> 
> 
> hi,
> it seems save to call of_node_put(np) with np==NULL, i assume the same is true
> for of_platform_device_create().
> 
> so the code collapses to:
> 
> _test_node(char *name)
> {
>  struct device_node *np;
>  np = of_find_node_by_name(NULL, name);
>  of_platform_device_create(np, name, NULL);
>  of_node_put(np);
>  return NULL?:0:1;
> }
> 
> maybe there is already something like find_node() ?

of_platform_device_create is not much used, and one might want to use it 
with other functions than of_find_node_by_name, as indeed is done here.  
Is the problem just a missing else between the calls?  Is it certain that 
all of the cases are disjoint?

julia