[Cocci] Improving size determinations with SmPL

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

Hello,

I have constructed the following small script for another application of the
semantic patch language.


@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 ... when any
 X* var;
 ... when any
 var = action(...,
              sizeof(
-                    X
+                    *var
                    ),
              ...
             )
 ... when any
 }


I know that this transformation approach is working to some degree.
I came along another source file where I do not get the desired code adjustment
with the software version ?1.0.6-00186-g0acd38ee? automatically.

elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination1.cocci fs/pstore/ram.c


How should the software situation be changed further so that the function
?ramoops_pstore_read? will be also handled in the way like I published it in the update suggestion ?pstore: Improve a size determination in three functions??
http://elixir.free-electrons.com/linux/v4.12.8/source/fs/pstore/ram.c#L291
https://patchwork.kernel.org/patch/9904523/
https://lkml.kernel.org/r/<23798ef4-5559-3454-ecf4-e6844b644ff3@users.sourceforge.net>

Regards,
Markus

[Cocci] Improving size determinations with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> Hello,
>
> I have constructed the following small script for another application of the
> semantic patch language.
>
>
> @replacement@
> identifier action, var, work;
> type T, X;
> @@
>  T work(...)
>  {
>  ... when any
>  X* var;
>  ... when any

... when any should be <...

>  var = action(...,
>               sizeof(
> -                    X
> +                    *var
>                     ),
>               ...
>              )
>  ... when any

... when any should be ...>

You request that the sizeof appear on every execution path.  <... ...>
will cause the transformation to be performed wherever appropriate, but
allow it not to be relevant on some execution paths.

julia

>  }
>
>
> I know that this transformation approach is working to some degree.
> I came along another source file where I do not get the desired code adjustment
> with the software version ?1.0.6-00186-g0acd38ee? automatically.
>
> elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination1.cocci fs/pstore/ram.c
>
>
> How should the software situation be changed further so that the function
> ?ramoops_pstore_read? will be also handled in the way like I published it in the update suggestion ?pstore: Improve a size determination in three functions??
> http://elixir.free-electrons.com/linux/v4.12.8/source/fs/pstore/ram.c#L291
> https://patchwork.kernel.org/patch/9904523/
> https://lkml.kernel.org/r/<23798ef4-5559-3454-ecf4-e6844b644ff3@users.sourceforge.net>
>
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

> ... when any should be ...>

I am unsure if I interpret your suggestion correctly.


@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 <... ... when any ...>
 X* var;
 <... ... when any ...>
 var = action(...,
              sizeof(
-                    X
+                    *var
                    ),
              ...
             )
 <... ... when any ...>
 }


elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination2.cocci fs/pstore/ram.c
?
minus: parse error: 
  File "/home/elfring/Projekte/Coccinelle/janitor/safer_size_determination2.cocci", line 7, column 6, charpos = 80
  around = '...',
  whole content =  <... ... when any ...>



> You request that the sizeof appear on every execution path.

I hope not. - I would like to express that this operator can appear for
a parameter of a function call.

Does the variable declaration ?struct persistent_ram_zone *tmp_prz, *prz_next;?
in the function ?ramoops_pstore_read? trigger challenges for the attempted
source code transformation?

Regards,
Markus

[Cocci] Improving size determinations with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> > ... when any should be ...>
>
> I am unsure if I interpret your suggestion correctly.

No.  Go read the original message again. It says exactly what to do and
where it should be done.  Eg, my message has two comments, but you have
made three changes, which is not correct.

julia

>
>
> @replacement@
> identifier action, var, work;
> type T, X;
> @@
>  T work(...)
>  {
>  <... ... when any ...>
>  X* var;
>  <... ... when any ...>
>  var = action(...,
>               sizeof(
> -                    X
> +                    *var
>                     ),
>               ...
>              )
>  <... ... when any ...>
>  }
>
>
> elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/safer_size_determination2.cocci fs/pstore/ram.c
> ?
> minus: parse error:
>   File "/home/elfring/Projekte/Coccinelle/janitor/safer_size_determination2.cocci", line 7, column 6, charpos = 80
>   around = '...',
>   whole content =  <... ... when any ...>
>
>
>
> > You request that the sizeof appear on every execution path.
>
> I hope not. - I would like to express that this operator can appear for
> a parameter of a function call.
>
> Does the variable declaration ?struct persistent_ram_zone *tmp_prz, *prz_next;?
> in the function ?ramoops_pstore_read? trigger challenges for the attempted
> source code transformation?
>
> Regards,
> Markus
>

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

>>> ... when any should be ...>
>>
>> I am unsure if I interpret your suggestion correctly.
> 
> No.  Go read the original message again. It says exactly what to do and
> where it should be done.

