* phy: tegra: xusb: Complete exception handling in five probe functions
@ 2019-10-28 21:20 Markus Elfring
2019-10-29 9:40 ` phy: tegra: xusb: Complete exception handling in " Markus Elfring
0 siblings, 1 reply; 4+ messages in thread
From: Markus Elfring @ 2019-10-28 21:20 UTC (permalink / raw)
To: linux-tegra, JC Kuo, Jonathan Hunter, Kishon Vijay Abraham I,
Thierry Reding
Cc: LKML, kernel-janitors, Kangjie Lu, Navid Emamdoost, Stephen McCamant
Hello,
I have taken another look also at the implementations of five probe functions.
I have got the impression then that the exception handling is incomplete so far
at these source code places.
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/phy/tegra/xusb-tegra124.c#L608
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/tegra/xusb-tegra124.c?id=8005803a2ca0af49f36f6e9329b5ecda3df27347#n608
How do you think about to move kfree() calls to other jump targets
according to the Linux coding style?
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: phy: tegra: xusb: Complete exception handling in probe functions
2019-10-28 21:20 phy: tegra: xusb: Complete exception handling in five probe functions Markus Elfring
@ 2019-10-29 9:40 ` Markus Elfring
2019-10-30 10:44 ` drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring
0 siblings, 1 reply; 4+ messages in thread
From: Markus Elfring @ 2019-10-29 9:40 UTC (permalink / raw)
To: linux-tegra, JC Kuo, Jonathan Hunter, Kishon Vijay Abraham I,
Thierry Reding
Cc: LKML, kernel-janitors, Kangjie Lu, Navid Emamdoost,
Stephen McCamant, Coccinelle
> I have got the impression then that the exception handling is incomplete so far
> at these source code places.
How do you think about to try the following small script for the semantic patch
language out on the directory “drivers/phy/tegra”?
@display@
expression object;
identifier exit;
@@
... when any
object = kzalloc(...)
...
if (...)
{
*kfree(object);
goto exit;
}
...
if (...)
* goto unregister;
Will any search pattern variations become more interesting for corresponding
automatic software transformations?
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drivers/phy/tegra: Completion for exception handling in probe functions with SmPL?
2019-10-29 9:40 ` phy: tegra: xusb: Complete exception handling in " Markus Elfring
@ 2019-10-30 10:44 ` Markus Elfring
2019-10-30 20:50 ` Markus Elfring
0 siblings, 1 reply; 4+ messages in thread
From: Markus Elfring @ 2019-10-30 10:44 UTC (permalink / raw)
To: linux-tegra, Coccinelle
Cc: JC Kuo, Jonathan Hunter, Kishon Vijay Abraham I, Thierry Reding,
LKML, kernel-janitors, Kangjie Lu, Navid Emamdoost,
Stephen McCamant, Dan Carpenter, YueHaibing
> Will any search pattern variations become more interesting for corresponding
> automatic software transformations?
I hoped to achieve something together with the semantic patch language
by the following transformation approach.
@adjustment@
expression object;
identifier exit;
@@
object = kzalloc(...)
...
if (...)
-{ kfree(object);
goto
- exit
+ release_memory
;
-}
... when any
device_unregister(...);
-exit
+release_memory
:
+kfree(object);
return ERR_PTR(...);
Do you find such a change suggestion reasonable (in principle)?
But I stumble on another unexpected test result.
elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/phy/tegra/xusb-tegra186.c ~/Projekte/Coccinelle/janitor/complete_exception_handling_in_probe_functions1.cocci
init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
HANDLING: drivers/phy/tegra/xusb-tegra186.c
How would you like to clarify why diff hunks were not generated
for functions like “tegra186_usb2_pad_probe” and “tegra186_usb3_pad_probe”
in such an use case?
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/phy/tegra/xusb-tegra186.c#L445
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/tegra/xusb-tegra186.c?id=bbf711682cd570697086e88388a2c718da918894#n445
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drivers/phy/tegra: Completion for exception handling in probe functions with SmPL?
2019-10-30 10:44 ` drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring
@ 2019-10-30 20:50 ` Markus Elfring
0 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2019-10-30 20:50 UTC (permalink / raw)
To: linux-tegra, Coccinelle
Cc: JC Kuo, Jonathan Hunter, Kishon Vijay Abraham I, Thierry Reding,
LKML, kernel-janitors, Kangjie Lu, Navid Emamdoost,
Stephen McCamant, Dan Carpenter, YueHaibing
> But I stumble on another unexpected test result.
I got more promising results by the following transformation approach.
@adjustment exists@
expression object;
identifier exit;
@@
object = kzalloc(...)
...
if (...)
-{ kfree(object);
goto
- exit
+ release_memory
;
-}
... when any
device_unregister(...);
-exit
+release_memory
:
+kfree(object);
return ERR_PTR(...);
The scope property of such a SmPL rule occasionally needs also more
software development attention.
https://github.com/coccinelle/coccinelle/blob/ed1eb8e06f800739d3992158d36945c0c4c6f0c7/docs/manual/cocci_syntax.tex#L860
I observe that the pretty-printing for the generated source code will need
further improvements (according to the Linux coding style).
Example:
elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/phy/tegra/xusb-tegra186.c ~/Projekte/Coccinelle/janitor/complete_exception_handling_in_probe_functions6.cocci
…
@@ -461,10 +461,8 @@ tegra186_usb2_pad_probe(struct tegra_xus
pad->soc = soc;
err = tegra_xusb_pad_init(pad, padctl, np);
- if (err < 0) {
- kfree(usb2);
- goto out;
- }
+ if (err < 0)
+ goto release_memory;
priv->usb2_trk_clk = devm_clk_get(&pad->dev, "trk");
if (IS_ERR(priv->usb2_trk_clk)) {
@@ -483,7 +481,8 @@ tegra186_usb2_pad_probe(struct tegra_xus
unregister:
device_unregister(&pad->dev);
-out:
+release_memory :
+kfree(usb2);
return ERR_PTR(err);
}
…
How reasonable do you find the presented update suggestion?
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-30 20:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28 21:20 phy: tegra: xusb: Complete exception handling in five probe functions Markus Elfring
2019-10-29 9:40 ` phy: tegra: xusb: Complete exception handling in " Markus Elfring
2019-10-30 10:44 ` drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring
2019-10-30 20:50 ` Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).