Re: [Cocci] [PATCH] coccinelle: api: add kvfree script

Re: [Cocci] [PATCH] coccinelle: api: add kvfree script

[Markus Elfring]

> Check that alloc and free types of functions match each other.

Further software development challenges are interesting also for such an use case.


> +/// Check that kvmalloc'ed memory is freed by kfree functions,
> +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
> +/// functions.

* How do you think about to offer a wording suggestion for subjects of
  generated patches?

* Will the presented case distinction trigger further improvements for
  the desired matching?

* Would you like to generalise the safe handling of allocations
  and corresponding release of system resources?


> +// Confidence: High

I suggest to reconsider this information once more.


> +virtual patch
> +virtual report
> +virtual org
> +virtual context

+virtual patch, report, org, context

Is such a SmPL code variant more succinct?


> +@choice@

* Can it be that this SmPL rule is not relevant for all operation modes?

* Will additional dependencies matter?


> +    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)

I would prefer an other coding style here.

* Items for such SmPL disjunctions can be specified also on multiple lines.

* The semantic patch language supports further means to handle function name lists
  in more convenient ways.
  Would you like to work with customised constraints?


> +msg = "WARNING: kmalloc is used to allocate this memory at line %s" % (k[0].line)
> +coccilib.report.print_report(p[0], msg)

* I propose once more omit the extra variable “msg” at similar places.
  The desired message object can be directly passed as a function parameter.

* I find the diagnostic text insufficient.

* Can the corresponding function category be dynamically determined?


Are you looking for opportunities to avoid unwanted code duplication?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] coccinelle: api: add kvfree script

[Julia Lawall]

> > +    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
>
> I would prefer an other coding style here.
>
> * Items for such SmPL disjunctions can be specified also on multiple lines.
>
> * The semantic patch language supports further means to handle function name lists
>   in more convenient ways.
>   Would you like to work with customised constraints?

Please don't follow this advice.  Coccinelle is not able to optimize its
search process according to the information in constraints.  It will
needlessly parse many files.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] coccinelle: api: add kvfree script

[Markus Elfring]

>>> +    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
>>
>> I would prefer an other coding style here.
>>
>> * Items for such SmPL disjunctions can be specified also on multiple lines.
>>
>> * The semantic patch language supports further means to handle function name lists
>>   in more convenient ways.
>>   Would you like to work with customised constraints?
>
> Please don't follow this advice.

I suggest to reconsider such feedback.

Do the previous comments contain helpful information?


> Coccinelle is not able to optimize its search process

The software contains some limitations which might be changeable.


> according to the information in constraints.

Will related solution ideas become more interesting?


> It will needlessly parse many files.

Such an effect would be undesirable.

* Would you like to clarify other software development options any more?

* How will design goals evolve?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] coccinelle: api: add kvfree script

[Markus Elfring]

>>> +    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
>>
>> I would prefer an other coding style here.
>>
>> * Items for such SmPL disjunctions can be specified also on multiple lines.
>>
>> * The semantic patch language supports further means to handle function name lists
>>   in more convenient ways.
>>   Would you like to work with customised constraints?
>
> Please don't follow this advice.

I have got recurring understanding difficulties with such a response.
Will quoted patch review issues clarified in any other ways?


> Coccinelle is not able to optimize its search process according to
> the information in constraints.  It will needlessly parse many files.

The software supports also SmPL constraints for some reasons.
Why should such functionality be used at all if the immediate reminder is
there seem to be more important optimisation aspects to consider before?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] coccinelle: api: add kvfree script

[Julia Lawall]

On Sat, 6 Jun 2020, Markus Elfring wrote:

> >>> +    E = \(kmalloc@kok\|kzalloc@kok\|krealloc@kok\|kcalloc@kok\|kmalloc_node@kok\|kzalloc_node@kok\|kmalloc_array@kok\|kmalloc_array_node@kok\|kcalloc_node@kok\)(...)
> >>
> >> I would prefer an other coding style here.
> >>
> >> * Items for such SmPL disjunctions can be specified also on multiple lines.
> >>
> >> * The semantic patch language supports further means to handle function name lists
> >>   in more convenient ways.
> >>   Would you like to work with customised constraints?
> >
> > Please don't follow this advice.
>
> I have got recurring understanding difficulties with such a response.
> Will quoted patch review issues clarified in any other ways?
>
>
> > Coccinelle is not able to optimize its search process according to
> > the information in constraints.  It will needlessly parse many files.
>
> The software supports also SmPL constraints for some reasons.
> Why should such functionality be used at all if the immediate reminder is
> there seem to be more important optimisation aspects to consider before?

If the number of functions is really large, constraints may be a better
idea.

If the names of the functions are not actually known, constraints may be a
better idea.

