Re: Rename the SmPL script “kzalloc-….cocci”?

Re: Rename the SmPL script “kzalloc-….cocci”?

[SF Markus Elfring]

Hello,

A script for the semantic patch language was extended in significant ways.

[PATCH v2] Coccinelle: kzalloc-simple: Add “all” zero allocating functions
https://lkml.org/lkml/2017/12/26/182
https://patchwork.kernel.org/patch/10133277/
https://lkml.kernel.org/r/<1514324410-14561-1-git-send-email-himanshujha199640@gmail.com>
https://systeme.lip6.fr/pipermail/cocci/2017-December/004783.html

Now I find that it became more advanced than the previous version.
How do you think about to update also the corresponding file name
(instead of keeping the word “simple” there)?

Regards,
Markus

Re: Rename the SmPL script “kzalloc-….cocci”?

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]



On Tue, 2 Jan 2018, SF Markus Elfring wrote:

> Hello,
>
> A script for the semantic patch language was extended in significant ways.
>
> [PATCH v2] Coccinelle: kzalloc-simple: Add “all” zero allocating functions
> https://lkml.org/lkml/2017/12/26/182
> https://patchwork.kernel.org/patch/10133277/
> https://lkml.kernel.org/r/<1514324410-14561-1-git-send-email-himanshujha199640@gmail.com>
> https://systeme.lip6.fr/pipermail/cocci/2017-December/004783.html
>
> Now I find that it became more advanced than the previous version.
> How do you think about to update also the corresponding file name
> (instead of keeping the word “simple” there)?

Why not send a patch for it yourself?

julia

Re: Rename the SmPL script “kzalloc-….cocci”?

[SF Markus Elfring]

>> Now I find that it became more advanced than the previous version.
>> How do you think about to update also the corresponding file name
>> (instead of keeping the word “simple” there)?
> 
> Why not send a patch for it yourself?

* I would like to check your views around renaming of such files.

* I am unsure which name will be better finally.
  Would we like to achieve another permalink here?

Regards,
Markus

Re: Rename the SmPL script “kzalloc-….cocci”?

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]



On Tue, 2 Jan 2018, SF Markus Elfring wrote:

> >> Now I find that it became more advanced than the previous version.
> >> How do you think about to update also the corresponding file name
> >> (instead of keeping the word “simple” there)?
> >
> > Why not send a patch for it yourself?
>
> * I would like to check your views around renaming of such files.
>
> * I am unsure which name will be better finally.
>   Would we like to achieve another permalink here?

Actually, according to th original name choice it is stillsimple, becaue
it doesn't account for the possibility of many statement between the alloc
and the memset and it doesn't account for different ways of expressing the
size between the two calls.

If you want to be more general than kzalloc, then perhaps
zalloc-simple.cocci would be ok.

julia

Re: Rename the SmPL script “kzalloc-….cocci”?

[SF Markus Elfring]

>> * I am unsure which name will be better finally.
>>   Would we like to achieve another permalink here?
> 
> Actually, according to th original name choice it is stillsimple,

The involved contributors have got different views if the available script
remains “simple” enough at the moment.


> becaue it doesn't account for the possibility of many statement between
> the alloc and the memset

* How close should these function call be kept together?
* Which additional statements would you tolerate between them?


> and it doesn't account for different ways of expressing the size between
> the two calls.

Would you like to get any extensions there?


> If you want to be more general than kzalloc, then perhaps
> zalloc-simple.cocci would be ok.

Will other suffixes be safer for a permanent file name so that confusion
could be avoided around different expectations for “simplicity”?

Regards,
Markus

Re: Rename the SmPL script “kzalloc-….cocci”?

[SF Markus Elfring]

> If you want to be more general than kzalloc, then perhaps
> zalloc-simple.cocci would be ok.

Are you going to commit such a file name adjustment when it seems
that you would not like to accept any other suggestion there?

Regards,
Markus

[PATCH] Coccinelle: Rename the script for a transformation of memory allocations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 3 Jan 2018 12:43:45 +0100

