Re: [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL?
> 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(>dev, "trk");
if (IS_ERR(priv->usb2_trk_clk)) {
@@ -483,7 +481,8 @@ tegra186_usb2_pad_probe(struct tegra_xus
unregister:
device_unregister(>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
Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL?
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 >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL?
> 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
Re: [Cocci] xusb-tegra186: Adding a function call behind a label with SmPL?
> 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
Re: [Cocci] [PATCH v5] coccicheck: Support search for SmPL scripts within selected directory hierarchy
I got the impression that you are struggling with difficulties (for unknown reasons) around adding space characters at some places. How would you like to improve this situation? > *Allow defining the environment variable “COCCI” as a directory to search > SmPL scripts. > > *Start a corresponding file determination if it contains an acceptable > path. * Allow defining the environment variable “COCCI” as a directory to search SmPL scripts. * Start a corresponding file determination if it contains an acceptable path. Would you like to update the provided software documentation together with the small extension of this bash script? Update candidates: * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?id=23fdb198ae81f47a574296dab5167c5e136a02ba#n189 * https://bottest.wiki.kernel.org/coccicheck#controlling_which_files_are_processed_by_coccinelle > --- ‣ Would you find the patch change log sufficient without the information “Changes in”? ‣ I find the specification “1:” unnecessary before a single description item. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] drivers/phy/tegra: Completion for exception handling in probe functions with SmPL?
> 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
Re: [Cocci] [v4] coccicheck: Support search for SmPL scripts within selected directory hierarchy
>> Would you like to update the provided software documentation together with >> the small extension of this bash script? > > I'd like to but i don't have rights to update. I suggest to take another look at change possibilities for affected documents. >> Update candidates: * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?id=23fdb198ae81f47a574296dab5167c5e136a02ba#n189 This file can be directly edited in a corresponding development branch for your contribution, can't it? >> * >> https://bottest.wiki.kernel.org/coccicheck#controlling_which_files_are_processed_by_coccinelle Are going to try any of the supported login options out for this wiki? Would you like to choose any other data synchronisation method? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