Your hint might be nice.


> Eg, my message has two comments, but you have made three changes, which is not correct.

I have tried out to convert the SmPL ellipsis into the optional dot variant.

But I wonder still about the error message ?minus: parse error:?
for an adjusted SmPL script variant.

@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 ... when any
 X* var;
 <... ... when any ...>
 var = action(...,
              sizeof(
-                    X
+                    *var
                    ),
              ...
             )
 <... ... when any ...>
 }


Regards,
Markus

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

>> ... when any should be <...
> 
> I have told you exactly what to do here.

You are right in the sense that I misinterpreted your change suggestion.


> The line above currently contains ... when any.  It (and only it) should be
> completely removed and changed to <...

But I find it inappropriate to enclose the function call code by an optional
SmPL nest construct.


> Nowhere do I suggest to put <... ... when any ...>

The suggested script variant seems to work finally.
Thanks for your information.

@replacement@
identifier action, var, work;
type T, X;
@@
 T work(...)
 {
 ... when any
 X* var;
 <...
 var = action(...,
              sizeof(
-                    X
+                    *var
                    ),
              ...
             )
 ...>
 }


Now I wonder why the SmPL construct ?<+... ...+>? does not produce the same
transformation result for the source file ?fs/pstore/ram.c?.

Regards,
Markus

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

> The suggested script variant seems to work finally.

I have tried it out once more with the following command.

elfring at Sonne:~/Projekte/Linux/next-patched> spatch.opt --timeout 67 -j 4 --in-place ~/Projekte/Coccinelle/janitor/safer_size_determination2.cocci fs/jfs
?
EXN: Common.TimeoutNote: processing took    74.0s: fs/jfs/jfs_txnmgr.c
?
EXN: Common.TimeoutNote: processing took    73.5s: fs/jfs/jfs_xtree.c
?
EXN: Common.TimeoutNote: processing took    75.2s: fs/jfs/jfs_dtree.c
?

How do you think about to take another look at the shown execution times?
Are the run time characteristics worth for another clarification?

By the way:
I would find another line break nicer in these messages.

Regards,
Markus

[Cocci] Improving size determinations with SmPL

[Julia Lawall]

On Fri, 18 Aug 2017, SF Markus Elfring wrote:

> > The suggested script variant seems to work finally.
>
> I have tried it out once more with the following command.
>
> elfring at Sonne:~/Projekte/Linux/next-patched> spatch.opt --timeout 67 -j 4 --in-place ~/Projekte/Coccinelle/janitor/safer_size_determination2.cocci fs/jfs
> ?
> EXN: Common.TimeoutNote: processing took    74.0s: fs/jfs/jfs_txnmgr.c
> ?
> EXN: Common.TimeoutNote: processing took    73.5s: fs/jfs/jfs_xtree.c
> ?
> EXN: Common.TimeoutNote: processing took    75.2s: fs/jfs/jfs_dtree.c
> ?
>
> How do you think about to take another look at the shown execution times?

I can only take a look at things when the complete code or a pointer to
the complete code is included in the mail message.  Here I have the c
code, but not the semantic patch.  In particular, I don't know whether you
are using the version with <... ...> or <+... ...+>.  The latter is more
time consuming because it has to do extra chcks.  Both, however, need to
follow the control-flow of the function, so they can be expensive if there
are many loops or conditionals.

julia

> Are the run time characteristics worth for another clarification?
>
> By the way:
> I would find another line break nicer in these messages.
>
> Regards,
> Markus
>

[Cocci] Improving size determinations with SmPL

[SF Markus Elfring]

> Here I have the c code,

You have also access to the referenced source files for this Linux software module.


> but not the semantic patch.

I find this kind of feedback surprising.

We discussed it a bit yesterday so that this SmPL script was stored also in
the mailing list archive.
https://systeme.lip6.fr/pipermail/cocci/2017-August/004350.html


> In particular, I don't know whether you are using the version with <... ...>

I am using the optional nest construct for a while also because


> or <+... ...+>.

this SmPL construct does not work for my use case. (I stored a corresponding
test script under an other file name.)
How will the discussion be continued for the bug report ?Get the non-optional
nest construct completely working for SmPL??
https://github.com/coccinelle/coccinelle/issues/114


> The latter is more time consuming because it has to do extra chcks.

I am curious if the software execution can fit better into more pleasing
time ranges.


> Both, however, need to follow the control-flow of the function,
> so they can be expensive if there are many loops or conditionals.

I assume that there are further improvement possibilities available
for the desired run time characteristics.

Regards,
Markus