A script for the semantic patch language was extended
in a significant way.
An other file name is more appropriate then to indicate
the provided functionality. Thus rename this file.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 ...kzalloc-simple.cocci => use zalloc functions with extra changes.cocci} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename scripts/coccinelle/api/alloc/{kzalloc-simple.cocci => use zalloc functions with extra changes.cocci} (100%)

diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci
similarity index 100%
rename from scripts/coccinelle/api/alloc/kzalloc-simple.cocci
rename to scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci
-- 
2.15.1

Re: [PATCH] Coccinelle: Rename the script for a transformation of memory allocations

[Julia Lawall]

On Wed, 3 Jan 2018, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 3 Jan 2018 12:43:45 +0100
>
> A script for the semantic patch language was extended
> in a significant way.
> An other file name is more appropriate then to indicate
> the provided functionality. Thus rename this file.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  ...kzalloc-simple.cocci => use zalloc functions with extra changes.cocci} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename scripts/coccinelle/api/alloc/{kzalloc-simple.cocci => use zalloc functions with extra changes.cocci} (100%)
>
> diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci
> similarity index 100%
> rename from scripts/coccinelle/api/alloc/kzalloc-simple.cocci
> rename to scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci

NACK. The name is too long and should not contain spaces.

julia

Re: Coccinelle: Rename the script for a transformation of memory allocations

[SF Markus Elfring]

>> rename from scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>> rename to scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci
> 
> NACK.

It seems that we need a few more tries to achieve the desired consensus.


> The name is too long

How would you like to express the provided functionality in a “permanent” file name?


> and should not contain spaces.

May names contain space characters generally for Linux files?

Regards,
Markus

Re: Coccinelle: Rename the script for a transformation of memory allocations

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]



On Wed, 3 Jan 2018, SF Markus Elfring wrote:

> >> rename from scripts/coccinelle/api/alloc/kzalloc-simple.cocci
> >> rename to scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci
> >
> > NACK.
>
> It seems that we need a few more tries to achieve the desired consensus.
>
>
> > The name is too long
>
> How would you like to express the provided functionality in a
> “permanent” file name?

I have not idea what a permanent file name is.  The current name could be
better without the leading k, but otherwise I think it is fine.

>
>
> > and should not contain spaces.
>
> May names contain space characters generally for Linux files?

I have never seen a file name with spaces in the Linux kernel, but I don't
know an easy way to check for that.  But personally, I really dislike
them, and I will nack any patch that proposes to use one.

julia

>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Coccinelle: Rename the script for a transformation of memory allocations

[SF Markus Elfring]

>> How would you like to express the provided functionality in a
>> “permanent” file name?
> 
> I have not idea what a permanent file name is.

Are you used to the selection of permalinks?


> The current name could be better without the leading k,
> but otherwise I think it is fine.

I suggest to express more details than to keep the word “simple” there.


>> May names contain space characters generally for Linux files?
> 
> I have never seen a file name with spaces in the Linux kernel,
> but I don't know an easy way to check for that.

Corresponding search tools can determine this if it would be desired.


> But personally, I really dislike them,

Interesting …


> and I will nack any patch that proposes to use one.

Would you insist to replace such “special characters” by dashes or underscores?

Regards,
Markus

Re: Coccinelle: Rename the script for a transformation of memory allocations

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]



On Wed, 3 Jan 2018, SF Markus Elfring wrote:

> >> How would you like to express the provided functionality in a
> >> “permanent” file name?
> >
> > I have not idea what a permanent file name is.
>
> Are you used to the selection of permalinks?
>
>
> > The current name could be better without the leading k,
> > but otherwise I think it is fine.
>
> I suggest to express more details than to keep the word “simple” there.
>
>
> >> May names contain space characters generally for Linux files?
> >
> > I have never seen a file name with spaces in the Linux kernel,
> > but I don't know an easy way to check for that.
>
> Corresponding search tools can determine this if it would be desired.
>
>
> > But personally, I really dislike them,
>
> Interesting …
>
>
> > and I will nack any patch that proposes to use one.
>
> Would you insist to replace such “special characters” by dashes or underscores?

Yes.  But I will also nack any very long name.

julia

Re: Coccinelle: Rename the script for a transformation of memory allocations

[SF Markus Elfring]

>> Would you insist to replace such “special characters” by dashes or underscores?
> 
> Yes.  But I will also nack any very long name.

Will any other artificial identifier become appropriate if it would be too hard
to achieve consensus on a more meaningful description in the file name?

Regards,
Markus

Re: Coccinelle: Rename the script for a transformation of memory allocations

[SF Markus Elfring]

>> Would you insist to replace such “special characters” by dashes or underscores?
> 
> Yes.  But I will also nack any very long name.

Would you find the the file name selection “zalloc-extra_changes1.cocci” acceptable?

Regards,
Markus

Re: Coccinelle: Rename the script for a transformation of memory allocations

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]



On Thu, 4 Jan 2018, SF Markus Elfring wrote:

> >> Would you insist to replace such “special characters” by dashes or underscores?
> >
> > Yes.  But I will also nack any very long name.
>
> Would you find the the file name selection “zalloc-extra_changes1.cocci” acceptable?

No.  I do't know what the extra changes are and I don't know what the
number 1 means.

julia

Re: Coccinelle: Rename the script for a transformation of memory allocations

[SF Markus Elfring]

>> Would you find the file name selection “zalloc-extra_changes1.cocci” acceptable?
> 
> No.

Next try then …


> I do't know what the extra changes are

This script for the semantic patch language was designed in the way
that more source code would be adjusted (or just reformatted)
than I see as an essential transformation for the replacement
of function calls in the shown case.

Example:
…
(
- x = kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
|
…


We are discussing consequences from the specification of a different
change granularity for a while.
I hope that another SmPL script variant could be integrated
which will work with a higher precision then so that involved
software developers can choose the preferred transformation approach
also on their own.


> and I don't know what the number 1 means.

How do you think about to start with numbering for subsequent
SmPL scripts?

Regards,
Markus

Re: [v2] Coccinelle: zalloc-simple: Safer transformations with SmPL

[SF Markus Elfring]

> I removed the blank line at EOF,
> then applied to linux-kbuild/misc.

This script for the semantic patch language was extended. Now I find
that this development version contains some update candidates.
Implementation details can become safer in a few software design directions.

I suggest to start with another look at the distinction between pointer data types
and the other types. How much can corresponding source code review result in
further software improvements?

Regards,
Markus

Re: Coccinelle: zalloc-simple: Checking consequences from the usage of at signs in Python strings

[SF Markus Elfring]

> I removed the blank line at EOF,
> then applied to linux-kbuild/misc.

This script for the semantic patch language is using the at sign within string
literals for Python code.

It is nice when this character seems to work also with the current software.
How does its usage fit to the following information in the SmPL manual?

https://github.com/coccinelle/coccinelle/blob/bf1c6a5869dd324f5faeeaa3a12d57270e478b21/docs/manual/cocci_syntax.tex#L50

“…
Furthermore, @ should not be used in this code.
Spatch scans the script code for the next @ and considers that to be the
beginning of the next rule, even if @ occurs within e.g., a comment.
…”


See also:
Configuration or escaping of @ characters for embedded programming language scripts
https://github.com/coccinelle/coccinelle/issues/36

Regards,
Markus

Re: Coccinelle: zalloc-simple: Checking consequences from the usage of at signs in Python strings

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]



On Fri, 19 Jan 2018, SF Markus Elfring wrote:

> > I removed the blank line at EOF,
> > then applied to linux-kbuild/misc.
>
> This script for the semantic patch language is using the at sign within string
> literals for Python code.
>
> It is nice when this character seems to work also with the current software.

So it works, but you are complaining anyway?

> How does its usage fit to the following information in the SmPL manual?
>
> https://github.com/coccinelle/coccinelle/blob/bf1c6a5869dd324f5faeeaa3a12d57270e478b21/docs/manual/cocci_syntax.tex#L50
>
> “…
> Furthermore, @ should not be used in this code.
> Spatch scans the script code for the next @ and considers that to be the
> beginning of the next rule, even if @ occurs within e.g., a comment.
> …”

I guess the conclusion is that it woks in strings (which are pretty
universal) and not in comments (which are language specific).

julia

>
> See also:
> Configuration or escaping of @ characters for embedded programming language scripts
> https://github.com/coccinelle/coccinelle/issues/36
>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Coccinelle: zalloc-simple: Checking consequences from the usage of at signs in Python strings

[SF Markus Elfring]

> So it works, but you are complaining anyway?

I dare to point a contradiction out between two information sources
once more.

Is this SmPL script really working in intended way already?


> I guess the conclusion is that it woks in strings (which are pretty
> universal) and not in comments (which are language specific).

Does the documentation need another update for the Coccinelle software
to achieve the desired correctness?


Which software design direction will get priority in such an use case?

Regards,
Markus

Re: Coccinelle: zalloc-simple: Checking consequences from the usage of at signs in Python strings

[SF Markus Elfring]

> So it works, but you are complaining anyway?

How serious do you interpret such information in the SmPL manual?


>> https://github.com/coccinelle/coccinelle/blob/bf1c6a5869dd324f5faeeaa3a12d57270e478b21/docs/manual/cocci_syntax.tex#L50
>>
>> “…
>> Furthermore, @ should not be used in this code.
…
>> …”
> 
> I guess the conclusion is that it woks in strings (which are pretty
> universal) and not in comments (which are language specific).

Would you like to achieve any safer data processing for potentially “reserved” characters?

Regards,
Markus

Re: [v2] Coccinelle: zalloc-simple: Delete function =?UTF-8?B?4oCca21lbV9jYWNoZV9hbGxvY

[SF Markus Elfring]

> I removed the blank line at EOF,
> then applied to linux-kbuild/misc.

I have taken another look at this script for the semantic patch language.
I imagined that I could refactor the shown SmPL disjunctions a bit.
But I noticed then that these SmPL rules contain a development mistake.

The deletion for a call of the function “memset” depends on the specification
that a size determination is passed by the expression “E1”.
The function “kmem_cache_alloc” was specified despite of the technical detail
that this function does not get a parameter passed which would correspond
to such a size information.
https://elixir.free-electrons.com/linux/v4.15/source/tools/testing/radix-tree/linux/slab.h#L14
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/testing/radix-tree/linux/slab.h?idS7659c08a7da298aa748854f65f2aa1f31b1378#n14

Thus I suggest to remove it from the first two SmPL rules and omit the rule “r4”.
Will the rule set be more consistent then?

Regards,
Markus

=?UTF-8?Q?Re=3A_=5Bv2=5D_Coccinelle=3A_zalloc-simple=3A_Delete_function_=E2=80=9Ckmem=5Fcache=5Fallo

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1354 bytes --]



On Wed, 31 Jan 2018, SF Markus Elfring wrote:

> > I removed the blank line at EOF,
> > then applied to linux-kbuild/misc.
>
> I have taken another look at this script for the semantic patch language.
> I imagined that I could refactor the shown SmPL disjunctions a bit.
> But I noticed then that these SmPL rules contain a development mistake.
>
> The deletion for a call of the function “memset” depends on the specification
> that a size determination is passed by the expression “E1”.
> The function “kmem_cache_alloc” was specified despite of the technical detail
> that this function does not get a parameter passed which would correspond
> to such a size information.
> https://elixir.free-electrons.com/linux/v4.15/source/tools/testing/radix-tree/linux/slab.h#L14
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/testing/radix-tree/linux/slab.h?id=537659c08a7da298aa748854f65f2aa1f31b1378#n14
>
> Thus I suggest to remove it from the first two SmPL rules and omit the rule “r4”.
> Will the rule set be more consistent then?

