Some files use both a non-devm allocation and a devm_allocation. Don't
complain about a free when the same function contains a non-devm
allocation.
Signed-off-by: Julia Lawall <[email protected]>
---
scripts/coccinelle/free/devm_free.cocci | 55 +++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)
diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index c990d2c..b2a2cf8b 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -56,9 +56,62 @@ expression x;
x = devm_ioport_map(...)
)
+@safe depends on context || org || report exists@
+expression x;
+position p;
+@@
+
+(
+ x = kmalloc(...)
+|
+ x = kvasprintf(...)
+|
+ x = kasprintf(...)
+|
+ x = kzalloc(...)
+|
+ x = kmalloc_array(...)
+|
+ x = kcalloc(...)
+|
+ x = kstrdup(...)
+|
+ x = kmemdup(...)
+|
+ x = get_free_pages(...)
+|
+ x = request_irq(...)
+|
+ x = ioremap(...)
+|
+ x = ioremap_nocache(...)
+|
+ x = ioport_map(...)
+)
+...
+(
+ kfree@p(x)
+|
+ kzfree@p(x)
+|
+ __krealloc@p(x, ...)
+|
+ krealloc@p(x, ...)
+|
+ free_pages@p(x, ...)
+|
+ free_page@p(x)
+|
+ free_irq@p(x)
+|
+ iounmap@p(x)
+|
+ ioport_unmap@p(x)
+)
+
@pb@
expression r.x;
-position p;
+position p != safe.p;
@@
(
On Thu, Feb 01, 2018 at 10:48:52AM +0100, Julia Lawall wrote:
> Some files use both a non-devm allocation and a devm_allocation. Don't
> complain about a free when the same function contains a non-devm
> allocation.
>
> Signed-off-by: Julia Lawall <[email protected]>
>
That's surprising... Do you have an example false positive?
regards,
dan carpenter
Here are the results that are eliminated by my change:
drivers/clk/axs10x/pll_clock.c:323:1-6 kfree(pll_clk);
drivers/clk/clk-gpio.c:131:2-7 kfree(clk_gpio);
drivers/clk/clk-hsdk-pll.c:410:1-6 kfree(pll_clk);
drivers/clk/hisilicon/clk.c:97:1-6 kfree(clk_data);
drivers/mfd/syscon.c:130:1-8 iounmap(base);
drivers/mfd/syscon.c:132:1-6 kfree(syscon);
drivers/pinctrl/freescale/pinctrl-mxs.c:139:2-7 kfree(group);
drivers/pinctrl/samsung/pinctrl-exynos5440.c:264:1-6 kfree(gname);
drivers/platform/chrome/cros_ec_debugfs.c:248:1-6 kfree(msg);
drivers/pwm/pwm-lp3943.c:56:3-8 kfree(pwm_map);
The semantic patch is pretty naive in that it assumes that all uses of the
same name point to the same thing.
Looking through the code, it looks like sometimes both an __init and a
probe function are provided, and the __init function doesn't have access to
a device object.
julia
On Thu, Feb 01, 2018 at 12:06:35PM +0100, Julia Lawall wrote:
> Here are the results that are eliminated by my change:
>
> drivers/clk/axs10x/pll_clock.c:323:1-6 kfree(pll_clk);
> drivers/clk/clk-gpio.c:131:2-7 kfree(clk_gpio);
> drivers/clk/clk-hsdk-pll.c:410:1-6 kfree(pll_clk);
> drivers/clk/hisilicon/clk.c:97:1-6 kfree(clk_data);
> drivers/mfd/syscon.c:130:1-8 iounmap(base);
> drivers/mfd/syscon.c:132:1-6 kfree(syscon);
> drivers/pinctrl/freescale/pinctrl-mxs.c:139:2-7 kfree(group);
> drivers/pinctrl/samsung/pinctrl-exynos5440.c:264:1-6 kfree(gname);
> drivers/platform/chrome/cros_ec_debugfs.c:248:1-6 kfree(msg);
> drivers/pwm/pwm-lp3943.c:56:3-8 kfree(pwm_map);
>
> The semantic patch is pretty naive in that it assumes that all uses of the
> same name point to the same thing.
>
Huh... It leads to false positives but it is an easier way to do cross
function analysis. I had wondered about that.
regards,
dan carpenter
> +@safe depends on context || org || report exists@
> +expression x;
> +position p;
> +@@
> +
> +(
> + x = kmalloc(...)
> +|
> + x = kvasprintf(...)
…
* How do you think about to reduce also a bit of duplicate SmPL code?
* Can it help to work with nested SmPL disjunctions here?
Regards,
Markus
2018-02-01 18:48 GMT+09:00 Julia Lawall <[email protected]>:
> Some files use both a non-devm allocation and a devm_allocation. Don't
> complain about a free when the same function contains a non-devm
> allocation.
>
> Signed-off-by: Julia Lawall <[email protected]>
>
> ---
> scripts/coccinelle/free/devm_free.cocci | 55 +++++++++++++++++++++++++++++++-
> 1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
> index c990d2c..b2a2cf8b 100644
> --- a/scripts/coccinelle/free/devm_free.cocci
> +++ b/scripts/coccinelle/free/devm_free.cocci
> @@ -56,9 +56,62 @@ expression x;
> x = devm_ioport_map(...)
> )
>
> +@safe depends on context || org || report exists@
> +expression x;
> +position p;
> +@@
> +
> +(
> + x = kmalloc(...)
> +|
> + x = kvasprintf(...)
> +|
> + x = kasprintf(...)
> +|
> + x = kzalloc(...)
> +|
> + x = kmalloc_array(...)
> +|
> + x = kcalloc(...)
> +|
> + x = kstrdup(...)
> +|
> + x = kmemdup(...)
> +|
> + x = get_free_pages(...)
> +|
> + x = request_irq(...)
> +|
> + x = ioremap(...)
> +|
> + x = ioremap_nocache(...)
> +|
> + x = ioport_map(...)
> +)
> +...
> +(
> + kfree@p(x)
> +|
> + kzfree@p(x)
> +|
> + __krealloc@p(x, ...)
> +|
> + krealloc@p(x, ...)
> +|
> + free_pages@p(x, ...)
> +|
> + free_page@p(x)
> +|
> + free_irq@p(x)
> +|
> + iounmap@p(x)
> +|
> + ioport_unmap@p(x)
> +)
> +
> @pb@
> expression r.x;
> -position p;
> +position p != safe.p;
> @@
>
> (
>
Anyway, it looks like
this patch makes the situation better.
Applied to linux-kbuild/kbuild. Thanks!
--
Best Regards
Masahiro Yamada