* Re: [Cocci] phy: tegra: xusb: Complete exception handling in probe functions [not found] <fe6868a4-01f5-2832-9081-7643be0ab4a1@web.de> @ 2019-10-29 9:40 ` Markus Elfring 2019-10-30 10:44 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring 0 siblings, 1 reply; 8+ 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: kernel-janitors, Kangjie Lu, LKML, 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 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? 2019-10-29 9:40 ` [Cocci] phy: tegra: xusb: Complete exception handling in probe functions Markus Elfring @ 2019-10-30 10:44 ` Markus Elfring 2019-10-30 17:30 ` [Cocci] xusb-tegra186: Renaming known jump labels " Markus Elfring ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Markus Elfring @ 2019-10-30 10:44 UTC (permalink / raw) To: linux-tegra, Coccinelle Cc: Jonathan Hunter, YueHaibing, JC Kuo, Kangjie Lu, kernel-janitors, LKML, Kishon Vijay Abraham I, Navid Emamdoost, Stephen McCamant, Thierry Reding, Dan Carpenter > 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 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cocci] xusb-tegra186: Renaming known jump labels with SmPL? 2019-10-30 10:44 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring @ 2019-10-30 17:30 ` Markus Elfring 2019-10-30 18:15 ` [Cocci] xusb-tegra186: Adding a function call behind a label " Markus Elfring 2019-10-30 20:50 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions " Markus Elfring 2 siblings, 0 replies; 8+ messages in thread From: Markus Elfring @ 2019-10-30 17:30 UTC (permalink / raw) To: Coccinelle > But I stumble on another unexpected test result. I have tried a related tiny transformation approach out. @renaming@ @@ -out +exit : return ERR_PTR(...); I got a diff hunk like the following then. elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/phy/tegra/xusb-tegra186.c ~/Projekte/Coccinelle/janitor/rename_out_label1.cocci … warning: line 3: should out be a metavariable? … @@ -483,7 +483,7 @@ tegra186_usb2_pad_probe(struct tegra_xus unregister: device_unregister(&pad->dev); -out: +exit : return ERR_PTR(err); } … Should the addition of an extra space character usually be avoided for such a jump label? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL? 2019-10-30 10:44 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring 2019-10-30 17:30 ` [Cocci] xusb-tegra186: Renaming known jump labels " Markus Elfring @ 2019-10-30 18:15 ` Markus Elfring [not found] ` <2db8691d768571f6c275f3d89401df23@lip6.fr> 2019-10-30 20:50 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions " Markus Elfring 2 siblings, 1 reply; 8+ messages in thread From: Markus Elfring @ 2019-10-30 18:15 UTC (permalink / raw) To: Coccinelle > But I stumble on another unexpected test result. I would like point out that the following simple (?) transformation approach does not generate the expected diff hunk at the moment. @addition@ expression object; @@ object = kzalloc(...) ... when any device_unregister(...); out: +kfree(object); return ERR_PTR(...); elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/phy/tegra/xusb-tegra186.c ~/Projekte/Coccinelle/janitor/add_kfree_call1.cocci Will further collateral evolution happen then also for the Coccinelle software? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <2db8691d768571f6c275f3d89401df23@lip6.fr>]
* Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL? [not found] ` <2db8691d768571f6c275f3d89401df23@lip6.fr> @ 2019-10-30 20:00 ` Markus Elfring 2019-10-30 20:44 ` Julia Lawall 2019-10-31 8:12 ` Markus Elfring 1 sibling, 1 reply; 8+ messages in thread From: Markus Elfring @ 2019-10-30 20:00 UTC (permalink / raw) To: Julia Lawall; +Cc: Coccinelle > There is no reason why a patch should be generated in this case. > As you should know well, A ... B only matches in a transformation case > if every path from A leads to code matching B. That is not the case in your example. The exception handling code should usually be executed at the end of function implementations after an error situation was detected. Do you try to refer to specific information from the software documentation like the following? https://github.com/coccinelle/coccinelle/blob/ed1eb8e06f800739d3992158d36945c0c4c6f0c7/docs/manual/cocci_syntax.tex#L179 “… A depends on clause can further indicate whether the clause should be satisfied for all the branches (forall) or only for one (exists). exists is the default. …” The following simple transformation approach seems to work in the way which I expected somehow initially. @addition exists@ expression object; @@ object = kzalloc(...) ... when any device_unregister(...); out: +kfree(object); return ERR_PTR(...); Does this change specification indicate then a disagreement about a default SmPL rule property? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL? 2019-10-30 20:00 ` Markus Elfring @ 2019-10-30 20:44 ` Julia Lawall 0 siblings, 0 replies; 8+ messages in thread From: Julia Lawall @ 2019-10-30 20:44 UTC (permalink / raw) To: Markus Elfring; +Cc: Coccinelle [-- Attachment #1: Type: text/plain, Size: 1311 bytes --] On Wed, 30 Oct 2019, Markus Elfring wrote: > > There is no reason why a patch should be generated in this case. > > As you should know well, A ... B only matches in a transformation case > > if every path from A leads to code matching B. That is not the case in your example. > > The exception handling code should usually be executed at the end of > function implementations after an error situation was detected. > > Do you try to refer to specific information from the software documentation > like the following? > > https://github.com/coccinelle/coccinelle/blob/ed1eb8e06f800739d3992158d36945c0c4c6f0c7/docs/manual/cocci_syntax.tex#L179 > “… > A depends on clause can further indicate whether the clause should be satisfied > for all the branches (forall) or only for one (exists). exists is the default. > …” There is no depends on in your code. The cited text is not relevant. julia > > The following simple transformation approach seems to work in the way > which I expected somehow initially. > > @addition exists@ > expression object; > @@ > object = kzalloc(...) > ... when any > device_unregister(...); > out: > +kfree(object); > return ERR_PTR(...); > > > Does this change specification indicate then a disagreement about > a default SmPL rule property? > > Regards, > Markus > [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL? [not found] ` <2db8691d768571f6c275f3d89401df23@lip6.fr> 2019-10-30 20:00 ` Markus Elfring @ 2019-10-31 8:12 ` Markus Elfring 1 sibling, 0 replies; 8+ messages in thread From: Markus Elfring @ 2019-10-31 8:12 UTC (permalink / raw) To: Julia Lawall; +Cc: Coccinelle >> @addition@ >> expression object; >> @@ >> object = kzalloc(...) >> ... when any >> device_unregister(...); >> out: >> +kfree(object); >> return ERR_PTR(...); > > There is no reason why a patch should be generated in this case. We come along different software development views also for this use case. I tend still to interpret some of my source code search patterns in other directions. > As you should know well, A ... B only matches in a transformation case > if every path from A leads to code matching B. Now I imagine that an explanation of such information can become more interesting also based on the applied technologies as a knowledge background. https://github.com/coccinelle/coccinelle/blob/ed1eb8e06f800739d3992158d36945c0c4c6f0c7/docs/manual/cocci_syntax.tex#L860 https://en.wikipedia.org/wiki/Computation_tree_logic#Relations_with_other_logics > That is not the case in your example. The shown SmPL script can probably indicate that a return statement should occasionally be executed if a jump to it (according to the label “out” for example) was performed before (in one if branch). The following simple transformation approach seems to work also in the way which I expected somehow initially. @addition@ expression object; @@ object = kzalloc(...) ... when exists device_unregister(...); out: +kfree(object); return ERR_PTR(...); Can any extensions for the software documentation help to make the application of the semantic patch language easier and safer finally? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? 2019-10-30 10:44 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring 2019-10-30 17:30 ` [Cocci] xusb-tegra186: Renaming known jump labels " Markus Elfring 2019-10-30 18:15 ` [Cocci] xusb-tegra186: Adding a function call behind a label " Markus Elfring @ 2019-10-30 20:50 ` Markus Elfring 2 siblings, 0 replies; 8+ messages in thread From: Markus Elfring @ 2019-10-30 20:50 UTC (permalink / raw) To: linux-tegra, Coccinelle Cc: Jonathan Hunter, YueHaibing, JC Kuo, Kangjie Lu, kernel-janitors, LKML, Kishon Vijay Abraham I, Navid Emamdoost, Stephen McCamant, Thierry Reding, Dan Carpenter > 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 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-31 8:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <fe6868a4-01f5-2832-9081-7643be0ab4a1@web.de> 2019-10-29 9:40 ` [Cocci] phy: tegra: xusb: Complete exception handling in probe functions Markus Elfring 2019-10-30 10:44 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL? Markus Elfring 2019-10-30 17:30 ` [Cocci] xusb-tegra186: Renaming known jump labels " Markus Elfring 2019-10-30 18:15 ` [Cocci] xusb-tegra186: Adding a function call behind a label " Markus Elfring [not found] ` <2db8691d768571f6c275f3d89401df23@lip6.fr> 2019-10-30 20:00 ` Markus Elfring 2019-10-30 20:44 ` Julia Lawall 2019-10-31 8:12 ` Markus Elfring 2019-10-30 20:50 ` [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions " 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).