If E1 is not bound by the kem_cache_alloc rule, then it will match
anything.  The user can check if it is appropriate.

Another option would be to use the type of the variable storing the result
of the call to compute the expected size.  Feel free to send a patch.

julia

Re: [v2] Coccinelle: zalloc-simple: Delete function =?UTF-8?B?4oCca21lbV9jYWNoZV9hbGxvY

[SF Markus Elfring]

>> Will the rule set be more consistent then?
> 
> If E1 is not bound by the kem_cache_alloc rule, then it will match anything.

How much was such a software behaviour intended by the discussed SmPL script?


> The user can check if it is appropriate.

How does such an information fit to expectations for safe source code analysis?


> Another option would be to use the type of the variable storing the result
> of the call to compute the expected size.

How would this suggestion help here?

Regards,
Markus

[PATCH] Coccinelle: zalloc-simple: Delete function "kmem_cache_alloc" from SmPL rules

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 1 Feb 2018 10:20:47 +0100

The deletion for a call of the function "memset" depends on
the specification that a size determination is passed by
the expression "E1".
The function "kmem_cache_alloc" was specified despite of the technical
detail that this function does not get a parameter passed which would
correspond to such a size information.

Thus remove it from the first two SmPL rules and omit the rule "r4".

Link: https://elixir.free-electrons.com/linux/v4.15/source/tools/testing/radix-tree/linux/slab.h#L14
Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/testing/radix-tree/linux/slab.h?idò6e52e08ab8e56f528ac14aa7929b3477de5616#n14
Fixes: 5e2d9da5b9ba350a4f13bd3b255be046bcf86465 ("Coccinelle: kzalloc-simple: Add all zero allocating functions")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 scripts/coccinelle/api/alloc/zalloc-simple.cocci | 41 +-----------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/scripts/coccinelle/api/alloc/zalloc-simple.cocci b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
index 92b20913055f..3bee6cdd99ea 100644
--- a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
@@ -34,7 +34,7 @@ statement S;
 @@
 
 * x = (T)\(kmalloc(E1, ...)\|vmalloc(E1)\|dma_alloc_coherent(...,E1,...)\|
-  kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
+  kmalloc_node(E1, ...)\|kmem_alloc(E1, ...)\|
   devm_kmalloc(...,E1,...)\|kvmalloc(E1, ...)\|pci_alloc_consistent(...,E1,...)\|
   kvmalloc_node(E1,...)\);
   if ((x=NULL) || ...) S
@@ -88,15 +88,6 @@ statement S;
 - x = (T)kmalloc_node(E1,E2,E3);
 + x = (T)kzalloc_node(E1,E2,E3);
 |
-- x = kmem_cache_alloc(E3,E4);
-+ x = kmem_cache_zalloc(E3,E4);
-|
-- x = (T *)kmem_cache_alloc(E3,E4);
-+ x = kmem_cache_zalloc(E3,E4);
-|
-- x = (T)kmem_cache_alloc(E3,E4);
-+ x = (T)kmem_cache_zalloc(E3,E4);
-|
 - x = kmem_alloc(E1,E2);
 + x = kmem_zalloc(E1,E2);
 |
@@ -268,36 +259,6 @@ x << r3.x;
 msg="WARNING: kzalloc_node should be used for %s, instead of kmalloc_node/memset" % (x)
 coccilib.report.print_report(p[0], msg)
 