If it is desired to collect some statistics about the matching process,
constraints allow that.

If it is desired to reason about values, for example that a constant is
greater or less than some value, then constraints allow that.

If it is desired to avoid changing code in a particular function, then
constraints allow that.

So there are a lot of reasons why constraints are useful.  There are
likely even more.  But hiding information that could be apparent to the
SmPL compiler and could be used to improve the performance of the matching
process is not one of them.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] coccinelle: api: add kvfree script

[Markus Elfring]

> So there are a lot of reasons why constraints are useful.

Thanks for such a view.

Thus I dare to point the possibility out to consider their application
for mentioned function name lists (besides using SmPL disjunctions).
Can coding style concerns be resolved in a more constructive way then?


> But hiding information that could be apparent to the SmPL compiler
> and could be used to improve the performance of the matching
> process is not one of them.

Will any software extensions become possible also in this area?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Coccinelle: Improving software components around usage of SmPL disjunctions

[Markus Elfring]

>> But hiding information that could be apparent to the SmPL compiler
>> and could be used to improve the performance of the matching
>> process is not one of them.
>
> Will any software extensions become possible also in this area?

You pointed out that SmPL disjunctions can trigger specific consequences
according to functionality which is different from SmPL constraints.
Both application areas support data processing for function name lists
to some degree.
Each element of a SmPL disjunction refers to a fragment of a detailed
source code search pattern. We are discussing use cases where a search
pattern is occasionally restricted in the way that only identifiers
should be found at a specific place.

* Does this detail provide the opportunity to improve the corresponding
  software any more?

* Can a feature request like “Working with variables for case match
  identification by SmPL disjunctions” become more interesting?
  https://github.com/coccinelle/coccinelle/issues/159

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Coccinelle: Extending capabilities for source file pre-selection

[Markus Elfring]

>> Coccinelle is not able to optimize its search process
>
> The software contains some limitations which might be changeable.
>
>
>> according to the information in constraints.
>
> Will related solution ideas become more interesting?

I propose to move parts of the mentioned concerns out of the way.


The software performs a source file pre-selection.
https://github.com/coccinelle/coccinelle/blob/7cf2c23e64066d5249a64a316cc5347831f7a63f/docs/manual/spatch_options.tex#L183

File indexes can become involved according to search tools like
“GLIMPSE” and “GNU idutils”.
They can restrict the support for desired queries on file contents.

I got the impression that SmPL constraint variants can be specified in ways
which would fit also to such data format restrictions.

Higher level SmPL constraints can be more challenging to map to advanced queries.


How are the chances to improve the software situation here?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] coccinelle: api: add kvfree script

[Markus Elfring]

> … Looks like it's impossible to break "when" lines.

Thus I became also curious on clarification for further software
development possibilities around the safer application of code exclusion
specifications by the means of the semantic patch language.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] coccinelle: api: add kvfree script

[Markus Elfring]

>> * A limited search approach was expressed. Will additional source code variations
>>   become relevant?
>>   + switch statement
>>   + if branches with single statements
>>   + conditional operator
>
> The point is that there is a kmalloc in one branch and a vmalloc in
> another branch, so a if with a single branch doesn't seem relevant.

Is an other wording more appropriate to handle if/else statements
without curly brackets?


> The other cases sem highly improbable.

This can be.

But how much do such details influence the confidence level
for such a SmPL script?


>>> +@opportunity depends on !patch …@
>> …
>>> +  E = \(kmalloc\|…\)(..., size, ...)
>>> +  ... when != E = E1
>>> +      when != size = E1
>>
>> I wonder that two assignments should be excluded here according to
>> the same expression metavariable.
>
> Doesn't matter.

Would different variable names reduce the potential for confusion?


> The metavariables are considered separately in the different whens.

Is this information relevant for a better software documentation?


>>> +@pkfree depends on patch exists@
>> …
>>> +- \(kfree\|kvfree\)(E)
>>> ++ vfree(E)
>>
>> Would you like to use a SmPL code variant like the following
>> at any more places?
>> (Is it occasionally helpful to increase the change precision?)
>>
>> +-\(kfree\|kvfree\)
>> ++vfree
>> +      (E)
>
> "increase the change precision" seems to be an obscure way to say "improve
> the formatting".

We come along a different understanding of such a transformation approach
once more.


> Indeed, leaving (E) as is would have the effect of not changing the formatting.

I just propose to leave source code unmodified as much as possible here.


> But the problem seems unlikely for a functoin with such a short name.

This can be.


> And this presentation will likely run afoul of the fact
> that you can't attach + code to a disjunction.

There is a minus character before such SmPL disjunctions.


> So the original presentation was more concise, and should be fine in practice.

Is less duplicated SmPL code useful?

I point a design alternative out.
Would you like to integrate it anyhow?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci