Re: Coccinelle: semantic patch for missing of_node_put

Re: Coccinelle: semantic patch for missing of_node_put

[Julia Lawall]

A semantic patch has no access to comments.  The only thing I can see to
do is to use python to interact with some external tools.  For example,
you could write some code to collect the comments in a file and the lines
on which they occur, and then get the comment that most closely precedes
the start of the function.

julia

Re: Coccinelle: semantic patch for missing of_node_put

[Markus Elfring]

> A semantic patch has no access to comments.

Thanks for your acknowledgement of the current situation.


> The only thing I can see to do is to use python to interact with some external tools.

I see more software development possibilities.

* Advanced data processing for source code comments
  https://github.com/coccinelle/coccinelle/issues/57

* Add a metavariable for the handling of source code
  https://github.com/coccinelle/coccinelle/issues/140


Regards,
Markus

Re: Coccinelle: semantic patch for missing of_node_put

[Pavel Machek]

Hi!

> A semantic patch has no access to comments.  The only thing I can see to
> do is to use python to interact with some external tools.  For example,
> you could write some code to collect the comments in a file and the lines
> on which they occur, and then get the comment that most closely precedes
> the start of the function.

How dangerous is missing of_node_put? AFAICT it will only result into
very small, one-time memory leak, right?

Could we make sure these patches are _not_ going to stable? Leaking
few bytes once per boot is not really a serious bug.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Coccinelle: semantic patch for missing of_node_put

[Julia Lawall]

On Mon, 20 May 2019, Pavel Machek wrote:

> Hi!
>
> > A semantic patch has no access to comments.  The only thing I can see to
> > do is to use python to interact with some external tools.  For example,
> > you could write some code to collect the comments in a file and the lines
> > on which they occur, and then get the comment that most closely precedes
> > the start of the function.
>
> How dangerous is missing of_node_put? AFAICT it will only result into
> very small, one-time memory leak, right?
>
> Could we make sure these patches are _not_ going to stable? Leaking
> few bytes once per boot is not really a serious bug.

Agreed.  Just tell Sasha.

julia

> 									Pavel
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>

Re: Coccinelle: semantic patch for missing of_node_put

[Julia Lawall]

On Mon, 20 May 2019, Pavel Machek wrote:

> Hi!
>
> > A semantic patch has no access to comments.  The only thing I can see to
> > do is to use python to interact with some external tools.  For example,
> > you could write some code to collect the comments in a file and the lines
> > on which they occur, and then get the comment that most closely precedes
> > the start of the function.
>
> How dangerous is missing of_node_put? AFAICT it will only result into
> very small, one-time memory leak, right?
>
> Could we make sure these patches are _not_ going to stable? Leaking
> few bytes once per boot is not really a serious bug.

Sasha,

Probably patches that add only of_node_put should not be auto selected for
stable.

julia

Re: Coccinelle: semantic patch for missing of_node_put

[Sasha Levin]

On Mon, May 20, 2019 at 11:52:37AM +0200, Julia Lawall wrote:
>
>
>On Mon, 20 May 2019, Pavel Machek wrote:
>
>> Hi!
>>
>> > A semantic patch has no access to comments.  The only thing I can see to
>> > do is to use python to interact with some external tools.  For example,
>> > you could write some code to collect the comments in a file and the lines
>> > on which they occur, and then get the comment that most closely precedes
>> > the start of the function.
>>
>> How dangerous is missing of_node_put? AFAICT it will only result into
>> very small, one-time memory leak, right?
>>
>> Could we make sure these patches are _not_ going to stable? Leaking
>> few bytes once per boot is not really a serious bug.
>
>Sasha,
>
>Probably patches that add only of_node_put should not be auto selected for
>stable.

I can filter them out, but those are fixes, right? Why are we concerned
about them making it into -stable?

--
Thanks,
Sasha

Re: Coccinelle: semantic patch for missing of_node_put

[Julia Lawall]

On Mon, 20 May 2019, Sasha Levin wrote:

> On Mon, May 20, 2019 at 11:52:37AM +0200, Julia Lawall wrote:
> >
> >
> > On Mon, 20 May 2019, Pavel Machek wrote:
> >
> > > Hi!
> > >
> > > > A semantic patch has no access to comments.  The only thing I can see to
> > > > do is to use python to interact with some external tools.  For example,
> > > > you could write some code to collect the comments in a file and the
> > > lines
> > > > on which they occur, and then get the comment that most closely precedes
> > > > the start of the function.
> > >
> > > How dangerous is missing of_node_put? AFAICT it will only result into
> > > very small, one-time memory leak, right?
> > >
> > > Could we make sure these patches are _not_ going to stable? Leaking
> > > few bytes once per boot is not really a serious bug.
> >
> > Sasha,
> >
> > Probably patches that add only of_node_put should not be auto selected for
> > stable.
>
> I can filter them out, but those are fixes, right? Why are we concerned
> about them making it into -stable?

One of them may have introduced a crash.  If there is a bad reference
count manipulation elsewhere, then fixing one could cause a later
incorrect one to make a double free.

On the other hand, I don't know if the one that seemed to cause a crash
really caused a crash.  It was detected by syzkaller, and it is also
possible that git bisect ended up at the wrong place.

In any case, forgetting an of_node_put will normally just waste a little
memory, so probably stable kernels don't care.

julia

Re: Coccinelle: semantic patch for missing of_node_put

[Markus Elfring]