-//-----------------------------------------------------------------
-@r4 depends on org || report@
-type T, T2;
-expression x;
-expression E1,E2,E3;
-statement S;
-position p;
-@@
-
- x = (T)kmem_cache_alloc@p(E2,E3);
- if ((x=NULL) || ...) S
- memset((T2)x,0,E1);
-
-@script:python depends on org@
-p << r4.p;
-x << r4.x;
-@@
-
-msg="%s" % (x)
-msg_safe=msg.replace("[","@(").replace("]",")")
-coccilib.org.print_todo(p[0], msg_safe)
-
-@script:python depends on report@
-p << r4.p;
-x << r4.x;
-@@
-
-msg="WARNING: kmem_cache_zalloc should be used for %s, instead of kmem_cache_alloc/memset" % (x)
-coccilib.report.print_report(p[0], msg)
-
 //-----------------------------------------------------------------
 @r5 depends on org || report@
 type T, T2;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Coccinelle: zalloc-simple: Delete function "kmem_cache_alloc" from SmPL rules

[Julia Lawall]

On Thu, 1 Feb 2018, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 1 Feb 2018 10:20:47 +0100
>
> The deletion for a call of the function "memset" depends on
> the specification that a size determination is passed by
> the expression "E1".
> The function "kmem_cache_alloc" was specified despite of the technical
> detail that this function does not get a parameter passed which would
> correspond to such a size information.
>
> Thus remove it from the first two SmPL rules and omit the rule "r4".

Nack.  It should be supported by the size determined in another way.

julia

> Link: https://elixir.free-electrons.com/linux/v4.15/source/tools/testing/radix-tree/linux/slab.h#L14
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/testing/radix-tree/linux/slab.h?idò6e52e08ab8e56f528ac14aa7929b3477de5616#n14
> Fixes: 5e2d9da5b9ba350a4f13bd3b255be046bcf86465 ("Coccinelle: kzalloc-simple: Add all zero allocating functions")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  scripts/coccinelle/api/alloc/zalloc-simple.cocci | 41 +-----------------------
>  1 file changed, 1 insertion(+), 40 deletions(-)
>
> diff --git a/scripts/coccinelle/api/alloc/zalloc-simple.cocci b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> index 92b20913055f..3bee6cdd99ea 100644
> --- a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> +++ b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> @@ -34,7 +34,7 @@ statement S;
>  @@
>
>  * x = (T)\(kmalloc(E1, ...)\|vmalloc(E1)\|dma_alloc_coherent(...,E1,...)\|
> -  kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
> +  kmalloc_node(E1, ...)\|kmem_alloc(E1, ...)\|
>    devm_kmalloc(...,E1,...)\|kvmalloc(E1, ...)\|pci_alloc_consistent(...,E1,...)\|
>    kvmalloc_node(E1,...)\);
>    if ((x=NULL) || ...) S
> @@ -88,15 +88,6 @@ statement S;
>  - x = (T)kmalloc_node(E1,E2,E3);
>  + x = (T)kzalloc_node(E1,E2,E3);
>  |
> -- x = kmem_cache_alloc(E3,E4);
> -+ x = kmem_cache_zalloc(E3,E4);
> -|
> -- x = (T *)kmem_cache_alloc(E3,E4);
> -+ x = kmem_cache_zalloc(E3,E4);
> -|
> -- x = (T)kmem_cache_alloc(E3,E4);
> -+ x = (T)kmem_cache_zalloc(E3,E4);
> -|
>  - x = kmem_alloc(E1,E2);
>  + x = kmem_zalloc(E1,E2);
>  |
> @@ -268,36 +259,6 @@ x << r3.x;
>  msg="WARNING: kzalloc_node should be used for %s, instead of kmalloc_node/memset" % (x)
>  coccilib.report.print_report(p[0], msg)
>
> -//-----------------------------------------------------------------
> -@r4 depends on org || report@
> -type T, T2;
> -expression x;
> -expression E1,E2,E3;
> -statement S;
> -position p;
> -@@
> -
> - x = (T)kmem_cache_alloc@p(E2,E3);
> - if ((x=NULL) || ...) S
> - memset((T2)x,0,E1);
> -
> -@script:python depends on org@
> -p << r4.p;
> -x << r4.x;
> -@@
> -
> -msg="%s" % (x)
> -msg_safe=msg.replace("[","@(").replace("]",")")
> -coccilib.org.print_todo(p[0], msg_safe)
> -
> -@script:python depends on report@
> -p << r4.p;
> -x << r4.x;
> -@@
> -
> -msg="WARNING: kmem_cache_zalloc should be used for %s, instead of kmem_cache_alloc/memset" % (x)
> -coccilib.report.print_report(p[0], msg)
> -
>  //-----------------------------------------------------------------
>  @r5 depends on org || report@
>  type T, T2;
> --
> 2.16.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Coccinelle: zalloc-simple: Delete function "kmem_cache_alloc" from SmPL rules

