[PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code
From: Julia Lawall
Initialize ret before returning on failure, as done elsewhere in the
function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
//
Signed-off-by: Julia Lawall
---
sound/ppc/snd_ps3.c |1 +
1 file changed, 1 insertion(+)
diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 1aa52ef..9b18b52 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct
ps3_system_bus_device *dev)
GFP_KERNEL);
if (!the_card.null_buffer_start_vaddr) {
pr_info("%s: nullbuffer alloc failed\n", __func__);
+ ret = -ENOMEM;
goto clean_preallocate;
}
pr_debug("%s: null vaddr=%p dma=%#llx\n", __func__,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code
Please ignore. Wrong patch set.
On Sun, 5 Apr 2015, Julia Lawall wrote:
> From: Julia Lawall
>
> Initialize ret before returning on failure, as done elsewhere in the
> function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> //
> (
> if@p1 (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
>
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> sound/ppc/snd_ps3.c |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
> index 1aa52ef..9b18b52 100644
> --- a/sound/ppc/snd_ps3.c
> +++ b/sound/ppc/snd_ps3.c
> @@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct
> ps3_system_bus_device *dev)
> GFP_KERNEL);
> if (!the_card.null_buffer_start_vaddr) {
> pr_info("%s: nullbuffer alloc failed\n", __func__);
> + ret = -ENOMEM;
> goto clean_preallocate;
> }
> pr_debug("%s: null vaddr=%p dma=%#llx\n", __func__,
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code
From: Julia Lawall
Initialize ret before returning on failure, as done elsewhere in the
function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
//
Signed-off-by: Julia Lawall
---
sound/ppc/snd_ps3.c |1 +
1 file changed, 1 insertion(+)
diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 1aa52ef..9b18b52 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct
ps3_system_bus_device *dev)
GFP_KERNEL);
if (!the_card.null_buffer_start_vaddr) {
pr_info("%s: nullbuffer alloc failed\n", __func__);
+ ret = -ENOMEM;
goto clean_preallocate;
}
pr_debug("%s: null vaddr=%p dma=%#llx\n", __func__,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code
Wrong patch, please ignore.
julia
On Sun, 23 Nov 2014, Julia Lawall wrote:
> From: Julia Lawall
>
> Initialize ret before returning on failure, as done elsewhere in the
> function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> //
> (
> if@p1 (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
>
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> sound/ppc/snd_ps3.c |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
> index 1aa52ef..9b18b52 100644
> --- a/sound/ppc/snd_ps3.c
> +++ b/sound/ppc/snd_ps3.c
> @@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct
> ps3_system_bus_device *dev)
> GFP_KERNEL);
> if (!the_card.null_buffer_start_vaddr) {
> pr_info("%s: nullbuffer alloc failed\n", __func__);
> + ret = -ENOMEM;
> goto clean_preallocate;
> }
> pr_debug("%s: null vaddr=%p dma=%#llx\n", __func__,
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/8] replace memset by memzero_explicit
Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier x;
local idexpression e;
type T,T1;
@@
{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)&x[0]
)
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when != e
when strict
}
@@
identifier i,x;
local idexpression e;
type T;
@@
{
... when any
struct i x;
... when any
when exists
e = (T)&x
... when any
when exists
- memset
+ memzero_explicit
(&x,
-0,
...)
... when != x
when != e
when strict
}
//
@@
identifier x;
type T,T1;
expression e;
@@
{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)&x[0]
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
@@
identifier i,x;
expression e;
type T;
@@
{
... when any
struct i x;
... when any
when exists
when != e = (T)&x
- memset
+ memzero_explicit
(&x,
-0,
...)
... when != x
when strict
}
//
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/8] crypto: replace memset by memzero_explicit
From: Julia Lawall
Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)
//
@@
identifier x;
type T;
@@
{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
//
This change was suggested by Daniel Borkmann
Signed-off-by: Julia Lawall
---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.
arch/powerpc/crypto/sha1.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index 0f88c7b..d3feba5 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -66,7 +66,7 @@ static int sha1_update(struct shash_desc *desc, const u8
*data,
src = data + done;
} while (done + 63 < len);
- memset(temp, 0, sizeof(temp));
+ memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx->buffer + partial, src, len - done);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/8] replace memset by memzero_explicit
Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier x;
local idexpression e;
type T,T1;
@@
{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)&x[0]
)
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when != e
when strict
}
@@
identifier i,x;
local idexpression e;
type T;
@@
{
... when any
struct i x;
... when any
when exists
e = (T)&x
... when any
when exists
- memset
+ memzero_explicit
(&x,
-0,
...)
... when != x
when != e
when strict
}
//
@@
identifier x;
type T,T1;
expression e;
@@
{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)&x[0]
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
@@
identifier i,x;
expression e;
type T;
@@
{
... when any
struct i x;
... when any
when exists
when != e = (T)&x
- memset
+ memzero_explicit
(&x,
-0,
...)
... when != x
when strict
}
//
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/8 v2] crypto: replace memset by memzero_explicit
From: Julia Lawall
Memset on a local variable may be removed when it is called just before the
variable goes out of scope. Using memzero_explicit defeats this
optimization. A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)
//
@@
identifier x;
type T;
@@
{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
(x,
-0,
...)
... when != x
when strict
}
//
This change was suggested by Daniel Borkmann
Signed-off-by: Julia Lawall
---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.
v2: fixed email address
arch/powerpc/crypto/sha1.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index 0f88c7b..d3feba5 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -66,7 +66,7 @@ static int sha1_update(struct shash_desc *desc, const u8
*data,
src = data + done;
} while (done + 63 < len);
- memset(temp, 0, sizeof(temp));
+ memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx->buffer + partial, src, len - done);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test
From: Julia Lawall
Delete successive tests to the same location. The code tested the result
of a previous call, that itself was already tested. It is changed to test
the result of the most recent call.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@s exists@
local idexpression y;
expression x,e;
@@
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
{ ... when forall
return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
{ ... when forall
return ...; }
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/85xx/p1022_ds.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c
b/arch/powerpc/platforms/85xx/p1022_ds.c
index e346edf..af5853c 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -302,7 +302,7 @@ static void p1022ds_set_monitor_port(enum
fsl_diu_monitor_port port)
goto exit;
}
cs1_addr = lbc_br_to_phys(ecm, num_laws, br1);
- if (!cs0_addr) {
+ if (!cs1_addr) {
pr_err("p1022ds: could not determine physical address for CS1"
" (BR1=%08x)\n", br1);
goto exit;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 7/14] cpu: delete unneeded test before of_node_put
From: Julia Lawall Simplify the error path to avoid calling of_node_put when it is not needed. The semantic patch that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e; @@ -if (e) of_node_put(e); // Signed-off-by: Julia Lawall --- arch/powerpc/sysdev/ppc4xx_cpm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/sysdev/ppc4xx_cpm.c b/arch/powerpc/sysdev/ppc4xx_cpm.c index 82e2cfe..ba95adf 100644 --- a/arch/powerpc/sysdev/ppc4xx_cpm.c +++ b/arch/powerpc/sysdev/ppc4xx_cpm.c @@ -281,7 +281,7 @@ static int __init cpm_init(void) printk(KERN_ERR "cpm: could not parse dcr property for %s\n", np->full_name); ret = -EINVAL; - goto out; + goto node_put; } cpm.dcr_host = dcr_map(np, dcr_base, dcr_len); @@ -290,7 +290,7 @@ static int __init cpm_init(void) printk(KERN_ERR "cpm: failed to map dcr property for %s\n", np->full_name); ret = -EINVAL; - goto out; + goto node_put; } /* All 4xx SoCs with a CPM controller have one of two @@ -330,9 +330,9 @@ static int __init cpm_init(void) if (cpm.standby || cpm.suspend) suspend_set_ops(&cpm_suspend_ops); +node_put: + of_node_put(np); out: - if (np) - of_node_put(np); return ret; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/14] powerpc/fsl: fsl_soc: delete unneeded test before of_node_put
From: Julia Lawall Of_node_put supports NULL as its argument, so the initial test is not necessary. Suggested by Uwe Kleine-König. The semantic patch that fixes this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e; @@ -if (e) of_node_put(e); // Signed-off-by: Julia Lawall --- arch/powerpc/sysdev/fsl_soc.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c index ffd1169..bfb5b6c 100644 --- a/arch/powerpc/sysdev/fsl_soc.c +++ b/arch/powerpc/sysdev/fsl_soc.c @@ -197,8 +197,7 @@ static int __init setup_rstcr(void) if (!rstcr && ppc_md.restart == fsl_rstcr_restart) printk(KERN_ERR "No RSTCR register, warm reboot won't work\n"); - if (np) - of_node_put(np); + of_node_put(np); return 0; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/14] powerpc/mpc5xxx: delete unneeded test before of_node_put
From: Julia Lawall Of_node_put supports NULL as its argument, so the initial test is not necessary. Suggested by Uwe Kleine-König. The semantic patch that fixes this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e; @@ -if (e) of_node_put(e); // Signed-off-by: Julia Lawall --- arch/powerpc/sysdev/mpc5xxx_clocks.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c b/arch/powerpc/sysdev/mpc5xxx_clocks.c index 5492dc5..f4f0301 100644 --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c @@ -26,8 +26,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) of_node_put(node); node = np; } - if (node) - of_node_put(node); + of_node_put(node); return p_bus_freq ? *p_bus_freq : 0; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/14] powerpc/pseries: delete unneeded test before of_node_put
From: Julia Lawall Of_node_put supports NULL as its argument, so the initial test is not necessary. Suggested by Uwe Kleine-König. The semantic patch that fixes this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e; @@ -if (e) of_node_put(e); // Signed-off-by: Julia Lawall --- arch/powerpc/platforms/pseries/iommu.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index 33b552f..2ea6831 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -575,8 +575,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus) while (isa_dn && isa_dn != dn) isa_dn = isa_dn->parent; - if (isa_dn_orig) - of_node_put(isa_dn_orig); + of_node_put(isa_dn_orig); /* Count number of direct PCI children of the PHB. */ for (children = 0, tmp = dn->child; tmp; tmp = tmp->sibling) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/14] powerpc: gamecube/wii: delete unneeded test before of_node_put
From: Julia Lawall
Simplify the error path to avoid calling of_node_put when it is not needed.
The semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@@
expression e;
@@
-if (e)
of_node_put(e);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
index 20a8ed9..7feb325 100644
--- a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
+++ b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
@@ -247,7 +247,7 @@ void __init ug_udbg_init(void)
np = of_find_compatible_node(NULL, NULL, "nintendo,flipper-exi");
if (!np) {
udbg_printf("%s: EXI node not found\n", __func__);
- goto done;
+ goto out;
}
exi_io_base = ug_udbg_setup_exi_io_base(np);
@@ -267,8 +267,8 @@ void __init ug_udbg_init(void)
}
done:
- if (np)
- of_node_put(np);
+ of_node_put(np);
+out:
return;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 7/14 v2] powerpc/4xx/cpm: delete unneeded test before of_node_put
From: Julia Lawall Simplify the error path to avoid calling of_node_put when it is not needed. The semantic patch that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e; @@ -if (e) of_node_put(e); // Signed-off-by: Julia Lawall --- v2: new subject arch/powerpc/sysdev/ppc4xx_cpm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/sysdev/ppc4xx_cpm.c b/arch/powerpc/sysdev/ppc4xx_cpm.c index 82e2cfe..ba95adf 100644 --- a/arch/powerpc/sysdev/ppc4xx_cpm.c +++ b/arch/powerpc/sysdev/ppc4xx_cpm.c @@ -281,7 +281,7 @@ static int __init cpm_init(void) printk(KERN_ERR "cpm: could not parse dcr property for %s\n", np->full_name); ret = -EINVAL; - goto out; + goto node_put; } cpm.dcr_host = dcr_map(np, dcr_base, dcr_len); @@ -290,7 +290,7 @@ static int __init cpm_init(void) printk(KERN_ERR "cpm: failed to map dcr property for %s\n", np->full_name); ret = -EINVAL; - goto out; + goto node_put; } /* All 4xx SoCs with a CPM controller have one of two @@ -330,9 +330,9 @@ static int __init cpm_init(void) if (cpm.standby || cpm.suspend) suspend_set_ops(&cpm_suspend_ops); +node_put: + of_node_put(np); out: - if (np) - of_node_put(np); return ret; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/25] fix error return code
These patches fix cases where the return variable is not set to an error code in an error case. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 19/25] fix error return code
From: Julia Lawall
Set the return variable to an error code as done elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
//
Signed-off-by: Julia Lawall
---
Not tested.
drivers/scsi/ps3rom.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index e6e2a30..04d77ce 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -387,6 +387,7 @@ static int ps3rom_probe(struct ps3_system_bus_device *_dev)
if (!host) {
dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed\n",
__func__, __LINE__);
+ error = -ENOMEM;
goto fail_teardown;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/15] don't export static symbol
These patches remove EXPORT_SYMBOL or EXPORT_SYMBOL_GPL declarations on
static functions.
This was done using the following semantic patch:
(http://coccinelle.lip6.fr/)
//
@r@
type T;
identifier f;
@@
static T f (...) { ... }
@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@
-EXPORT_SYMBOL(f);
@@
identifier r.f;
declarer name EXPORT_SYMBOL_GPL;
@@
-EXPORT_SYMBOL_GPL(f);
//
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/15] powerpc: don't export static symbol
From: Julia Lawall
The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r@
type T;
identifier f;
@@
static T f (...) { ... }
@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@
-EXPORT_SYMBOL(f);
//
Furthermore, the function is never used, so its definition is dropped as
well.
Signed-off-by: Julia Lawall
---
arch/powerpc/sysdev/qe_lib/qe_io.c | 25 -
1 file changed, 25 deletions(-)
diff --git a/arch/powerpc/sysdev/qe_lib/qe_io.c
b/arch/powerpc/sysdev/qe_lib/qe_io.c
index d099941..7ea0174 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_io.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_io.c
@@ -190,28 +190,3 @@ int par_io_of_config(struct device_node *np)
return 0;
}
EXPORT_SYMBOL(par_io_of_config);
-
-#ifdef DEBUG
-static void dump_par_io(void)
-{
- unsigned int i;
-
- printk(KERN_INFO "%s: par_io=%p\n", __func__, par_io);
- for (i = 0; i < num_par_io_ports; i++) {
- printk(KERN_INFO " cpodr[%u]=%08x\n", i,
- in_be32(&par_io[i].cpodr));
- printk(KERN_INFO " cpdata[%u]=%08x\n", i,
- in_be32(&par_io[i].cpdata));
- printk(KERN_INFO " cpdir1[%u]=%08x\n", i,
- in_be32(&par_io[i].cpdir1));
- printk(KERN_INFO " cpdir2[%u]=%08x\n", i,
- in_be32(&par_io[i].cpdir2));
- printk(KERN_INFO " cppar1[%u]=%08x\n", i,
- in_be32(&par_io[i].cppar1));
- printk(KERN_INFO " cppar2[%u]=%08x\n", i,
- in_be32(&par_io[i].cppar2));
- }
-
-}
-EXPORT_SYMBOL(dump_par_io);
-#endif /* DEBUG */
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/mpc5xxx: Use of_get_next_parent to simplify code
On Sun, 11 Oct 2015, Christophe JAILLET wrote:
> of_get_next_parent can be used to simplify the while() loop and
> avoid the need of a temp variable.
Can you do something with the loop in __of_translate_address, in
drivers/of/address.c? Is there not an iterator for this?
julia
>
> Signed-off-by: Christophe JAILLET
> ---
> arch/powerpc/sysdev/mpc5xxx_clocks.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c
> b/arch/powerpc/sysdev/mpc5xxx_clocks.c
> index f4f0301..5732926 100644
> --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c
> +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c
> @@ -13,7 +13,6 @@
>
> unsigned long mpc5xxx_get_bus_frequency(struct device_node *node)
> {
> - struct device_node *np;
> const unsigned int *p_bus_freq = NULL;
>
> of_node_get(node);
> @@ -22,9 +21,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node
> *node)
> if (p_bus_freq)
> break;
>
> - np = of_get_parent(node);
> - of_node_put(node);
> - node = np;
> + node = of_get_next_parent(node);
> }
> of_node_put(node);
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/6] add missing of_node_put
The various for_each device_node iterators performs an of_node_get on each
iteration, so a break out of the loop requires an of_node_put.
The complete semantic patch that fixes this problem is
(http://coccinelle.lip6.fr):
//
@r@
local idexpression n;
expression e1,e2;
iterator name for_each_node_by_name, for_each_node_by_type,
for_each_compatible_node, for_each_matching_node,
for_each_matching_node_and_match, for_each_child_of_node,
for_each_available_child_of_node, for_each_node_with_property;
iterator i;
statement S;
expression list [n1] es;
@@
(
(
for_each_node_by_name(n,e1) S
|
for_each_node_by_type(n,e1) S
|
for_each_compatible_node(n,e1,e2) S
|
for_each_matching_node(n,e1) S
|
for_each_matching_node_and_match(n,e1,e2) S
|
for_each_child_of_node(e1,n) S
|
for_each_available_child_of_node(e1,n) S
|
for_each_node_with_property(n,e1) S
)
&
i(es,n,...) S
)
@@
local idexpression r.n;
iterator r.i;
expression e;
expression list [r.n1] es;
@@
i(es,n,...) {
...
(
of_node_put(n);
|
e = n
|
return n;
|
+ of_node_put(n);
? return ...;
)
...
}
@@
local idexpression r.n;
iterator r.i;
expression e;
expression list [r.n1] es;
@@
i(es,n,...) {
...
(
of_node_put(n);
|
e = n
|
+ of_node_put(n);
? break;
)
...
}
... when != n
@@
local idexpression r.n;
iterator r.i;
expression e;
identifier l;
expression list [r.n1] es;
@@
i(es,n,...) {
...
(
of_node_put(n);
|
e = n
|
+ of_node_put(n);
? goto l;
)
...
}
...
l: ... when != n//
---
arch/powerpc/kernel/btext.c |4 +++-
arch/powerpc/kernel/machine_kexec_64.c|4 +++-
arch/powerpc/platforms/cell/iommu.c |1 +
arch/powerpc/platforms/embedded6xx/hlwd-pic.c |1 +
arch/powerpc/platforms/powernv/opal-lpc.c |1 +
arch/powerpc/platforms/pseries/setup.c|3 ++-
6 files changed, 11 insertions(+), 3 deletions(-)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/6] powerpc/6xx: add missing of_node_put
for_each_compatible_node performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.
A simplified version of the semantic patch that fixes this problem is as
follows (http://coccinelle.lip6.fr):
//
@@
expression e;
local idexpression n;
@@
@@
local idexpression n;
expression e;
@@
for_each_compatible_node(n,...) {
...
(
of_node_put(n);
|
e = n
|
+ of_node_put(n);
? break;
)
...
}
... when != n
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/embedded6xx/hlwd-pic.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
index 9b79757..b581b42 100644
--- a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
@@ -215,6 +215,7 @@ void hlwd_pic_probe(void)
irq_set_chained_handler(cascade_virq,
hlwd_pic_irq_cascade);
hlwd_irq_host = host;
+ of_node_put(np);
break;
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/6] powerpc/pseries: add missing of_node_put
for_each_node_by_type performs an of_node_get on each iteration, so a break
out of the loop requires an of_node_put. In this code, there are two paths
that break out of the loop. In the first one (open-pic case), the code
originally contained an of_node_get. That increments the reference count,
which cancels out against the decrement of the reference count that is
needed here, and thus the patch drops the of_node_get. In the second one
(ppc-xicp case), there is no need to save the node value, and so an
of_node_put is added as described by the following semantic patch rule
(http://coccinelle.lip6.fr):
//
@@
expression e,e1;
local idexpression n;
@@
for_each_node_by_type(n, e1) {
... when != of_node_put(n)
when != e = n
(
return n;
|
+ of_node_put(n);
? return ...;
)
...
}
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/pseries/setup.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 36df46e..7185cd3 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -236,7 +236,7 @@ static void __init pseries_discover_pic(void)
for_each_node_by_name(np, "interrupt-controller") {
typep = of_get_property(np, "compatible", NULL);
if (strstr(typep, "open-pic")) {
- pSeries_mpic_node = of_node_get(np);
+ pSeries_mpic_node = np;
ppc_md.init_IRQ = pseries_mpic_init_IRQ;
setup_kexec_cpu_down_mpic();
smp_init_pseries_mpic();
@@ -245,6 +245,7 @@ static void __init pseries_discover_pic(void)
ppc_md.init_IRQ = pseries_xics_init_IRQ;
setup_kexec_cpu_down_xics();
smp_init_pseries_xics();
+ of_node_put(np);
return;
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/6] powerpc/powernv: add missing of_node_put
for_each_compatible_node performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.
A simplified version of the semantic patch that fixes this problem is as
follows (http://coccinelle.lip6.fr):
//
@@
local idexpression n;
expression e;
@@
for_each_compatible_node(n,...) {
...
(
of_node_put(n);
|
e = n
|
+ of_node_put(n);
? break;
)
...
}
... when != n
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/powernv/opal-lpc.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powernv/opal-lpc.c
b/arch/powerpc/platforms/powernv/opal-lpc.c
index e4169d6..d28c4a9 100644
--- a/arch/powerpc/platforms/powernv/opal-lpc.c
+++ b/arch/powerpc/platforms/powernv/opal-lpc.c
@@ -401,6 +401,7 @@ void opal_lpc_init(void)
if (!of_get_property(np, "primary", NULL))
continue;
opal_lpc_chip_id = of_get_ibm_chip_id(np);
+ of_node_put(np);
break;
}
if (opal_lpc_chip_id < 0)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/6] powerpc/btext: add missing of_node_put
for_each_node_by_type performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.
A simplified version of the semantic patch that fixes this problem is as
follows (http://coccinelle.lip6.fr):
//
@@
local idexpression n;
expression e;
@@
for_each_node_by_type(n,...) {
...
(
of_node_put(n);
|
e = n
|
+ of_node_put(n);
? break;
)
...
}
... when != n
//
Signed-off-by: Julia Lawall
---
arch/powerpc/kernel/btext.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 41c011c..8d05ef2 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -257,8 +257,10 @@ int __init btext_find_display(int allow_nonstdout)
rc = btext_initialize(np);
printk("result: %d\n", rc);
}
- if (rc == 0)
+ if (rc == 0) {
+ of_node_put(np);
break;
+ }
}
return rc;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/6] powerpc/kexec: add missing of_node_put
for_each_node_by_type performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.
A simplified version of the semantic patch that fixes this problem is as
follows (http://coccinelle.lip6.fr):
//
@@
expression e,e1;
local idexpression n;
@@
for_each_node_by_type(n, e1) {
... when != of_node_put(n)
when != e = n
(
return n;
|
+ of_node_put(n);
? return ...;
)
...
}
//
Signed-off-by: Julia Lawall
---
arch/powerpc/kernel/machine_kexec_64.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/machine_kexec_64.c
b/arch/powerpc/kernel/machine_kexec_64.c
index 0fbd75d..4ba61ce 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -103,8 +103,10 @@ int default_machine_kexec_prepare(struct kimage *image)
begin = image->segment[i].mem;
end = begin + image->segment[i].memsz;
- if ((begin < high) && (end > low))
+ if ((begin < high) && (end > low)) {
+ of_node_put(node);
return -ETXTBSY;
+ }
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/6] powerpc/cell: add missing of_node_put
for_each_node_by_name performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.
A simplified version of the semantic patch that fixes this problem is as
follows (http://coccinelle.lip6.fr):
//
@@
expression e,e1;
local idexpression n;
@@
for_each_node_by_name(n, e1) {
... when != of_node_put(n)
when != e = n
(
return n;
|
+ of_node_put(n);
? return ...;
)
...
}
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/cell/iommu.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/cell/iommu.c
b/arch/powerpc/platforms/cell/iommu.c
index 14a582b..4edceff 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -1107,6 +1107,7 @@ static int __init cell_iommu_fixed_mapping_init(void)
if (hbase < dbase || (hend > (dbase + dsize))) {
pr_debug("iommu: hash window doesn't fit in"
"real DMA window\n");
+ of_node_put(np);
return -1;
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/4] add NULL test
Add NULL tests on various calls to kzalloc and devm_kzalloc. The semantic match that finds these problems is as follows: (http://coccinelle.lip6.fr/) // @@ expression x,y; identifier fld; @@ ( x = \(vmalloc\|kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|krealloc\| kmemdup\|kstrdup\| devm_kzalloc\|devm_kmalloc\|devm_kcalloc\|devm_kasprintf\| kmalloc_array\)(...,<+... __GFP_NOFAIL ...+>,...); | * x = \(vmalloc\|kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|krealloc\| kmemdup\|kstrdup\| devm_kzalloc\|devm_kmalloc\|devm_kcalloc\|devm_kasprintf\| kmalloc_array\)(...); ) ... when != (x) == NULL when != (x) != NULL when != (x) == 0 when != (x) != 0 when != x = y ( x->fld | *x | x[...] ) // --- drivers/s390/char/con3215.c |2 ++ drivers/s390/char/raw3270.c |2 ++ sound/soc/fsl/imx-pcm-dma.c |2 ++ sound/soc/intel/baytrail/sst-baytrail-pcm.c |2 ++ sound/soc/omap/omap-hdmi-audio.c|2 ++ 5 files changed, 10 insertions(+) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/4] ASoC: imx-pcm-dma: add NULL test
Add NULL test on call to devm_kzalloc. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression x; @@ * x = devm_kzalloc(...); ... when != x == NULL *x // Signed-off-by: Julia Lawall --- sound/soc/fsl/imx-pcm-dma.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c index 1fc01ed..f3d3d1f 100644 --- a/sound/soc/fsl/imx-pcm-dma.c +++ b/sound/soc/fsl/imx-pcm-dma.c @@ -62,6 +62,8 @@ int imx_pcm_dma_init(struct platform_device *pdev, size_t size) config = devm_kzalloc(&pdev->dev, sizeof(struct snd_dmaengine_pcm_config), GFP_KERNEL); + if (!config) + return -ENOMEM; *config = imx_dmaengine_pcm_config; if (size) config->prealloc_buffer_size = size; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] xen/hvc: constify hv_ops structures
These hv_ops structures are never modified, so declare them as const. Most
were const already.
Done with the help of Coccinelle.
Signed-off-by: Julia Lawall
---
drivers/tty/hvc/hvc_xen.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index fa816b7..1505056 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -162,7 +162,7 @@ static int domU_read_console(uint32_t vtermno, char *buf,
int len)
return recv;
}
-static struct hv_ops domU_hvc_ops = {
+static const struct hv_ops domU_hvc_ops = {
.get_chars = domU_read_console,
.put_chars = domU_write_console,
.notifier_add = notifier_add_irq,
@@ -188,7 +188,7 @@ static int dom0_write_console(uint32_t vtermno, const char
*str, int len)
return len;
}
-static struct hv_ops dom0_hvc_ops = {
+static const struct hv_ops dom0_hvc_ops = {
.get_chars = dom0_read_console,
.put_chars = dom0_write_console,
.notifier_add = notifier_add_irq,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/spufs: adjust list element pointer type
Other uses of &gang->aff_list_head, eg in spufs_assert_affinity, indicate
that the list elements have type spu_context, not spu as used here. Change
the type of tmp accordingly.
This has no impact on the execution, because tmp is not used in the body of
the loop.
Fixes: c5fc8d2a92461 ("[CELL] cell: add placement computation for scheduling of
affinity contexts")
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/cell/spufs/sched.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/sched.c
b/arch/powerpc/platforms/cell/spufs/sched.c
index f18d5067cd0f..487fcb47f10d 100644
--- a/arch/powerpc/platforms/cell/spufs/sched.c
+++ b/arch/powerpc/platforms/cell/spufs/sched.c
@@ -344,8 +344,7 @@ static struct spu *aff_ref_location(struct spu_context
*ctx, int mem_aff,
static void aff_set_ref_point_location(struct spu_gang *gang)
{
int mem_aff, gs, lowest_offset;
- struct spu_context *ctx;
- struct spu *tmp;
+ struct spu_context *tmp, *ctx;
mem_aff = gang->aff_ref_ctx->flags & SPU_CREATE_AFFINITY_MEM;
lowest_offset = 0;
Re: [PATCH] powerpc/spufs: adjust list element pointer type
On Fri, 8 May 2020, Jeremy Kerr wrote: > Hi Julia, > > > Other uses of &gang->aff_list_head, eg in spufs_assert_affinity, indicate > > that the list elements have type spu_context, not spu as used here. Change > > the type of tmp accordingly. > > Looks good to me; we could even use ctx there, rather than the separate > tmp variable. I thought about that, but it seemed a little bit abusive, since ctx is used in an iteration over another list. But if you prefer that I can change it. julia > > Reviewed-by: Jeremy Kerr > > Cheers, > > > Jeremy > >
Re: [PATCH v2 1/2] treewide: remove unused address argument from pte_alloc functions
> I wrote something like this as below but it failed to compile, Julia any > suggestions on how to express this? > > @pte_alloc_func_proto depends on patch exists@ > type T1, T2, T3, T4; > identifier fn =~ > "^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$"; > @@ > > ( > - T3 fn(T1, T2); > + T3 fn(T1); > | > - T3 fn(T1, T2, T4); > + T3 fn(T1, T2); > ) What goes wrong? It seems fine to me. julia
Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
On Wed, 5 Dec 2018, Michael Ellerman wrote:
> Hi Dan,
>
> Thanks for the patch.
>
> Dan Carpenter writes:
> > The ipic_info[] array only has 95 elements so I have made the bounds
> > check smaller to prevent a read overflow. It was Smatch that found
> > this issue:
> >
> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > error: buffer overflow 'ipic_info' 95 <= 127
> >
> > Signed-off-by: Dan Carpenter
> > ---
> > I wasn't able to find any callers of this code. Maybe we removed the
> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> > should just remove it. I'm not really comfortable doing that myself,
> > because I don't know the code well enough and can't build test
> > it properly.
>
> Hah wow, last usage removed in 2006!
>
> I don't see any mention of it since then, so I'll remove it. If it
> breaks something we can put it back.
>
> Can smatch help us find things like this that are defined non-static but
> never used?
I wrote a Coccinelle script for this, that just uses grep. Of course the
results need checking because uses can be constructed within macros using
#.
Are things that are defined static but are never used useful to keep
around?
julia
Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
On Wed, 5 Dec 2018, Michael Ellerman wrote:
> Julia Lawall writes:
> > On Wed, 5 Dec 2018, Michael Ellerman wrote:
> >
> >> Hi Dan,
> >>
> >> Thanks for the patch.
> >>
> >> Dan Carpenter writes:
> >> > The ipic_info[] array only has 95 elements so I have made the bounds
> >> > check smaller to prevent a read overflow. It was Smatch that found
> >> > this issue:
> >> >
> >> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> >> > error: buffer overflow 'ipic_info' 95 <= 127
> >> >
> >> > Signed-off-by: Dan Carpenter
> >> > ---
> >> > I wasn't able to find any callers of this code. Maybe we removed the
> >> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> >> > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> >> > should just remove it. I'm not really comfortable doing that myself,
> >> > because I don't know the code well enough and can't build test
> >> > it properly.
> >>
> >> Hah wow, last usage removed in 2006!
> >>
> >> I don't see any mention of it since then, so I'll remove it. If it
> >> breaks something we can put it back.
> >>
> >> Can smatch help us find things like this that are defined non-static but
> >> never used?
> >
> > I wrote a Coccinelle script for this, that just uses grep. Of course the
> > results need checking because uses can be constructed within macros using
> > #.
>
> That would be cool. I can't immediately see it in scripts/coccinelle, is
> it somewhere else?
No, it needs improvement... I'll try to do something with it soon. I
don't think it is well suited to scrips/coccinelle, because it needs to
know where the kernel tree is to do the grep.
julia
>
> > Are things that are defined static but are never used useful to keep
> > around?
>
> No, but the compiler will usually tell us about them via -Wunused-function.
>
> cheers
>
Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
On Thu, 6 Dec 2018, Christophe LEROY wrote:
>
>
> Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
> > Hi Dan,
> >
> > Thanks for the patch.
> >
> > Dan Carpenter writes:
> > > The ipic_info[] array only has 95 elements so I have made the bounds
> > > check smaller to prevent a read overflow. It was Smatch that found
> > > this issue:
> > >
> > > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > > error: buffer overflow 'ipic_info' 95 <= 127
> > >
> > > Signed-off-by: Dan Carpenter
> > > ---
> > > I wasn't able to find any callers of this code. Maybe we removed the
> > > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> > > should just remove it. I'm not really comfortable doing that myself,
> > > because I don't know the code well enough and can't build test
> > > it properly.
> >
> > Hah wow, last usage removed in 2006!
> >
> > I don't see any mention of it since then, so I'll remove it. If it
> > breaks something we can put it back.
> >
> > Can smatch help us find things like this that are defined non-static but
> > never used?
> >
>
> I think we have to do that carrefully. Some of those functions might be used
> by out-of-tree boards.
>
> I'm thinking especially at ipic_get_mcp_status() and ipic_set_mcp_status().
> They are used in my 832x boards's machine check handler to know when a machine
> check is a timeout from the 832x watchdog.
The message I have gotten in the past is that the Linux kernel doesn't
support code that is not used in the Linux kernel. However, if I were to
do this, I would send the code to the individual maintainers, who
presumably would know what is actually needed and what is not.
Perhaps a good sanity check would be if the code has been used in the
past. If there was a use in the past that has been removed, then perhaps
it is more likely that the function was intended for internal kernel use
rather than the case that you are describing.
julia
Re: Coccinelle: Checking of_node_put() calls with SmPL
On Thu, 11 Jul 2019, wen.yan...@zte.com.cn wrote:
> > > we developed a coccinelle script to detect such problems.
> >
> > Would you find the implementation of the function “dt_init_idle_driver”
> > suspicious according to discussed source code search patterns?
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpuidle/dt_idle_states.c?id=e9a83bd2322035ed9d7dcf35753d3f984d76c6a5#n208
> > https://elixir.bootlin.com/linux/v5.2/source/drivers/cpuidle/dt_idle_states.c#L208
> >
> >
> > > This script is still being improved.
> >
> > Will corresponding software development challenges become more interesting?
>
> Hello Markus,
> This is the simplified code pattern for it:
>
> 172 for (i = 0; ; i++) {
> 173 state_node = of_parse_phandle(...); ---> Obtain here
> ...
> 177 match_id = of_match_node(matches, state_node);
> 178 if (!match_id) {
> 179 err = -ENODEV;
> 180 break; ---> Jump out of
> the loop without releasing it
> 181 }
> 182
> 183 if (!of_device_is_available(state_node)) {
> 184 of_node_put(state_node);
> 185 continue;---> Release the
> object references within a loop
> 186 }
> ...
> 208 of_node_put(state_node); --> Release the object
> references within a loop
> 209 }
> 210
> 211 of_node_put(state_node); -->There may be double free
> here.
>
> This code pattern is very interesting and the coccinelle software should also
> recognize this pattern.
In my experience, when you start looking at these of_node_put things, all
sorts of strange things appear...
julia
Re: [PATCH -next] powerpc/pseries: Fix platform_no_drv_owner.cocci warnings
On Mon, 18 Feb 2019, YueHaibing wrote:
> Remove .owner field if calls are used which set it automatically
> Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
>
> Signed-off-by: YueHaibing
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
> b/arch/powerpc/platforms/pseries/papr_scm.c
> index bba281b1fe1b..e3f5c1a01950 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -21,6 +21,7 @@
>(1ul << ND_CMD_GET_CONFIG_DATA) | \
>(1ul << ND_CMD_SET_CONFIG_DATA))
>
> +
No need to add a blank line.
julia
> struct papr_scm_priv {
> struct platform_device *pdev;
> struct device_node *dn;
> @@ -358,7 +359,6 @@ static struct platform_driver papr_scm_driver = {
> .remove = papr_scm_remove,
> .driver = {
> .name = "papr_scm",
> - .owner = THIS_MODULE,
> .of_match_table = papr_scm_match,
> },
> };
>
>
>
>
[PATCH 00/12] add missing of_node_put after of_device_is_available
Failure of of_device_is_available implies that the device node should be put, if it is not used otherwise. --- arch/arm/mach-omap2/display.c|4 +++- arch/powerpc/platforms/83xx/usb.c|4 +++- drivers/bus/arm-cci.c|4 +++- drivers/cpufreq/armada-8k-cpufreq.c |4 +++- drivers/crypto/amcc/crypto4xx_trng.c |4 +++- drivers/firmware/psci.c |4 +++- drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c |4 +++- drivers/gpu/drm/tegra/rgb.c |4 +++- drivers/phy/tegra/xusb.c |4 +++- drivers/soc/amlogic/meson-gx-socinfo.c |4 +++- drivers/tee/optee/core.c |4 +++- drivers/video/fbdev/omap2/omapfb/dss/omapdss-boot-init.c |4 +++- 12 files changed, 36 insertions(+), 12 deletions(-)
[PATCH 03/12] PowerPC-83xx: add missing of_node_put after of_device_is_available
Add an of_node_put when a tested device node is not available.
The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):
//
@@
identifier f;
local idexpression e;
expression x;
@@
e = f(...);
... when != of_node_put(e)
when != x = e
when != e = x
when any
if (<+...of_device_is_available(e)...+>) {
... when != of_node_put(e)
(
return e;
|
+ of_node_put(e);
return ...;
)
}
//
Fixes: c026c98739c7e ("powerpc/83xx: Do not configure or probe disabled FSL DR
USB controllers")
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/83xx/usb.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff -u -p a/arch/powerpc/platforms/83xx/usb.c
b/arch/powerpc/platforms/83xx/usb.c
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -221,8 +221,10 @@ int mpc837x_usb_cfg(void)
int ret = 0;
np = of_find_compatible_node(NULL, NULL, "fsl-usb2-dr");
- if (!np || !of_device_is_available(np))
+ if (!np || !of_device_is_available(np)) {
+ of_node_put(np);
return -ENODEV;
+ }
prop = of_get_property(np, "phy_type", NULL);
if (!prop || (strcmp(prop, "ulpi") && strcmp(prop, "serial"))) {
Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
On Mon, 4 Jun 2018, Dan Carpenter wrote:
> There is a copy and paste bug so we accidentally use the RX_ shift when
> we're in TX_ mode.
>
> Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> Signed-off-by: Dan Carpenter
> ---
> Static analysis work. Not tested. This affects the success path, so
> we should probably test it.
Maybe this is another one? I don't have time to look into it at the
moment...
drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
/* For strict priority entries defines the number of consecutive
* slots for the highest priority.
*/
REG_WR(bp, (port) ? NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS :
NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS, 0x100);
/* Mapping between the CREDIT_WEIGHT registers and actual client
* numbers
*/
I find some others that choose between constants, such as ... ? 0 : 0.
julia
>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index c646d8713861..681f7d4b7724 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32
> tdm_num)
> {
> u32 shift;
>
> - shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
> + shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : TX_SYNC_SHIFT_BASE;
> shift -= tdm_num * 2;
>
> return shift;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()
On Mon, 4 Jun 2018, Dan Carpenter wrote:
> On Mon, Jun 04, 2018 at 10:25:14PM +0900, Julia Lawall wrote:
> >
> >
> > On Mon, 4 Jun 2018, Dan Carpenter wrote:
> >
> > > There is a copy and paste bug so we accidentally use the RX_ shift when
> > > we're in TX_ mode.
> > >
> > > Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> > > Signed-off-by: Dan Carpenter
> > > ---
> > > Static analysis work. Not tested. This affects the success path, so
> > > we should probably test it.
> >
> > Maybe this is another one? I don't have time to look into it at the
> > moment...
> >
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> >
> > /* For strict priority entries defines the number of consecutive
> > * slots for the highest priority.
> > */
> > REG_WR(bp, (port) ? NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS :
> >NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS, 0x100);
> > /* Mapping between the CREDIT_WEIGHT registers and actual client
> > * numbers
> > */
> >
> > I find some others that choose between constants, such as ... ? 0 : 0.
>
> I feel like it should warn about all of those because people shouldn't
> be submitting unfinished written code to the kernel. Coccinelle is a
> lot better for this than Smatch is because it's pre-processor stuff.
OK, maybe I can report these in the next few days.
thanks,
julia
Re: [PATCH v5 2/4] resource: Use list_head to link sibling resource
This looks wrong. After a list iterator, the index variable points to a
dummy structure.
julia
url:
https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600
:: branch date: 7 hours ago
:: commit date: 7 hours ago
>> kernel/resource.c:265:17-20: ERROR: invalid reference to the index variable
>> of the iterator on line 253
#
https://github.com/0day-ci/linux/commit/e906f15906750a86913ba2b1f08bad99129d3dfc
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout e906f15906750a86913ba2b1f08bad99129d3dfc
vim +265 kernel/resource.c
^1da177e4 Linus Torvalds 2005-04-16 247
5eeec0ec9 Yinghai Lu 2009-12-22 248 static void
__release_child_resources(struct resource *r)
5eeec0ec9 Yinghai Lu 2009-12-22 249 {
e906f1590 Baoquan He 2018-06-12 250struct resource *tmp, *next;
5eeec0ec9 Yinghai Lu 2009-12-22 251resource_size_t size;
5eeec0ec9 Yinghai Lu 2009-12-22 252
e906f1590 Baoquan He 2018-06-12 @253list_for_each_entry_safe(tmp,
next, &r->child, sibling) {
5eeec0ec9 Yinghai Lu 2009-12-22 254tmp->parent = NULL;
e906f1590 Baoquan He 2018-06-12 255
INIT_LIST_HEAD(&tmp->sibling);
5eeec0ec9 Yinghai Lu 2009-12-22 256
__release_child_resources(tmp);
5eeec0ec9 Yinghai Lu 2009-12-22 257
5eeec0ec9 Yinghai Lu 2009-12-22 258printk(KERN_DEBUG
"release child resource %pR\n", tmp);
5eeec0ec9 Yinghai Lu 2009-12-22 259/* need to restore
size, and keep flags */
5eeec0ec9 Yinghai Lu 2009-12-22 260size =
resource_size(tmp);
5eeec0ec9 Yinghai Lu 2009-12-22 261tmp->start = 0;
5eeec0ec9 Yinghai Lu 2009-12-22 262tmp->end = size - 1;
5eeec0ec9 Yinghai Lu 2009-12-22 263}
e906f1590 Baoquan He 2018-06-12 264
e906f1590 Baoquan He 2018-06-12 @265INIT_LIST_HEAD(&tmp->child);
5eeec0ec9 Yinghai Lu 2009-12-22 266 }
5eeec0ec9 Yinghai Lu 2009-12-22 267
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH 0/4] use mmgrab
Mmgrab was introduced in commit f1f1007644ff ("mm: add new mmgrab()
helper") and most of the kernel was updated to use it. Update a few
remaining files.
---
arch/openrisc/kernel/smp.c |2 +-
drivers/misc/cxl/context.c |2 +-
drivers/vfio/pci/vfio_pci_nvlink2.c |2 +-
drivers/vfio/vfio_iommu_spapr_tce.c |2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
[PATCH 1/4] misc: cxl: use mmgrab
Mmgrab was introduced in commit f1f1007644ff ("mm: add new mmgrab()
helper") and most of the kernel was updated to use it. Update a
remaining file.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
@@ expression e; @@
- atomic_inc(&e->mm_count);
+ mmgrab(e);
Signed-off-by: Julia Lawall
---
drivers/misc/cxl/context.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index aed9c445d1e2..fb2eff69e449 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -352,7 +352,7 @@ void cxl_context_free(struct cxl_context *ctx)
void cxl_context_mm_count_get(struct cxl_context *ctx)
{
if (ctx->mm)
- atomic_inc(&ctx->mm->mm_count);
+ mmgrab(ctx->mm);
}
void cxl_context_mm_count_put(struct cxl_context *ctx)
[PATCH 00/16] constify copied structure
Make const static structures that are just copied into other structures.
The semantic patch that detects the opportunity for this change is as
follows: (http://coccinelle.lip6.fr/)
@r disable optional_qualifier@
identifier i,j;
position p;
@@
static struct i j@p = { ... };
@upd@
position p1;
identifier r.j;
expression e;
@@
e = j@p1
@ref@
position p2 != {r.p,upd.p1};
identifier r.j;
@@
j@p2
@script:ocaml depends on upd && !ref@
i << r.i;
j << r.j;
p << r.p;
@@
if j = (List.hd p).current_element
then Coccilib.print_main i p
---
arch/powerpc/sysdev/mpic.c |4 ++--
drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c |2 +-
drivers/media/i2c/mt9v111.c |2 +-
drivers/media/platform/davinci/isif.c |2 +-
drivers/media/usb/cx231xx/cx231xx-dvb.c |2 +-
drivers/media/usb/dvb-usb-v2/anysee.c |4 ++--
drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c |2 +-
drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c|2 +-
drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c |2 +-
drivers/ptp/ptp_clockmatrix.c |2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c |2 +-
drivers/video/fbdev/sa1100fb.c |2 +-
net/sunrpc/xdr.c|2 +-
sound/isa/ad1816a/ad1816a_lib.c |2 +-
sound/pci/hda/hda_controller.c |2 +-
sound/soc/qcom/qdsp6/q6asm-dai.c|2 +-
16 files changed, 18 insertions(+), 18 deletions(-)
[PATCH 09/16] powerpc/mpic: constify copied structure
The mpic_ipi_chip and mpic_irq_ht_chip structures are only copied
into other structures, so make them const.
The opportunity for this change was found using Coccinelle.
Signed-off-by: Julia Lawall
---
arch/powerpc/sysdev/mpic.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 934a77324f6b..a3a72b780e67 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -964,7 +964,7 @@ static struct irq_chip mpic_irq_chip = {
};
#ifdef CONFIG_SMP
-static struct irq_chip mpic_ipi_chip = {
+static const struct irq_chip mpic_ipi_chip = {
.irq_mask = mpic_mask_ipi,
.irq_unmask = mpic_unmask_ipi,
.irq_eoi= mpic_end_ipi,
@@ -978,7 +978,7 @@ static struct irq_chip mpic_tm_chip = {
};
#ifdef CONFIG_MPIC_U3_HT_IRQS
-static struct irq_chip mpic_irq_ht_chip = {
+static const struct irq_chip mpic_irq_ht_chip = {
.irq_startup= mpic_startup_ht_irq,
.irq_shutdown = mpic_shutdown_ht_irq,
.irq_mask = mpic_mask_irq,
[PATCH 00/10] use resource_size
Use resource_size rather than a verbose computation on the end and start fields. The semantic patch that makes these changes is as follows: (http://coccinelle.lip6.fr/) @@ struct resource ptr; @@ - ((ptr.end) - (ptr.start) + 1) + resource_size(&ptr) @@ struct resource *ptr; @@ - ((ptr->end) - (ptr->start) + 1) + resource_size(ptr) @@ struct resource ptr; @@ - ((ptr.end) + 1 - (ptr.start)) + resource_size(&ptr) @@ struct resource *ptr; @@ - ((ptr->end) + 1 - (ptr->start)) + resource_size(ptr) --- arch/mips/kernel/setup.c |6 ++ arch/powerpc/platforms/83xx/km83xx.c |2 +- arch/powerpc/platforms/powernv/pci-ioda.c |4 ++-- arch/x86/kernel/crash.c |2 +- drivers/net/ethernet/freescale/fman/mac.c |4 ++-- drivers/usb/gadget/udc/omap_udc.c |6 +++--- drivers/video/fbdev/cg14.c|3 +-- drivers/video/fbdev/s1d13xxxfb.c | 16 sound/drivers/ml403-ac97cr.c |4 +--- sound/soc/sof/imx/imx8.c |3 +-- 10 files changed, 22 insertions(+), 28 deletions(-)
[PATCH 05/10] powerpc/83xx: use resource_size
Use resource_size rather than a verbose computation on the end and start fields. The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) @@ struct resource ptr; @@ - (ptr.end - ptr.start + 1) + resource_size(&ptr) Signed-off-by: Julia Lawall --- arch/powerpc/platforms/83xx/km83xx.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/83xx/km83xx.c b/arch/powerpc/platforms/83xx/km83xx.c index 3d89569e9e71..ada42f03915a 100644 --- a/arch/powerpc/platforms/83xx/km83xx.c +++ b/arch/powerpc/platforms/83xx/km83xx.c @@ -63,7 +63,7 @@ static void quirk_mpc8360e_qe_enet10(void) return; } - base = ioremap(res.start, res.end - res.start + 1); + base = ioremap(res.start, resource_size(&res)); /* * set output delay adjustments to default values according
[PATCH 10/10] powerpc/powernv: use resource_size
Use resource_size rather than a verbose computation on the end and start fields. The semantic patch that makes these changes is as follows: (http://coccinelle.lip6.fr/) @@ struct resource ptr; @@ - (ptr.end - ptr.start + 1) + resource_size(&ptr) Signed-off-by: Julia Lawall --- arch/powerpc/platforms/powernv/pci-ioda.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index da1068a9c263..364140145ce0 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -792,7 +792,7 @@ static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe) fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER; parent = pe->pbus->self; if (pe->flags & PNV_IODA_PE_BUS_ALL) - count = pe->pbus->busn_res.end - pe->pbus->busn_res.start + 1; + count = resource_size(&pe->pbus->busn_res); else count = 1; @@ -874,7 +874,7 @@ static int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe) fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER; parent = pe->pbus->self; if (pe->flags & PNV_IODA_PE_BUS_ALL) - count = pe->pbus->busn_res.end - pe->pbus->busn_res.start + 1; + count = resource_size(&pe->pbus->busn_res); else count = 1;
Re: [PATCH 0/4] use mmgrab
On Fri, 3 Jan 2020, Dan Carpenter wrote:
> On Sun, Dec 29, 2019 at 04:42:54PM +0100, Julia Lawall wrote:
> > Mmgrab was introduced in commit f1f1007644ff ("mm: add new mmgrab()
> > helper") and most of the kernel was updated to use it. Update a few
> > remaining files.
>
> I wonder if there is an automatic way to generate these kind of
> Coccinelle scripts which use inlines instead of open coding. Like maybe
> make a list of one line functions, and then auto generate a recipe. Or
> the mmgrab() function could have multiple lines if the first few were
> just sanity checks for NULL or something...
I tried this at one point (10 years ago!):
https://pages.lip6.fr/Julia.Lawall/acp4is09-lawall.pdf
Perhaps it is worth reviving.
julia
[PATCH 8/9] arch/powerpc/sysdev/fsl_rmu.c: introduce missing kfree
rmu needs to be freed before leaving the function in an error case.
A simplified version of the semantic match that finds the problem is as
follows: (http://coccinelle.lip6.fr)
//
@r exists@
local idexpression x;
statement S;
identifier f1;
position p1,p2;
expression *ptr != NULL;
@@
x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...x...+> }
x->f1
...>
(
return \(0\|<+...x...+>\|ptr\);
|
return@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)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/sysdev/fsl_rmu.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/fsl_rmu.c b/arch/powerpc/sysdev/fsl_rmu.c
index 02445a5..1548578 100644
--- a/arch/powerpc/sysdev/fsl_rmu.c
+++ b/arch/powerpc/sysdev/fsl_rmu.c
@@ -1081,6 +1081,7 @@ int fsl_rio_setup_rmu(struct rio_mport *mport, struct
device_node *node)
if (!msg_addr) {
pr_err("%s: unable to find 'reg' property of message-unit\n",
node->full_name);
+ kfree(rmu);
return -ENOMEM;
}
msg_start = of_read_number(msg_addr, aw);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 12/15] arch/powerpc/sysdev/fsl_pci.c: add missing iounmap
From: Julia Lawall
Add missing iounmap in error handling code, in a case where the function
already preforms iounmap on some other execution path.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
expression e;
statement S,S1;
int ret;
@@
e = \(ioremap\|ioremap_nocache\)(...)
... when != iounmap(e)
if (<+...e...+>) S
... when any
when != iounmap(e)
*if (...)
{ ... when != iounmap(e)
return ...; }
... when any
iounmap(e);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/sysdev/fsl_pci.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 4ce547e..bb025da 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -179,12 +179,12 @@ static void __init setup_pci_atmu(struct pci_controller
*hose,
if (paddr_hi == paddr_lo) {
pr_err("%s: No outbound window space\n", name);
- return ;
+ goto out;
}
if (paddr_lo == 0) {
pr_err("%s: No space for inbound window\n", name);
- return ;
+ goto out;
}
/* setup PCSRBAR/PEXCSRBAR */
@@ -273,6 +273,7 @@ static void __init setup_pci_atmu(struct pci_controller
*hose,
(u64)hose->dma_window_size);
}
+out:
iounmap(pci);
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/6] arch/powerpc/kvm/e500_tlb.c: fix error return code
From: Julia Lawall
Convert a 0 error return code to a negative one, as returned elsewhere in the
function.
A new label is also added to avoid freeing things that are known to not yet
be allocated.
A simplified version of the semantic match that finds the first problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier ret;
expression e,e1,e2,e3,e4,x;
@@
(
if (\(ret != 0\|ret < 0\) || ...) { ... return ...; }
|
ret = 0
)
... when != ret = e1
*x =
\(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\|ioremap\|ioremap_nocache\|devm_ioremap\|devm_ioremap_nocache\)(...);
... when != x = e2
when != ret = e3
*if (x == NULL || ...)
{
... when != ret = e4
* return ret;
}
//
Signed-off-by: Julia Lawall
---
arch/powerpc/kvm/e500_tlb.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index c8f6c58..85fcdf7 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -1176,21 +1176,27 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
}
virt = vmap(pages, num_pages, VM_MAP, PAGE_KERNEL);
- if (!virt)
+ if (!virt) {
+ ret = -ENOMEM;
goto err_put_page;
+ }
privs[0] = kzalloc(sizeof(struct tlbe_priv) * params.tlb_sizes[0],
GFP_KERNEL);
privs[1] = kzalloc(sizeof(struct tlbe_priv) * params.tlb_sizes[1],
GFP_KERNEL);
- if (!privs[0] || !privs[1])
- goto err_put_page;
+ if (!privs[0] || !privs[1]) {
+ ret = -ENOMEM;
+ goto err_privs;
+ }
g2h_bitmap = kzalloc(sizeof(u64) * params.tlb_sizes[1],
GFP_KERNEL);
- if (!g2h_bitmap)
- goto err_put_page;
+ if (!g2h_bitmap) {
+ ret = -ENOMEM;
+ goto err_privs;
+ }
free_gtlb(vcpu_e500);
@@ -1230,10 +1236,11 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
kvmppc_recalc_tlb1map_range(vcpu_e500);
return 0;
-err_put_page:
+err_privs:
kfree(privs[0]);
kfree(privs[1]);
+err_put_page:
for (i = 0; i < num_pages; i++)
put_page(pages[i]);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/5] drivers/net/ethernet/freescale/fs_enet: fix error return code
From: Julia Lawall
Convert a 0 error return code to a negative one, as returned elsewhere in the
function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier ret;
expression e,e1,e2,e3,e4,x;
@@
(
if (\(ret != 0\|ret < 0\) || ...) { ... return ...; }
|
ret = 0
)
... when != ret = e1
*x =
\(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\|ioremap\|ioremap_nocache\|devm_ioremap\|devm_ioremap_nocache\)(...);
... when != x = e2
when != ret = e3
*if (x == NULL || ...)
{
... when != ret = e4
* return ret;
}
//
Signed-off-by: Julia Lawall
---
drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c |4 +++-
drivers/net/ethernet/freescale/fs_enet/mii-fec.c |8 ++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
index 0f2d1a7..1514533 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
@@ -174,8 +174,10 @@ static int __devinit fs_enet_mdio_probe(struct
platform_device *ofdev)
new_bus->phy_mask = ~0;
new_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
- if (!new_bus->irq)
+ if (!new_bus->irq) {
+ ret = -ENOMEM;
goto out_unmap_regs;
+ }
new_bus->parent = &ofdev->dev;
dev_set_drvdata(&ofdev->dev, new_bus);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
index 55bb867..cdf702a 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
@@ -137,8 +137,10 @@ static int __devinit fs_enet_mdio_probe(struct
platform_device *ofdev)
snprintf(new_bus->id, MII_BUS_ID_SIZE, "%x", res.start);
fec->fecp = ioremap(res.start, resource_size(&res));
- if (!fec->fecp)
+ if (!fec->fecp) {
+ ret = -ENOMEM;
goto out_fec;
+ }
if (get_bus_freq) {
clock = get_bus_freq(ofdev->dev.of_node);
@@ -172,8 +174,10 @@ static int __devinit fs_enet_mdio_probe(struct
platform_device *ofdev)
new_bus->phy_mask = ~0;
new_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
- if (!new_bus->irq)
+ if (!new_bus->irq) {
+ ret = -ENOMEM;
goto out_unmap_regs;
+ }
new_bus->parent = &ofdev->dev;
dev_set_drvdata(&ofdev->dev, new_bus);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/5] arch/powerpc/platforms/powernv/pci.c: Remove potential NULL dereferences
From: Julia Lawall
If the NULL test is necessary, the initialization involving a dereference of
the tested value should be moved after the NULL test.
The sematic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@@
type T;
expression E;
identifier i,fld;
statement S;
@@
- T i = E->fld;
+ T i;
... when != E
when != i
if (E == NULL) S
+ i = E->fld;
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/powernv/pci.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci.c
b/arch/powerpc/platforms/powernv/pci.c
index be3cfc5..928e97b 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -287,12 +287,13 @@ static int pnv_pci_read_config(struct pci_bus *bus,
int where, int size, u32 *val)
{
struct pci_controller *hose = pci_bus_to_host(bus);
- struct pnv_phb *phb = hose->private_data;
+ struct pnv_phb *phb;
u32 bdfn = (((uint64_t)bus->number) << 8) | devfn;
s64 rc;
if (hose == NULL)
return PCIBIOS_DEVICE_NOT_FOUND;
+ phb = hose->private_data;
switch (size) {
case 1: {
@@ -331,11 +332,12 @@ static int pnv_pci_write_config(struct pci_bus *bus,
int where, int size, u32 val)
{
struct pci_controller *hose = pci_bus_to_host(bus);
- struct pnv_phb *phb = hose->private_data;
+ struct pnv_phb *phb;
u32 bdfn = (((uint64_t)bus->number) << 8) | devfn;
if (hose == NULL)
return PCIBIOS_DEVICE_NOT_FOUND;
+ phb = hose->private_data;
cfg_dbg("pnv_pci_write_config bus: %x devfn: %x +%x/%x -> %08x\n",
bus->number, devfn, where, size, val);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code
From: Julia Lawall
Initialize ret before returning on failure, as done elsewhere in the
function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
//
Signed-off-by: Julia Lawall
---
sound/ppc/snd_ps3.c |1 +
1 file changed, 1 insertion(+)
diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 1aa52ef..9b18b52 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct
ps3_system_bus_device *dev)
GFP_KERNEL);
if (!the_card.null_buffer_start_vaddr) {
pr_info("%s: nullbuffer alloc failed\n", __func__);
+ ret = -ENOMEM;
goto clean_preallocate;
}
pr_debug("%s: null vaddr=%p dma=%#llx\n", __func__,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/6] i2c: cpm: use devm_ functions
From: Julia Lawall The various devm_ functions allocate memory that is released when a driver detaches. This patch uses these functions for data that is allocated in the probe function of a platform device and is only freed in the remove function. Signed-off-by: Julia Lawall --- Not compiled. drivers/i2c/busses/i2c-cpm.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c index c1e1096..ec97415 100644 --- a/drivers/i2c/busses/i2c-cpm.c +++ b/drivers/i2c/busses/i2c-cpm.c @@ -640,7 +640,7 @@ static int __devinit cpm_i2c_probe(struct platform_device *ofdev) struct cpm_i2c *cpm; const u32 *data; - cpm = kzalloc(sizeof(struct cpm_i2c), GFP_KERNEL); + cpm = devm_kzalloc(&ofdev->dev, sizeof(struct cpm_i2c), GFP_KERNEL); if (!cpm) return -ENOMEM; @@ -683,7 +683,6 @@ out_shut: cpm_i2c_shutdown(cpm); out_free: dev_set_drvdata(&ofdev->dev, NULL); - kfree(cpm); return result; } @@ -697,7 +696,6 @@ static int __devexit cpm_i2c_remove(struct platform_device *ofdev) cpm_i2c_shutdown(cpm); dev_set_drvdata(&ofdev->dev, NULL); - kfree(cpm); return 0; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
On Tue, 4 Sep 2012, Lars-Peter Clausen wrote: > On 09/04/2012 10:42 AM, Artem Bityutskiy wrote: > > Aiaiai! :-) [1] [2] > > > > I've build-tested this using aiaiai and it reports that this change breaks > > the build: > > > > dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < > > ~/tmp/julia2.mbox > > Tested the patch(es) on top of the following commits: > > ba64756 Quick fixes - applied by aiaiai > > 651c6fa JFFS2: don't fail on bitflips in OOB > > e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data > > ea9d312 mtd: cmdlinepart: minor cleanups > > > > > > Failed to build the following commit for configuration > > "powerpc-mpc512x_defconfig" (architecture powerpc)": > > > > 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups > > > > ... > > drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe': > > drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set > > but not used [-Wunused-but-set-variable] > > drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set > > but not used [-Wunused-but-set-variable] > > drivers/built-in.o: In function `mpc5121_nfc_probe': > > mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get' > > make[1]: *** [vmlinux] Error 1 > > > > > > > > I do not really know why, but it seems that clock framework is not > > supported for powerpc. CCing the PPC mailing list. Preserved the context > > below for the PPC people. > > > > I've been bitten by the same issue recently, also cause by one of these > cocci devm patches. devm_clk_get is only available if the generic > clk_get/clk_put implementation is used. Not all architectures do this and > some implement their own clk_get/clk_put, etc functions. Since devm_clk_get > is merely a wrapper around clk_get/clk_put there is no reason why it should > depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically > available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set), > but it is on a different machine right now, will try to submit it later today. Sorry about this. I wasn't aware that devm_clk_get wasn't supported by all architectures, and I have no way of compiling code for these architectures... But I wonder why it is not, since devm-ness doesn't seem to have anything to do with architecture-specific details? It would be really nice to have it for all architectures, because the clock functions are just as (or at least almost as) common as kzalloc, ioremap, etc. thanks, julia ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/5] arch/powerpc/kernel/rtas_flash.c: eliminate possible double free
From: Julia Lawall
The function initialize_flash_pde_data is only called four times. All four
calls are in the function rtas_flash_init, and on the failure of any of the
calls, remove_flash_pde is called on the third argument of each of the
calls. There is thus no need for initialize_flash_pde_data to call
remove_flash_pde on the same argument. remove_flash_pde kfrees the data
field of its argument, and does not clear that field, so this amounts ot a
possible double free.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@r@
identifier f,free,a;
parameter list[n] ps;
type T;
expression e;
@@
f(ps,T a,...) {
... when any
when != a = e
if(...) { ... free(a); ... return ...; }
... when any
}
@@
identifier r.f,r.free;
expression x,a;
expression list[r.n] xs;
@@
* x = f(xs,a,...);
if (...) { ... free(a); ... return ...; }
//
Signed-off-by: Julia Lawall
---
Not tested.
arch/powerpc/kernel/rtas_flash.c |4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index 20b0120..8329190 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -650,10 +650,8 @@ static int initialize_flash_pde_data(const char
*rtas_call_name,
int token;
dp->data = kzalloc(buf_size, GFP_KERNEL);
- if (dp->data == NULL) {
- remove_flash_pde(dp);
+ if (dp->data == NULL)
return -ENOMEM;
- }
/*
* This code assumes that the status int is the first member of the
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] arch/powerpc: Eliminate double sizeof
From: Julia Lawall Taking sizeof the result of sizeof is quite strange and does not seem to be what is wanted here. This was fixed using the following semantic patch. (http://www.emn.fr/x-info/coccinelle/) // @@ expression E; @@ - sizeof ( sizeof (E) - ) @@ type T; @@ - sizeof ( sizeof (T) - ) // Signed-off-by: Julia Lawall --- arch/powerpc/platforms/82xx/pq2ads-pci-pic.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -u -p a/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c b/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c --- a/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c +++ b/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c @@ -186,7 +186,7 @@ out_unmap_regs: iounmap(priv->regs); out_free_bootmem: free_bootmem((unsigned long)priv, -sizeof(sizeof(struct pq2ads_pci_pic))); +sizeof(struct pq2ads_pci_pic)); of_node_put(np); out_unmap_irq: irq_dispose_mapping(irq); ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/6] arch/powerpc/platforms/pseries: Use kasprintf
From: Julia Lawall
kasprintf combines kmalloc and sprintf, and takes care of the size
calculation itself.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
expression a,flag;
expression list args;
statement S;
@@
a =
- \(kmalloc\|kzalloc\)(...,flag)
+ kasprintf(flag,args)
<... when != a
if (a == NULL || ...) S
...>
- sprintf(a,args);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/pseries/dlpar.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff -u -p a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -78,13 +78,12 @@ static struct device_node *dlpar_parse_c
* prepend this to the full_name.
*/
name = (char *)ccwa + ccwa->name_offset;
- dn->full_name = kmalloc(strlen(name) + 2, GFP_KERNEL);
+ dn->full_name = kasprintf(GFP_KERNEL, "/%s", name);
if (!dn->full_name) {
kfree(dn);
return NULL;
}
- sprintf(dn->full_name, "/%s", name);
return dn;
}
@@ -409,15 +408,13 @@ static ssize_t dlpar_cpu_probe(const cha
* directory of the device tree. CPUs actually live in the
* cpus directory so we need to fixup the full_name.
*/
- cpu_name = kzalloc(strlen(dn->full_name) + strlen("/cpus") + 1,
- GFP_KERNEL);
+ cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
if (!cpu_name) {
dlpar_free_cc_nodes(dn);
rc = -ENOMEM;
goto out;
}
- sprintf(cpu_name, "/cpus%s", dn->full_name);
kfree(dn->full_name);
dn->full_name = cpu_name;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/7] arch/powerpc/kernel: Use set_cpus_allowed_ptr
From: Julia Lawall Use set_cpus_allowed_ptr rather than set_cpus_allowed. The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) // @@ expression E1,E2; @@ - set_cpus_allowed(E1, cpumask_of_cpu(E2)) + set_cpus_allowed_ptr(E1, cpumask_of(E2)) @@ expression E; identifier I; @@ - set_cpus_allowed(E, I) + set_cpus_allowed_ptr(E, &I) // Signed-off-by: Julia Lawall --- arch/powerpc/kernel/smp.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff -u -p a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c --- a/arch/powerpc/kernel/smp.c +++ b/arch/powerpc/kernel/smp.c @@ -561,12 +561,12 @@ void __init smp_cpus_done(unsigned int m * se we pin us down to CPU 0 for a short while */ old_mask = current->cpus_allowed; - set_cpus_allowed(current, cpumask_of_cpu(boot_cpuid)); + set_cpus_allowed_ptr(current, cpumask_of(boot_cpuid)); if (smp_ops && smp_ops->setup_cpu) smp_ops->setup_cpu(boot_cpuid); - set_cpus_allowed(current, old_mask); + set_cpus_allowed_ptr(current, &old_mask); snapshot_timebases(); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
question about drivers/macintosh/windfarm_pm91.c
The function wf_smu_remove in the file drivers/macintosh/windfarm_pm91.c ends with the following code: if (wf_smu_slots_fans) kfree(wf_smu_cpu_fans); if (wf_smu_drive_fans) kfree(wf_smu_cpu_fans); if (wf_smu_cpu_fans) kfree(wf_smu_cpu_fans); This looks quite strange. Is it supposed to be if (x) kfree(x); in each case? julia ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: question about drivers/macintosh/windfarm_pm91.c
On Mon, 29 Mar 2010, Benjamin Herrenschmidt wrote: > On Sun, 2010-03-28 at 17:48 +0200, Julia Lawall wrote: > > The function wf_smu_remove in the file drivers/macintosh/windfarm_pm91.c > > ends with the following code: > > > > if (wf_smu_slots_fans) > > kfree(wf_smu_cpu_fans); > > if (wf_smu_drive_fans) > > kfree(wf_smu_cpu_fans); > > if (wf_smu_cpu_fans) > > kfree(wf_smu_cpu_fans); > > > > This looks quite strange. Is it supposed to be if (x) kfree(x); in each > > case? > > Definitely a typo. In fact, the if () aren't even necessary. Patch > welcome :-) I'll send something tomorrow. I will perhaps keep the ifs, to maintain the style of the rest of the function. julia ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] drivers/macintosh: Correct potential double free
From: Julia Lawall The conditionals were testing different values, but then all freeing the same one, which could result in a double free. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression x,e; identifier f; iterator I; statement S; @@ *kfree(x); ... when != &x when != x = e when != I(x,...) S *x // Signed-off-by: Julia Lawall --- drivers/macintosh/windfarm_pm91.c |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/macintosh/windfarm_pm91.c b/drivers/macintosh/windfarm_pm91.c index bea9916..3442732 100644 --- a/drivers/macintosh/windfarm_pm91.c +++ b/drivers/macintosh/windfarm_pm91.c @@ -687,12 +687,9 @@ static int __devexit wf_smu_remove(struct platform_device *ddev) wf_put_control(cpufreq_clamp); /* Destroy control loops state structures */ - if (wf_smu_slots_fans) - kfree(wf_smu_cpu_fans); - if (wf_smu_drive_fans) - kfree(wf_smu_cpu_fans); - if (wf_smu_cpu_fans) - kfree(wf_smu_cpu_fans); + kfree(wf_smu_slots_fans); + kfree(wf_smu_drive_fans); + kfree(wf_smu_cpu_fans); return 0; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/12] arch/powerpc/kernel: Add missing unlock
From: Julia Lawall
Add an unlock before exiting the function.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@r exists@
expression E1;
identifier f;
@@
f (...) { <+...
* spin_lock_irq (E1,...);
... when != E1
* return ...;
...+> }
//
Signed-off-by: Julia Lawall
---
arch/powerpc/kernel/vio.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index 77f6421..2ca69fa 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -644,8 +644,10 @@ void vio_cmo_set_dev_desired(struct vio_dev *viodev,
size_t desired)
found = 1;
break;
}
- if (!found)
+ if (!found) {
+ spin_unlock_irqrestore(&vio_cmo.lock, flags);
return;
+ }
/* Increase/decrease in desired device entitlement */
if (desired >= viodev->cmo.desired) {
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/12] drivers/macintosh: Add missing unlock
From: Julia Lawall
In some error handling cases the lock is not unlocked.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@r exists@
expression E1;
identifier f;
@@
f (...) { <+...
* spin_lock_irqsave (E1,...);
... when != E1
* return ...;
...+> }
//
Signed-off-by: Julia Lawall
---
drivers/macintosh/macio-adb.c |1 +
drivers/macintosh/smu.c |4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/macio-adb.c b/drivers/macintosh/macio-adb.c
index 79119f5..bd6da7a 100644
--- a/drivers/macintosh/macio-adb.c
+++ b/drivers/macintosh/macio-adb.c
@@ -155,6 +155,7 @@ static int macio_adb_reset_bus(void)
while ((in_8(&adb->ctrl.r) & ADB_RST) != 0) {
if (--timeout == 0) {
out_8(&adb->ctrl.r, in_8(&adb->ctrl.r) & ~ADB_RST);
+ spin_unlock_irqrestore(&macio_lock, flags);
return -1;
}
}
diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
index f96feeb..28f75cf 100644
--- a/drivers/macintosh/smu.c
+++ b/drivers/macintosh/smu.c
@@ -1182,8 +1182,10 @@ static ssize_t smu_read_command(struct file *file,
struct smu_private *pp,
return -EOVERFLOW;
spin_lock_irqsave(&pp->lock, flags);
if (pp->cmd.status == 1) {
- if (file->f_flags & O_NONBLOCK)
+ if (file->f_flags & O_NONBLOCK) {
+ spin_unlock_irqrestore(&pp->lock, flags);
return -EAGAIN;
+ }
add_wait_queue(&pp->wait, &wait);
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] arch/powerpc/platforms: Eliminate use after free
From: Julia Lawall
dlpar_free_cc_nodes frees its argument, so dlpar_online_cpu should not be
called on the same value. Skip over the call to dlpar_online_cpu by
jumping directly to out.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
expression E,E2;
@@
dlpar_free_cc_nodes(E)
...
(
E = E2
|
* E
)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/pseries/dlpar.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index e1682bc..1540a41 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -433,6 +433,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t
count)
if (rc) {
dlpar_release_drc(drc_index);
dlpar_free_cc_nodes(dn);
+ goto out;
}
rc = dlpar_online_cpu(dn);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/9] arch/powerpc/platforms/pseries/iommu.c: add missing kfree
From: Julia Lawall
At this point, window has not been stored anywhere, so it has to be freed
before leaving the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@exists@
local idexpression x;
statement S,S1;
expression E;
identifier fl;
expression *ptr != NULL;
@@
x = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...kfree(x)...+> }
when any
when != true x == NULL
x->fl
...>
(
if (x == NULL) S1
|
if (...) { ... when != x
when forall
(
return \(0\|<+...x...+>\|ptr\);
|
* return ...;
)
}
)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/pseries/iommu.c |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 2b20b05..6351af8 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -940,14 +940,14 @@ static u64 enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
if (ret) {
dev_info(&dev->dev, "failed to map direct window for %s: %d\n",
dn->full_name, ret);
- goto out_clear_window;
+ goto out_free_window;
}
ret = prom_add_property(pdn, win64);
if (ret) {
dev_err(&dev->dev, "unable to add dma window property for %s:
%d",
pdn->full_name, ret);
- goto out_clear_window;
+ goto out_free_window;
}
window->device = pdn;
@@ -959,6 +959,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
dma_addr = of_read_number(&create.addr_hi, 2);
goto out_unlock;
+out_free_window:
+ kfree(window);
+
out_clear_window:
remove_ddw(pdn);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
From: Julia Lawall
At this point, ehv_pic has been allocated but not stored anywhere, so it
should be freed before leaving the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@exists@
local idexpression x;
statement S,S1;
expression E;
identifier fl;
expression *ptr != NULL;
@@
x = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...kfree(x)...+> }
when any
when != true x == NULL
x->fl
...>
(
if (x == NULL) S1
|
if (...) { ... when != x
when forall
(
return \(0\|<+...x...+>\|ptr\);
|
* return ...;
)
}
)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/sysdev/ehv_pic.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
index af1a5df..b6731e4 100644
--- a/arch/powerpc/sysdev/ehv_pic.c
+++ b/arch/powerpc/sysdev/ehv_pic.c
@@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
if (!ehv_pic->irqhost) {
of_node_put(np);
+ kfree(ehv_pic);
return;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/4] sound/aoa/fabrics/layout.c: remove unneeded kfree
From: Julia Lawall
The label outnodev is only used when kzalloc has not yet taken place or has
failed, so there is no need for the call for kfree under this label.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier x;
expression E1!=0,E2,E3,E4;
statement S;
iterator I;
@@
(
if (...) { ... when != kfree(x)
when != x = E3
when != E3 = x
* return ...;
}
... when != x = E2
when != I(...,x,...) S
if (...) { ... when != x = E4
kfree(x); ... return ...; }
)
//
Signed-off-by: Julia Lawall
---
sound/aoa/fabrics/layout.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
index 3fd1a7e..552b97a 100644
--- a/sound/aoa/fabrics/layout.c
+++ b/sound/aoa/fabrics/layout.c
@@ -1073,10 +1073,10 @@ static int aoa_fabric_layout_probe(struct soundbus_dev
*sdev)
sdev->pcmid = -1;
list_del(&ldev->list);
layouts_list_items--;
+ kfree(ldev);
outnodev:
of_node_put(sound);
layout_device = NULL;
- kfree(ldev);
return -ENODEV;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] sound/soc/fsl/mpc8610_hpcd.c: add missing of_node_put
From: Julia Lawall
The first change is to add an of_node_put, since codec_np has previously
been allocated. The rest of the patch reorganizes the error handling code
so the only code executed is that which is needed.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
identifier x;
expression E1!=0,E2,E3,E4;
statement S;
iterator I;
@@
(
if (...) { ... when != of_node_put(x)
when != x = E3
when != E3 = x
* return ...;
}
... when != x = E2
when != I(...,x,...) S
if (...) { ... when != x = E4
of_node_put(x); ... return ...; }
)
//
Signed-off-by: Julia Lawall
---
sound/soc/fsl/mpc8610_hpcd.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index a192979..358f0ba 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -345,8 +345,10 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
}
machine_data = kzalloc(sizeof(struct mpc8610_hpcd_data), GFP_KERNEL);
- if (!machine_data)
- return -ENOMEM;
+ if (!machine_data) {
+ ret = -ENOMEM;
+ goto error_alloc;
+ }
machine_data->dai[0].cpu_dai_name = dev_name(&ssi_pdev->dev);
machine_data->dai[0].ops = &mpc8610_hpcd_ops;
@@ -494,7 +496,7 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
ret = platform_device_add(sound_device);
if (ret) {
dev_err(&pdev->dev, "platform device add failed\n");
- goto error;
+ goto error_sound;
}
dev_set_drvdata(&pdev->dev, sound_device);
@@ -502,14 +504,12 @@ static int mpc8610_hpcd_probe(struct platform_device
*pdev)
return 0;
+error_sound:
+ platform_device_unregister(sound_device);
error:
- of_node_put(codec_np);
-
- if (sound_device)
- platform_device_unregister(sound_device);
-
kfree(machine_data);
-
+error_alloc:
+ of_node_put(codec_np);
return ret;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] sound/soc/fsl/fsl_dma.c: add missing of_node_put
From: Julia Lawall of_parse_phandle increments the reference count of np, so this should be decremented before trying the next possibility. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e,e1,e2; @@ *e = of_parse_phandle(...) ... when != of_node_put(e) when != true e == NULL when != e2 = e e = e1 // Signed-off-by: Julia Lawall --- sound/soc/fsl/fsl_dma.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index 0efc04a..b33271b 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); if (np == dma_channel_np) return ssi_np; + of_node_put(np); np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); if (np == dma_channel_np) return ssi_np; + of_node_put(np); } return NULL; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put
From: Julia Lawall
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/)
//
@@
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
//
Signed-off-by: Julia Lawall
---
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);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] arch/powerpc/platforms/cell/iommu.c: add missing of_node_put
From: Julia Lawall
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/)
//
@@
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
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/cell/iommu.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/cell/iommu.c
b/arch/powerpc/platforms/cell/iommu.c
index 26a0671..d0c5dfd 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -1038,6 +1038,8 @@ static int __init cell_iommu_fixed_mapping_init(void)
/* The fixed mapping is only supported on axon machines */
np = of_find_node_by_name(NULL, "axon");
+ of_node_put(np);
+
if (!np) {
pr_debug("iommu: fixed mapping disabled, no axons found\n");
return -1;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put
On Mon, 22 Aug 2011, walter harms wrote:
>
>
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall
> >
> > 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/)
> >
> > //
> > @@
> > 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
> > //
> >
> > Signed-off-by: Julia Lawall
> >
> > ---
> > 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] arch/powerpc/platforms/powermac/setup.c: add missing of_node_put
On Mon, 22 Aug 2011, walter harms wrote:
>
>
> Am 21.08.2011 18:10, schrieb Julia Lawall:
> > From: Julia Lawall
> >
> > 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/)
> >
> > //
> > @@
> > 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
> > //
> >
> > Signed-off-by: Julia Lawall
> >
> > ---
> > 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] sound/soc/fsl/fsl_dma.c: add missing of_node_put
On Mon, 22 Aug 2011, Timur Tabi wrote: > Julia Lawall wrote: > > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c > > index 0efc04a..b33271b 100644 > > --- a/sound/soc/fsl/fsl_dma.c > > +++ b/sound/soc/fsl/fsl_dma.c > > @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct > > device_node *dma_channel_np) > > np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); > > if (np == dma_channel_np) > > return ssi_np; > > + of_node_put(np); > > > > np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); > > if (np == dma_channel_np) > > return ssi_np; > > + of_node_put(np); > > } > > Thanks for catching the problem, Julia, but the fix is not quite correct. My > code assumes that of_parse_phandle() doesn't claim the node, but it doesn't > actually use the node pointer, either. All I care about is whether 'np' is > equal to dma_channel_np. I'm not going to use 'np'. So I think the real fix > is > this: > > @@ -880,10 +880,12 @@ static struct device_node *find_ssi_node(struct > device_node *dma_channel_np) > np = of_parse_phandle(ssi_np, "fsl,playback-dma", 0); > + of_node_put(np); > if (np == dma_channel_np) > return ssi_np; > > np = of_parse_phandle(ssi_np, "fsl,capture-dma", 0); > + of_node_put(np); > if (np == dma_channel_np) > return ssi_np; > } > > return NULL; OK, that looks reasonable. julia ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/10] arch/powerpc/platforms/pseries: Use kstrdup
From: Julia Lawall Use kstrdup when the goal of an allocation is copy a string into the allocated region. The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) // @@ expression from,to; expression flag,E1,E2; statement S; @@ - to = kmalloc(strlen(from) + 1,flag); + to = kstrdup(from, flag); ... when != \(from = E1 \| to = E1 \) if (to==NULL || ...) S ... when != \(from = E2 \| to = E2 \) - strcpy(to, from); // Signed-off-by: Julia Lawall --- arch/powerpc/platforms/pseries/reconfig.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff -u -p a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c --- a/arch/powerpc/platforms/pseries/reconfig.c +++ b/arch/powerpc/platforms/pseries/reconfig.c @@ -118,12 +118,10 @@ static int pSeries_reconfig_add_node(con if (!np) goto out_err; - np->full_name = kmalloc(strlen(path) + 1, GFP_KERNEL); + np->full_name = kstrdup(path, GFP_KERNEL); if (!np->full_name) goto out_err; - strcpy(np->full_name, path); - np->properties = proplist; of_node_set_flag(np, OF_DYNAMIC); kref_init(&np->kref); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 10/10] arch/powerpc/platforms/iseries: Use kstrdup
From: Julia Lawall
Use kstrdup when the goal of an allocation is copy a string into the
allocated region.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
expression from,to;
expression flag,E1,E2;
statement S;
@@
- to = kmalloc(strlen(from) + 1,flag);
+ to = kstrdup(from, flag);
... when != \(from = E1 \| to = E1 \)
if (to==NULL || ...) S
... when != \(from = E2 \| to = E2 \)
- strcpy(to, from);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/iseries/vio.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff -u -p a/arch/powerpc/platforms/iseries/vio.c
b/arch/powerpc/platforms/iseries/vio.c
--- a/arch/powerpc/platforms/iseries/vio.c
+++ b/arch/powerpc/platforms/iseries/vio.c
@@ -87,12 +87,11 @@ static struct device_node *new_node(cons
if (!np)
return NULL;
- np->full_name = kmalloc(strlen(path) + 1, GFP_KERNEL);
+ np->full_name = kstrdup(path, GFP_KERNEL);
if (!np->full_name) {
kfree(np);
return NULL;
}
- strcpy(np->full_name, path);
of_node_set_flag(np, OF_DYNAMIC);
kref_init(&np->kref);
np->parent = of_node_get(parent);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RFC] usb gadget: introduce usb_gadget_probe_driver
> diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
> index 731150d..c266c1e 100644
> --- a/drivers/usb/gadget/amd5536udc.c
> +++ b/drivers/usb/gadget/amd5536udc.c
> @@ -1954,13 +1954,14 @@ static int setup_ep0(struct udc *dev)
> }
>
> /* Called by gadget driver to register itself */
> -int usb_gadget_register_driver(struct usb_gadget_driver *driver)
> +int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
> + int (*bind)(struct usb_gadget *))
> {
> struct udc *dev = udc;
> int retval;
> u32 tmp;
>
> - if (!driver || !driver->bind || !driver->setup
> + if (!driver || bind || !driver->setup
Is it intentional that !driver->bind has become just bind in this case?
That doesn't seem to happen in the other cases.
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] usb gadget: don't save bind callback in struct usb_gadget_driver
> diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
> index 731150d..c266c1e 100644
> --- a/drivers/usb/gadget/amd5536udc.c
> +++ b/drivers/usb/gadget/amd5536udc.c
> @@ -1954,13 +1954,14 @@ static int setup_ep0(struct udc *dev)
> }
>
> /* Called by gadget driver to register itself */
> -int usb_gadget_register_driver(struct usb_gadget_driver *driver)
> +int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
> + int (*bind)(struct usb_gadget *))
> {
> struct udc *dev = udc;
> int retval;
> u32 tmp;
>
> - if (!driver || !driver->bind || !driver->setup
> + if (!driver || bind || !driver->setup
I have the impression that this should be !bind rather than bind.
That would be like what is done in the patch for
drivers/usb/gadget/fsl_udc_core.c below
> diff --git a/drivers/usb/gadget/fsl_udc_core.c
> b/drivers/usb/gadget/fsl_udc_core.c
> index 08a9a62..c16b402 100644
> --- a/drivers/usb/gadget/fsl_udc_core.c
> +++ b/drivers/usb/gadget/fsl_udc_core.c
> @@ -1765,7 +1765,8 @@ static irqreturn_t fsl_udc_irq(int irq, void *_udc)
> * Hook to gadget drivers
> * Called by initialization code of gadget drivers
> **/
> -int usb_gadget_register_driver(struct usb_gadget_driver *driver)
> +int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
> + int (*bind)(struct usb_gadget *))
> {
> int retval = -ENODEV;
> unsigned long flags = 0;
> @@ -1775,8 +1776,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver
> *driver)
>
> if (!driver || (driver->speed != USB_SPEED_FULL
> && driver->speed != USB_SPEED_HIGH)
> - || !driver->bind || !driver->disconnect
> - || !driver->setup)
> + || !bind || !driver->disconnect || !driver->setup)
> return -EINVAL;
Here !driver->bind is converted to !bind.
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] arch/powerpc: Drop unnecessary of_node_put
From: Julia Lawall
for_each_node_by_name only exits when its first argument is NULL, and a
subsequent call to of_node_put on that argument is unnecessary.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
iterator name for_each_node_by_name;
expression np,E;
identifier l;
@@
for_each_node_by_name(np,...) {
... when != break;
when != goto l;
}
... when != np = E
- of_node_put(np);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/powermac/feature.c |1 -
arch/powerpc/platforms/powermac/pci.c |2 --
2 files changed, 3 deletions(-)
diff -u -p a/arch/powerpc/platforms/powermac/feature.c
b/arch/powerpc/platforms/powermac/feature.c
--- a/arch/powerpc/platforms/powermac/feature.c
+++ b/arch/powerpc/platforms/powermac/feature.c
@@ -2872,7 +2872,6 @@ set_initial_features(void)
core99_airport_enable(np, 0, 0);
}
}
- of_node_put(np);
}
/* On all machines that support sound PM, switch sound off */
diff -u -p a/arch/powerpc/platforms/powermac/pci.c
b/arch/powerpc/platforms/powermac/pci.c
--- a/arch/powerpc/platforms/powermac/pci.c
+++ b/arch/powerpc/platforms/powermac/pci.c
@@ -1155,13 +1155,11 @@ void __init pmac_pcibios_after_init(void
pmac_call_feature(PMAC_FTR_1394_CABLE_POWER, nd, 0, 0);
}
}
- of_node_put(nd);
for_each_node_by_name(nd, "ethernet") {
if (nd->parent && of_device_is_compatible(nd, "gmac")
&& of_device_is_compatible(nd->parent, "uni-north"))
pmac_call_feature(PMAC_FTR_GMAC_ENABLE, nd, 0, 0);
}
- of_node_put(nd);
}
void pmac_pci_fixup_cardbus(struct pci_dev* dev)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/9] arch/powerpc/platforms/powermac: Drop unnecessary null test
From: Julia Lawall
for_each_node_by_name binds its first argument to a non-null value, and
thus any null test on the value of that argument is superfluous.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
iterator I;
expression x,E;
@@
I(x,...) { <...
(
- (x != NULL) &&
E
...> }
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/powermac/feature.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powermac/feature.c
b/arch/powerpc/platforms/powermac/feature.c
index 9e1b9fd..537957b 100644
--- a/arch/powerpc/platforms/powermac/feature.c
+++ b/arch/powerpc/platforms/powermac/feature.c
@@ -2867,7 +2867,7 @@ set_initial_features(void)
/* Switch airport off */
for_each_node_by_name(np, "radio") {
- if (np && np->parent == macio_chips[0].of_node) {
+ if (np->parent == macio_chips[0].of_node) {
macio_chips[0].flags |= MACIO_FLAG_AIRPORT_ON;
core99_airport_enable(np, 0, 0);
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 8/9] arch/powerpc/kernel: Drop unnecessary null test
From: Julia Lawall
list_for_each_entry binds its first argument to a non-null value, and thus
any null test on the value of that argument is superfluous.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
iterator I;
expression x,E,E1,E2;
statement S,S1,S2;
@@
I(x,...) { <...
- if (x != NULL || ...)
S
...> }
//
Signed-off-by: Julia Lawall
---
arch/powerpc/kernel/pci_of_scan.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/pci_of_scan.c
b/arch/powerpc/kernel/pci_of_scan.c
index 6ddb795..62dd363 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -336,8 +336,7 @@ static void __devinit __of_scan_bus(struct device_node
*node,
if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
struct device_node *child = pci_device_to_OF_node(dev);
- if (dev)
- of_scan_pci_bridge(child, dev);
+ of_scan_pci_bridge(child, dev);
}
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_path.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
//
Signed-off-by: Julia Lawall
---
drivers/macintosh/via-pmu-led.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/macintosh/via-pmu-led.c b/drivers/macintosh/via-pmu-led.c
index d242976..19c3718 100644
--- a/drivers/macintosh/via-pmu-led.c
+++ b/drivers/macintosh/via-pmu-led.c
@@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
if (dt == NULL)
return -ENODEV;
model = of_get_property(dt, "model", NULL);
- if (model == NULL)
+ if (model == NULL) {
+ of_node_put(dt);
return -ENODEV;
+ }
if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
strncmp(model, "iBook", strlen("iBook")) != 0 &&
strcmp(model, "PowerMac7,2") != 0 &&
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/7] arch/powerpc/platforms/maple/setup.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_path.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/maple/setup.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/maple/setup.c
b/arch/powerpc/platforms/maple/setup.c
index 3fff8d9..fe34c3d 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -358,6 +358,7 @@ static int __init maple_cpc925_edac_setup(void)
model = (const unsigned char *)of_get_property(np, "model", NULL);
if (!model) {
printk(KERN_ERR "%s: Unabel to get model info\n", __func__);
+ of_node_put(np);
return -ENODEV;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/7] arch/powerpc/sysdev/qe_lib/qe.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/sysdev/qe_lib/qe.c |1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 3da8014..90020de 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -640,6 +640,7 @@ unsigned int qe_get_num_of_snums(void)
if ((num_of_snums < 28) || (num_of_snums > QE_NUM_OF_SNUM)) {
/* No QE ever has fewer than 28 SNUMs */
pr_err("QE: number of snum is invalid\n");
+ of_node_put(qe);
return -EINVAL;
}
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/7] arch/powerpc/platforms/powermac/pfunc_core.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_phandle.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/powermac/pfunc_core.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c
b/arch/powerpc/platforms/powermac/pfunc_core.c
index cec6359..b0c3777 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -837,8 +837,10 @@ struct pmf_function *__pmf_find_function(struct
device_node *target,
return NULL;
find_it:
dev = pmf_find_device(actor);
- if (dev == NULL)
- return NULL;
+ if (dev == NULL) {
+ result = NULL;
+ goto out;
+ }
list_for_each_entry(func, &dev->functions, link) {
if (name && strcmp(name, func->name))
@@ -850,8 +852,9 @@ struct pmf_function *__pmf_find_function(struct device_node
*target,
result = func;
break;
}
- of_node_put(actor);
pmf_put_device(dev);
+out:
+ of_node_put(actor);
return result;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 7/7] arch/powerpc/platforms/cell: Add of_node_put to avoid memory leak
Add calls to of_node_put in the error handling code following calls to
of_find_node_by_path and of_find_node_by_phandle.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
of_node_put(x);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/cell/ras.c|4 +++-
arch/powerpc/platforms/cell/spider-pic.c |4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/cell/ras.c
b/arch/powerpc/platforms/cell/ras.c
index 1d3c4ef..5ec1e47 100644
--- a/arch/powerpc/platforms/cell/ras.c
+++ b/arch/powerpc/platforms/cell/ras.c
@@ -173,8 +173,10 @@ static int __init cbe_ptcal_enable(void)
return -ENODEV;
size = of_get_property(np, "ibm,cbe-ptcal-size", NULL);
- if (!size)
+ if (!size) {
+ of_node_put(np);
return -ENODEV;
+ }
pr_debug("%s: enabling PTCAL, size = 0x%x\n", __func__, *size);
order = get_order(*size);
diff --git a/arch/powerpc/platforms/cell/spider-pic.c
b/arch/powerpc/platforms/cell/spider-pic.c
index 5876e88..3f2e557 100644
--- a/arch/powerpc/platforms/cell/spider-pic.c
+++ b/arch/powerpc/platforms/cell/spider-pic.c
@@ -258,8 +258,10 @@ static unsigned int __init
spider_find_cascade_and_node(struct spider_pic *pic)
return NO_IRQ;
imap += intsize + 1;
tmp = of_get_property(iic, "#interrupt-cells", NULL);
- if (tmp == NULL)
+ if (tmp == NULL) {
+ of_node_put(iic);
return NO_IRQ;
+ }
intsize = *tmp;
/* Assume unit is last entry of interrupt specifier */
unit = imap[intsize - 1];
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] arch/powerpc/platforms/83xx/mpc837x_mds.c: Add missing iounmap
The function of_iomap returns the result of calling ioremap, so iounmap
should be called on the result in the error handling code, as done in the
normal exit of the function.
The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1;
identifier l;
statement S;
@@
*x = of_iomap(...);
... when != iounmap(x)
when != if (...) { ... iounmap(x); ... }
when != E = x
when any
(
if (x == NULL) S
|
if (...) {
... when != iounmap(x)
when != if (...) { ... iounmap(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
)
... when != x = E1
when any
iounmap(x);
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/83xx/mpc837x_mds.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/mpc837x_mds.c
b/arch/powerpc/platforms/83xx/mpc837x_mds.c
index f9751c8..8306832 100644
--- a/arch/powerpc/platforms/83xx/mpc837x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc837x_mds.c
@@ -48,8 +48,10 @@ static int mpc837xmds_usb_cfg(void)
return -1;
np = of_find_node_by_name(NULL, "usb");
- if (!np)
- return -ENODEV;
+ if (!np) {
+ ret = -ENODEV;
+ goto out;
+ }
phy_type = of_get_property(np, "phy_type", NULL);
if (phy_type && !strcmp(phy_type, "ulpi")) {
clrbits8(bcsr_regs + 12, BCSR12_USB_SER_PIN);
@@ -65,8 +67,9 @@ static int mpc837xmds_usb_cfg(void)
}
of_node_put(np);
+out:
iounmap(bcsr_regs);
- return 0;
+ return ret;
}
/*
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node or of_find_node_by_type.
This patch also substantially reorganizes the error handling code in the
function, to that it is possible first to jump to code that frees qe_port
and then to jump to code that also puts np.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
//
Signed-off-by: Julia Lawall
---
drivers/serial/ucc_uart.c | 67 --
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index 3f4848e..38a5ef0 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
ret = of_address_to_resource(np, 0, &res);
if (ret) {
dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
- kfree(qe_port);
- return ret;
+ goto out_free;
}
if (!res.start) {
dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
- kfree(qe_port);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
qe_port->port.mapbase = res.start;
@@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
if (!iprop) {
iprop = of_get_property(np, "device-id", NULL);
if (!iprop) {
- kfree(qe_port);
dev_err(&ofdev->dev, "UCC is unspecified in "
"device tree\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
}
if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
qe_port->ucc_num = *iprop - 1;
@@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
sprop = of_get_property(np, "rx-clock-name", NULL);
if (!sprop) {
dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
qe_port->us_info.rx_clock = qe_clock_source(sprop);
if ((qe_port->us_info.rx_clock < QE_BRG1) ||
(qe_port->us_info.rx_clock > QE_BRG16)) {
dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
#ifdef LOOPBACK
@@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
sprop = of_get_property(np, "tx-clock-name", NULL);
if (!sprop) {
dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
qe_port->us_info.tx_clock = qe_clock_source(sprop);
#endif
if ((qe_port->us_info.tx_clock < QE_BRG1) ||
(qe_port->us_info.tx_clock > QE_BRG16)) {
dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
/* Get the port number, numbered 0-3 */
iprop = of_get_property(np, "port-number", NULL);
if (!iprop) {
dev_err(&ofdev->dev, "missing port-number in device tree\n");
- kfree(qe_port);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
qe_port->port.line = *iprop;
if (qe_port->port.line >=
[PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_type.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/chrp/nvram.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/chrp/nvram.c
b/arch/powerpc/platforms/chrp/nvram.c
index ba3588f..d3ceff0 100644
--- a/arch/powerpc/platforms/chrp/nvram.c
+++ b/arch/powerpc/platforms/chrp/nvram.c
@@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
return;
nbytes_p = of_get_property(nvram, "#bytes", &proplen);
- if (nbytes_p == NULL || proplen != sizeof(unsigned int))
+ if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
+ of_node_put(nvram);
return;
+ }
nvram_size = *nbytes_p;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
On Tue, 31 Aug 2010, walter harms wrote:
>
>
> Julia Lawall schrieb:
> > Add a call to of_node_put in the error handling code following a call to
> > of_find_node_by_path.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > //
> > @r exists@
> > local idexpression x;
> > expression E,E1;
> > statement S;
> > @@
> >
> > *x =
> > (of_find_node_by_path
> > |of_find_node_by_name
> > |of_find_node_by_phandle
> > |of_get_parent
> > |of_get_next_parent
> > |of_get_next_child
> > |of_find_compatible_node
> > |of_match_node
> > )(...);
> > ...
> > if (x == NULL) S
> > <... when != x = E
> > *if (...) {
> > ... when != of_node_put(x)
> > when != if (...) { ... of_node_put(x); ... }
> > (
> > return <+...x...+>;
> > |
> > * return ...;
> > )
> > }
> > ...>
> > of_node_put(x);
> > //
> >
> > Signed-off-by: Julia Lawall
> >
> > ---
> > drivers/macintosh/via-pmu-led.c |4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/macintosh/via-pmu-led.c
> > b/drivers/macintosh/via-pmu-led.c
> > index d242976..19c3718 100644
> > --- a/drivers/macintosh/via-pmu-led.c
> > +++ b/drivers/macintosh/via-pmu-led.c
> > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > if (dt == NULL)
> > return -ENODEV;
> > model = of_get_property(dt, "model", NULL);
> > - if (model == NULL)
> > + if (model == NULL) {
> > + of_node_put(dt);
> > return -ENODEV;
> > + }
> > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > strcmp(model, "PowerMac7,2") != 0 &&
> >
>
> is there any rule that says when to use strncmp ? it seems perfecly valid to
> use strcpy here
> (what is done in the last cmp).
Perhaps there are some characters after eg PowerBook that one doesn't want
to compare with?
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
question about arch/powerpc/platforms/cell/celleb_scc_uhc.c
The file arch/powerpc/platforms/cell/celleb_scc_uhc.c contains the
following function:
static inline int uhc_clkctrl_ready(u32 val)
{
const u32 mask = SCC_UHC_USBCEN | SCC_UHC_USBCEN;
return((val & mask) == mask);
}
The variable mask is a bit or of two identical constants. Later in the
same file in the function enable_scc_uhc, I see the code:
val |= (SCC_UHC_USBCEN | SCC_UHC_USBEN);
Should the code in uhc_clkctrl_ready also be SCC_UHC_USBCEN | SCC_UHC_USBEN?
Or something else?
thanks,
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: question about arch/powerpc/platforms/cell/celleb_scc_uhc.c
On Wed, 1 Sep 2010, Joe Perches wrote:
> On Wed, 2010-09-01 at 17:51 +0200, Julia Lawall wrote:
> > The file arch/powerpc/platforms/cell/celleb_scc_uhc.c contains the
> > following function:
> > static inline int uhc_clkctrl_ready(u32 val)
> > {
> > const u32 mask = SCC_UHC_USBCEN | SCC_UHC_USBCEN;
> > return((val & mask) == mask);
> > }
> > The variable mask is a bit or of two identical constants. Later in the
> > same file in the function enable_scc_uhc, I see the code:
> > val |= (SCC_UHC_USBCEN | SCC_UHC_USBEN);
> > Should the code in uhc_clkctrl_ready also be SCC_UHC_USBCEN | SCC_UHC_USBEN?
> > Or something else?
>
> Thanks Julia.
>
> This was also noticed a couple of years ago and not applied
> http://lkml.indiana.edu/hypermail/linux/kernel/0808.2/1428.html
>
> One day...
OK, thanks, I will send a patch shortly.
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] arch/powerpc/platforms/52xx/efika.c: Add of_node_put to avoid memory leak
This function is implemented as though the function of_get_next_child does
not increment the reference count of its result, but actually it does.
Thus the patch adds of_node_put in error handling code and drops a call to
of_node_get.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E1;
position p1,p2;
@@
x...@p1 = of_get_next_child(...);
... when != x = E1
of_node_...@p2(x)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
cocci.print_main("call",p1)
cocci.print_secs("get",p2)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/52xx/efika.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/efika.c
b/arch/powerpc/platforms/52xx/efika.c
index 45c0cb9..18c1048 100644
--- a/arch/powerpc/platforms/52xx/efika.c
+++ b/arch/powerpc/platforms/52xx/efika.c
@@ -99,7 +99,7 @@ static void __init efika_pcisetup(void)
if (bus_range == NULL || len < 2 * sizeof(int)) {
printk(KERN_WARNING EFIKA_PLATFORM_NAME
": Can't get bus-range for %s\n", pcictrl->full_name);
- return;
+ goto out_put;
}
if (bus_range[1] == bus_range[0])
@@ -111,12 +111,12 @@ static void __init efika_pcisetup(void)
printk(" controlled by %s\n", pcictrl->full_name);
printk("\n");
- hose = pcibios_alloc_controller(of_node_get(pcictrl));
+ hose = pcibios_alloc_controller(pcictrl);
if (!hose) {
printk(KERN_WARNING EFIKA_PLATFORM_NAME
": Can't allocate PCI controller structure for %s\n",
pcictrl->full_name);
- return;
+ goto out_put;
}
hose->first_busno = bus_range[0];
@@ -124,6 +124,9 @@ static void __init efika_pcisetup(void)
hose->ops = &rtas_pci_ops;
pci_process_bridge_OF_ranges(hose, pcictrl, 0);
+ return;
+out_put:
+ of_node_put(pcictrl);
}
#else
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] arch/powerpc/kernel/irq.c: Add of_node_put to avoid memory leak
In this case, a device_node structure is stored in another structure that
is then freed without first decrementing the reference count of the
device_node structure.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
expression x;
identifier f;
position p1,p2;
@@
x...@p1->f =
\(of_find_node_by_path\|of_find_node_by_name\|of_find_node_by_phandle\|of_get_parent\|of_get_next_parent\|of_get_next_child\|of_find_compatible_node\|of_match_node\|of_find_node_by_type\|of_find_node_with_property\|of_find_matching_node\|of_parse_phandle\|of_node_get\)(...);
... when != of_node_put(x)
kf...@p2(x)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
cocci.print_main("call",p1)
cocci.print_secs("free",p2)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/kernel/irq.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 4a65386..0a5bbe5 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -587,8 +587,10 @@ struct irq_host *irq_alloc_host(struct device_node
*of_node,
* this will be fixed once slab is made available early
* instead of the current cruft
*/
- if (mem_init_done)
+ if (mem_init_done) {
+ of_node_put(host->of_node);
kfree(host);
+ }
return NULL;
}
irq_map[0].host = host;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] drivers/net/fs_enet/fs_enet-main.c: Add of_node_put to avoid memory leak
In this case, a device_node structure is stored in another structure that
is then freed without first decrementing the reference count of the
device_node structure.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
expression x;
identifier f;
position p1,p2;
@@
x...@p1->f =
\(of_find_node_by_path\|of_find_node_by_name\|of_find_node_by_phandle\|of_get_parent\|of_get_next_parent\|of_get_next_child\|of_find_compatible_node\|of_match_node\|of_find_node_by_type\|of_find_node_with_property\|of_find_matching_node\|of_parse_phandle\|of_node_get\)(...);
... when != of_node_put(x)
kf...@p2(x)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
cocci.print_main("call",p1)
cocci.print_secs("free",p2)
//
Signed-off-by: Julia Lawall
---
drivers/net/fs_enet/fs_enet-main.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/fs_enet/fs_enet-main.c
b/drivers/net/fs_enet/fs_enet-main.c
index d6e3111..d684f18 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -1036,7 +1036,7 @@ static int __devinit fs_enet_probe(struct platform_device
*ofdev,
ndev = alloc_etherdev(privsize);
if (!ndev) {
ret = -ENOMEM;
- goto out_free_fpi;
+ goto out_put;
}
SET_NETDEV_DEV(ndev, &ofdev->dev);
@@ -1099,6 +1099,7 @@ out_cleanup_data:
out_free_dev:
free_netdev(ndev);
dev_set_drvdata(&ofdev->dev, NULL);
+out_put:
of_node_put(fpi->phy_node);
out_free_fpi:
kfree(fpi);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