> On the other hand, I don't know if the one that seemed to cause a crash
> really caused a crash.  It was detected by syzkaller, and it is also
> possible that git bisect ended up at the wrong place.

Do you refer to any known bug report here?


> In any case, forgetting an of_node_put will normally just waste a little
> memory, so probably stable kernels don't care.

Will it be nice to achieve better exception handling also for these
software versions over time?

Regards,
Markus

Re: Coccinelle: semantic patch for missing of_node_put

[Markus Elfring]

> 1, A simple method.
> We did some experiments, and we could get the list of functions that need to be considered directly through this script:
>
> $ spatch --tokens-c drivers/of/base.c  2>&1  | grep "Tag3 " | grep "of_node_put() on it when done." | awk -F " - " '{print $1}' | grep  -o "of_[[:print:]]*"

Thanks for your command demonstration.

* How are the chances to get these tags better documented?
  https://github.com/coccinelle/coccinelle/blob/66a1118e04a6aaf1acdae89623313c8e05158a8d/docs/manual/spatch_options.tex#L165

* Would you like to combine the texts from the first two greps
  in a single search pattern?

* I imagine that sort criteria can become relevant for
  the determined function name list.

* Will a software build script be needed for this purpose?


> 2, A general method.
> We also try to get the list of functions to consider by writing a SmPL,
> but this method is not feasible at present, because it is not easy to parse the comment
> header information of these functions.

I am curious if corresponding software development challenges
will be picked up more.


> @r1@
> identifier fn;
> comment x;

This item is not mentioned as a key word in the manual for
the semantic patch language so far while the word is used
at seven places in this document.


> @@
>
> struct device_node * fn (...)
> {
> ...
> }

You can not get the desired information if a metavariable like “x”
is not actually used in the SmPL search code.

How do you think about to take corresponding source code positions
better into account?


> 3, It's probably interesting to get valuable informations from the comments of a function.

Other development tools provide better support for this data processing area.


> We will continue to learn the source code of coccinelle and try to find a way to achieve it.

How will the situation evolve here?


> Please kindly give me some help.

Do you find the following clarification request interesting?

Fix two calls for the program “ocamldoc”
https://github.com/coccinelle/coccinelle/issues/111


> We will continue to optimize this SmPL and send a V2 version next week.

I got another development concern in the meantime.
It seems that you would like to use iteration functionality (add_if_not_present).
https://github.com/coccinelle/coccinelle/blob/99e081e9b89d49301b7bd2c5e5aac88c66eaaa6a/docs/manual/cocci_syntax.tex#L1826

How will it matter here?

Regards,
Markus

Re: Coccinelle: semantic patch for missing of_node_put

[Markus Elfring]

> $ spatch --tokens-c drivers/of/base.c  2>&1  | grep "Tag3 " | grep "of_node_put() on it when done." | awk -F " - " '{print $1}' | grep  -o "of_[[:print:]]*"

This command example points some details out for further software development considerations.

1. I find it questionable that relevant data are provided by the output channel
   “stderr” so far.
   https://github.com/coccinelle/coccinelle/blob/66a1118e04a6aaf1acdae89623313c8e05158a8d/docs/manual/spatch_options.tex#L165

2. The OCaml code “"Tag" ^ string_of_int t ^” occurs in three source files.
   * It is commented out in one file.
     https://github.com/coccinelle/coccinelle/blob/761cf6a1fbbf3173896ff61f0ea7e4a83a5b2a57/commons/common.ml#L305

   * These places refer to the source file “dumper.ml 1.2” by Richard W. M. Jones.
     Thus it seems that this code is relevant at the moment.
     https://github.com/coccinelle/coccinelle/blob/175de16bc7e535b6a89a62b81a673b0d0cd7075c/commons/ocamlextra/dumper.ml#L1

3. How will the software documentation evolve here?

4. Safe data processing can be performed only if the involved structures
   will remain clear for a while.
   Is the situation partly unclear?

   Should the information after which function calls the function “of_node_put”
   should be called be determined from any other documentation format?

5. A programming language like “awk” has got the potential to extract useful data
   (also without calling the tool “grep” additionally).

Regards,
Markus

Re: Coccinelle: semantic patch for missing of_node_put

[Markus Elfring]

> 2, A general method.
> We also try to get the list of functions to consider by writing a SmPL,
> but this method is not feasible at present, because it is not easy to parse the comment
> header information of these functions.

The situation was improved once more also for the Coccinelle software.
How do you think about to develop any more variants based on information
from a script (like the following) for the semantic patch language?

@initialize:python@
@@
import re, sys
filter = re.compile(" when done")

@find@
comments c;
identifier x;
type t;
@@
 t@c x(...)
 { ... }

@script:python selection@
input << find.c;
@@
if filter.search(input[0].before, 2):
   sys.stderr.write(input[0].before + "\n=====\n")
else:
   cocci.include_match(False)

@display@
identifier find.x;
type find.t;
@@
*t x(...)
 { ... }


Does such a source code analysis approach indicate any details
which should be improved for the affected software documentation?

Regards,
Markus

Re: Coccinelle: Searching for “when done” in function comments

[Markus Elfring]

> Does such a source code analysis approach indicate any details
> which should be improved for the affected software documentation?

I have constructed another SmPL script variant.

It can point out that the text “ when done” occurs in comment lines before
48 function implementations which are provided by 20 source files of
the software “Linux next-20190605”.
Will this source code analysis result trigger further development efforts
around detection of unique wordings in these descriptions?

Regards,
Markus