[SF Markus Elfring]

>> The function "kmem_cache_alloc" was specified despite of the technical
>> detail that this function does not get a parameter passed which would
>> correspond to such a size information.
>>
>> Thus remove it from the first two SmPL rules and omit the rule "r4".
> 
> Nack.

I find such a rejection surprising once more.


> It should be supported by the size determined in another way.

I am curious on how our different views could be clarified further
for this special software situation.

* Do we agree that a proper size determination is essential for every
  condition in the discussed SmPL rules together with forwarding
  this information?

* How can a name be ever relevant (within the published SmPL approach)
  for a function when it was designed in the way that it should generally
  work without a size parameter?

Regards,
Markus

Re: Coccinelle: zalloc-simple: Delete function "kmem_cache_alloc" from SmPL rules

[Julia Lawall]

On Thu, 1 Feb 2018, SF Markus Elfring wrote:

> >> The function "kmem_cache_alloc" was specified despite of the technical
> >> detail that this function does not get a parameter passed which would
> >> correspond to such a size information.
> >>
> >> Thus remove it from the first two SmPL rules and omit the rule "r4".
> >
> > Nack.
>
> I find such a rejection surprising once more.
>
>
> > It should be supported by the size determined in another way.
>
> I am curious on how our different views could be clarified further
> for this special software situation.
>
> * Do we agree that a proper size determination is essential for every
>   condition in the discussed SmPL rules together with forwarding
>   this information?

No.  I don't mind a few false positives.  The user can look at the answer
and see if it is a false positive or not.

Furthermore, I told you how to address this function so that the size
issue would be taken care of.  That is the patch that I would accept.

>
> * How can a name be ever relevant (within the published SmPL approach)
>   for a function when it was designed in the way that it should generally
>   work without a size parameter?

No idea what this means.

julia

Re: Coccinelle: zalloc-simple: Delete function "kmem_cache_alloc" from SmPL rules

[SF Markus Elfring]

>> * Do we agree that a proper size determination is essential for every
>>   condition in the discussed SmPL rules together with forwarding
>>   this information?
> 
> No.  I don't mind a few false positives.

I have got other source code analysis expectations there.

This SmPL script contains the tag “Confidence: High”.


> The user can look at the answer and see if it is a false positive or not.

The situation is questionable because of a specific software design detail.

Unsafe source code search patterns could be stored under other script names
(or even different directories).
How do you think about to move the SmPL code (which I proposed for deletion)
to another script if you would insist to preserve it?


> Furthermore, I told you how to address this function so that the size
> issue would be taken care of.

You offered another bit of information.

I find your interpretation of this aspect also unclear at the moment.


> That is the patch that I would accept.

Are there any better development solutions left over?


>> * How can a name be ever relevant (within the published SmPL approach)
>>   for a function when it was designed in the way that it should generally
>>   work without a size parameter?
> 
> No idea what this means.

I am trying again to resolve corresponding communication difficulties.
Thus I suggest to take another look at the following SmPL code fragment.

…
  kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
…
* memset((T2)x,0,E1);
…


How many constraints should be considered for function parameters here?

Regards,
Markus

Re: Coccinelle: zalloc-simple: Checking consistency for SmPL rules

[SF Markus Elfring]

>> * Do we agree that a proper size determination is essential for every
>>   condition in the discussed SmPL rules together with forwarding
>>   this information?
> 
> No.  I don't mind a few false positives.

Do you care to split SmPL rules by their confidence category in such an use case?

Regards,
Markus