> How would you like to improve the pretty-printing for the Coccinelle software?
I can reproduce this glitch by the combination of test files like the following,
can't you?
@adjustment@
expression result;
@@
if (...)
-{
-result = -ENODEV;
goto
- out_kfree_ioc
+ e_nodev
;
-}
static
> And the unwanted space characters are where?
At the end of the shown three lines.
Would you like to try the following commands out on the updated source file?
* git diff --check …
drivers/scsi/megaraid/megaraid_sas_base.c:8275: trailing whitespace.
…
* scripts/checkpatch.pl --types
> The following SmPL script variant can generate an usable test result.
Yesterday I noticed during the preparation of a corresponding commit
that unwanted space characters were added at three places in the generated
patch.
elfring@Sonne:~/Projekte/Linux/next-patched> spatch --in-place
> * Defalut value
* Default value
> * Directory selection
> * Single file selection
>
> Would it be better?
Partly, yes.
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
>>> Unfortunately, I stumble on the error message “replacement: already
>>> tagged token:
>>> C code context” then.
>
> This is what I would expect.
I came along different application imaginations.
> You could use one rule with an exists to put a position variable in the place
> where you
> Unfortunately, I stumble on the error message “replacement: already tagged
> token:
> C code context” then.
It might be that the transformation approach was too generic for
the implementation of the function “megasas_mgmt_ioctl_fw”.
>> @addition@
>> expression object;
>> @@
>> object = kzalloc(...)
>> ... when any
>> device_unregister(...);
>> out:
>> +kfree(object);
>> return ERR_PTR(...);
>
> There is no reason why a patch should be generated in this case.
We come along different software development views also for
> 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
;
-}
> 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
> 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:
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
> 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 =
>> 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:
*
> I have got the impression then that the exception handling is incomplete so
> far
> at these source code places.
How do you think about to try the following small script for the semantic patch
language out on the directory “drivers/phy/tegra”?
@display@
expression object;
identifier exit;
@@
> static struct platform_driver driver = { . remove = acrtion, };
>
> There is no need for any form of ...
I feel still unsure about the omission of SmPL ellipses
for the search of designated initialisers.
> On the other hand the trailing comma is required.
Thanks for such information.
Can
> I don't know why you would even expect this to work.
I suggest to take another look at related information sources.
https://en.cppreference.com/w/c/language/struct_initialization
> .remove is not a mainingful construct in the C language.
I got an other impression from the declaration of a
Hello,
I hoped to achieve something together with the semantic patch language
by the following search pattern.
@display@
identifier action, driver;
@@
static struct platform_driver driver =
{
<+...
*.remove = action
...+>
};
Unfortunately, I stumble on another error message.
> Would you like to update the provided software documentation together with
> the small extension of this bash script?
>
> Update candidates:
Please reconsider also the section “Using Coccinelle with a single semantic
patch”:
> 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 the paragraph formatting be nicer as an enumeration
as I suggested it previously?
Would you like to update the provided
> I would need the semantic patch to understand the problem.
Please take another look at the corresponding clarification request
with the topic “Checking pretty-printing around a transformation for
array_size()”.
https://lore.kernel.org/cocci/384c6657-5ddc-ff47-14e2-2ffad3129...@web.de/
> But I find the source code formatting occasionally questionable
> according to the Linux coding style.
elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch
../janitor/use_array_size_for_kmemdup2.cocci task_test6.c
…
@@ -1,6 +1,6 @@
void* my_task(void)
{
void* copy;
-copy = kmemdup(ohs,
> Thanks,i think it would be better!
Will this clarification trigger any further collateral evolution
for related information sources?
Example:
https://bottest.wiki.kernel.org/coccicheck
By the way:
Would you like to take another look at the preservation of delimiters
(like space
> Allow defining COCCI as a directory to search SmPL scripts. Start a
> corresponding file determination if the environment variable “COCCI”
> contains an acceptable path.
Can another wording fine-tuning matter for such a change description?
* Allow defining the environment variable “COCCI” as
> I always appreciate the code refactoring
> that reduces the object size.
Would you like to compare effects around conversions for
the mentioned wrapper function any more?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
> How do you think about to use the subject "[PATCH] coccicheck: Support search
> in a selected directory"?
I am unsure about such a reduced variant.
Its acceptance will depend also on other Linux contributors as usual.
Regards,
Markus
___
Cocci
>> How do you think about to use the subject “[PATCH] coccicheck:
>> Support search for SmPL scripts within selected directory hierarchy”?
>
> I would like to use subject as you said.
Thanks for your positive feedback.
> But it seems a little bit long,
I hope that this suggestion is still
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=13b86bc4cd648eae69fdcf3d04b2750c76350053#n656
>
> Huh? I was talking about the log message, not the subject line.
Thanks for this clarification.
> With that degree of
> Second the commit log could be more concise as:
I like your desire for choosing a more appropriate commit message.
> Allow defining COCCI as a directory that contains .cocci files.
I would prefer to concentrate the patch subject on other information.
> In general, at least in simple cases,
I find that the commit message should still be considerably improved.
How do you think about to use the subject “[PATCH] coccicheck:
Support search for SmPL scripts within selected directory hierarchy”?
> Put a modification in scripts/coccicheck which supports users in
> configuring COCCI
> What is the relation between the last two if branches?
A small addition is proposed for this bash script.
> If the first one fails, does that mean that $COCCI has no definition?
No. - It was checked if an empty sting was passed.
A file system check is tried then to determine if a valid
> This patch puts a modification in scripts/coccicheck which supports users
> in configuring COCCI parameter as a directory to traverse files in directory.
* I suggest to improve this change description according to a recommended
“imperative mood”.
Hello,
I have tried another tiny SmPL script out.
@display@
statement s1, s2;
type T;
@@
T nouveau_bo_alloc(...)
{
... when any
{
... when any
* s1
* s2
}
... when any
}
Such a search pattern can point source code places out which can trigger
further development considerations.
>> The uncertainty around the partly (un)documented software behaviour
>> for SmPL when constraints makes it unclear then if the presented
>> source code place should finally be treated as a false positive.
Will it be easier to clarify this open issue based on a smaller
source code example?
>> The uncertainty around the partly (un)documented software behaviour
>> for SmPL when constraints makes it unclear then if the presented
>> source code place should finally be treated as a false positive.
Are you going to do anything more for this concern?
>> Should it have been excluded
>>> I think part of the issue is that the script reports a WARNING
Would anybody like to change this category to “INFO”?
>> How much does this information influence really the stress tolerance
>> and change resistance (or acceptance) for the presented collateral evolution?
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/arm/display/komeda/komeda_dev.c?id=c4b9850b3676869ac0def5885d781d17f64b3a86#n222
>>
>> …
>> @@ -222,… @@ struct komeda_dev *komeda_dev_create(str
>>
>> clk_prepare_enable(mdev->aclk);
>>
>> -
> Markus has been black-listed by several core maintainers already,
I am still curious if this communication filter will ever be adjusted
in more positive directions.
> I think you're wasting your time arguing.
I hope that also this software development discussion can become
more constructive.
> Now I would like to point another analysis concern out.
You informed me about an approach for the usage of when constraints
with the semantic patch language in the following way according to
the discussion topic “Detection of unused function return values”
(on 2011-12-11).
“…
Rule r checks
> What are the additional effects?
I suggest to take another look at the commit
7945f929f1a77a1c8887a97ca07f87626858ff42
("drivers: provide devm_platform_ioremap_resource()" from 2019-02-20)
which triggered the discussed software evolution.
> I think part of the issue is that the script reports a WARNING
How much does this information influence really the stress tolerance
and change resistance (or acceptance) for the presented collateral evolution?
> While it is useful for new drivers to use devm_platform_ioremap_resource,
This is nice.
> this script is currently used to spam maintainers,
This view is unfortunate.
Do we stumble on a target conflict again?
> often updating very old drivers.
This can also happen.
> The net benefit is
>> @display@
>> expression* x, y;
>> @@
>> *y = (x)(...);
>> ... when != y
>
> device_link_add does not look like a function pointer.
Can a known function name be equivalent to a function pointer?
See also:
https://en.cppreference.com/w/c/language/pointer#Pointers_to_functions
> expression *y;
This specific information is interesting.
It was mentioned that further software development concerns
can occur around the possible application of function pointers.
Now I would like to point another analysis concern out.
The following SmPL search approach does not present the
> Function pointers certainly have a return type,
This is fine as usual.
> but the return type could be int.
This can also happen.
But I would like to specify the restriction for a known pointer variant
at this place.
Regards,
Markus
___
Cocci
>> Will the distinction be improved for the safe usage of function pointers
>> also together with the semantic patch language?
>
> I don't see any reason why declaring x as expression *x; should imply
> anything about the type of the value returned by a function pointer.
Function pointers have
>> The metavariable “x” can be restricted to a pointer expression.
>> But does such specification ensure also that the function pointer is
>> connected
>> with a pointer return type?
>
> No.
Thanks for another clarification.
Will the distinction be improved for the safe usage of function
> In int *i; there is no pointer dereference.
Thanks for this clarification.
The semantic patch language syntax needs a different interpretation
of the desired meaning.
How do you think about to add the mentioned detail to the SmPL manual?
> Y should match a pointer-typed expression.
This
>> Which would be a simple way to find only calls of functions
>> which have got a pointer return type?
>
> expression *y;
I have got understanding difficulties with this SmPL variable specification.
I (as a C programmer) would tend to interpret an asterisk before
an expression as a pointer
> Why not just
@display@
expression x, y;
@@
*y = (x)(...);
... when != y
It seems that such a source code search approach has got a high probability
for false positives, doesn't it?
Thus I am trying again to restrict it in reasonable ways.
Which would be a simple way to find only calls of
Hello,
I have tried another small script out for the semantic patch language.
The following variant with two SmPL rules gets accepted by
the Coccinelle software.
@display1@
identifier pointing;
type T;
@@
*T* pointing(...);
@display2@
identifier pointing;
type T;
@@
*T* pointing(...)
{ ... }
>> @replacement@
>> identifier M;
>> @@
>> #define M
>> -snprintf
>> +spgprintf_d
>> (...)
>
> I think you could put \ in your semantic patch, like in C,
The escaping of line breaks would be required for the complete handling of
logical source lines.
Such functionality does not help with the
> You could try it?
I observed that both variants (and others) fail for this issue.
Can you reproduce the reported software difficulties also
in your current test system?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
>> @replacement@
>> identifier M;
>> @@
>> #define M
>> -snprintf
>> +spgprintf_d
>> (...)
>
> I think you could put \ in your semantic patch, like in C,
I am interested also in the support for escaping of line breaks
according to the desired handling of logical source lines.
But how will the
> Anyway, to my understanding the goal was not to modify the #define part,
> but rather only the code part.
Will it become supported by the semantic patch language to change
source code directly after the occurrence of an empty macro?
@replacement@
identifier M;
@@
#define M
-snprintf
>> I got the software development idea then to change the function return type
>> to “void” together with the deletion of return statements by the help of
>> the tool “Coccinelle 1.0.8-4-g842075f7” instead of adjusting return
>> values.
>>
>> @replacement@
>> constant C;
>> identifier action,
> Unfortunately, I do not get the transformation result which I would expect
> for this approach. But I have noticed also that a similar SmPL script
> can work as expected (if a function like “unpin_extent_cache” was not marked
> as “static”?).
The following SmPL script variant does also not work
Hello,
I have noticed that an update suggestion can be provided for
a source file by a known script for the semantic patch language.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/misc/returnvar.cocci?id=8e0d0ad206f08506c893326ca7c9c3d9cc042cef
Update
Hello,
I have noticed that an update suggestion can be provided for
a source file by a known script for the semantic patch language.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/misc/returnvar.cocci?id=8e0d0ad206f08506c893326ca7c9c3d9cc042cef
Update
Hello,
I have tried another source code transformation approach out with
the software combination “Coccinelle 1.0.8-4-g842075f7”.
@replacement@
expression count, item, source, target;
type t1, t2;
@@
target = kmemdup(source,
+ array_size(
(
sizeof(
-
>> BAD:! #define MACRO(name)snprintf(pg, PAGE_SIZE, %u\n", ptr->name)
>
> If a " is missing then the code can't be parsed and Coccinelle won't do
> anything.
I suggest to take additional software improvement opportunities into account.
Should a semantic patch language script become able
>> BAD:! #define MACRO(name)snprintf(pg, PAGE_SIZE, %u\n", ptr->name)
>
> If a " is missing then the code can't be parsed and Coccinelle won't do
> anything.
I would appreciate if the error reporting will become nicer.
> But I don't know if this was a bug in the original code
You
> Macros are supposed to be written on one line.
I guess that you refer to a logical source line.
https://en.cppreference.com/w/c/language/translation_phases#Phase_2
I am looking for better support around changing contents from physical text
lines
also by the means of the semantic patch
>>> * Fix search for parameterised macros with SmPL
>>> https://github.com/coccinelle/coccinelle/issues/94
There are further software development challenges to consider
around preprocessor code.
elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c
Jürgen_Groß-Beispiel2.c
…
ERROR-RECOV:
> BAD:! #define MACRO(name)snprintf(pg, PAGE_SIZE, %u\n", ptr->name)
I hope that a missing double quote character will not be overlooked again
for the specification of a format string.
Regards,
Markus
___
Cocci mailing list
>> * Fix search for parameterised macros with SmPL
>> https://github.com/coccinelle/coccinelle/issues/94
…
> There are no #defines in his semantic patch.
This design detail can probably be taken also better into account
if other software development challenges will finally be solved.
> There are no #defines in his semantic patch.
I noticed this implementation detail already from his clarification request.
> When will you notice that?
Can the shown transformation approaches be applied then also
to replacement lists (or would you prefer to refer to macro bodies)?
Example:
> This works nearly always as expected, but not in some macros.
Do you find any information from previous bug reports
also helpful for the clarification of your use case?
Examples:
* Fix search for parameterised macros with SmPL
https://github.com/coccinelle/coccinelle/issues/94
* Complete
> #define macro2a(par)\
> par++; \
> func(buf, 1, 17)
>
> Nothing happens here either.
May I see also the complete source code examples?
> Because the final ; is omitted the body of the macro is not valid C.
I suggest to reconsider also this
>> I imagine that context-dependent data processing will be needed here
>> to distinguish if preprocessor code should be adjusted (or not).
>> How much will this technical aspect matter?
>
> Macros are supposed to be written on one line.
I suggest to reconsider also this view.
Would you like to
> Changes within #define code should be supported if Coccinelle is able
> to parse the code independently of its usage context.
I suggest to take a closer look at the current software situation.
@replacement@
expression x;
identifier macro;
@@
#define macro(name)
-snprintf
+spgprintf_d
(...,
>> https://github.com/coccinelle/coccinelle/commit/5e3f73c732075f7afadf383d088f3b6d0e1345b1
>
> This seems completely irrelevant.
I got an other view also for this software area.
How many items can be assumed behind the word “etc.” in the section
“upcoming/planned/todo”?
> The question was
>> https://github.com/coccinelle/coccinelle/issues/35
>
> This comment is completely irrelevant.
I have got an other view.
> There is a difference between a virtual rule, that one can declare
> as being matched or not from the command line, and a virtual metavariable
> in a script, that has to
>> Does the semantic patch language (Coccinelle software) support
>> the adjustment of preprocessor definitions as in the shown
>> source code example?
>
> Can't you find the answer to this question by yourself by trying it?
My test did not show an useful patch result for the shown SmPL
>>>virtual patch
>>
>> This variable should be omitted if it will not be used in subsequent SmPL
>> rules.
>
> There is no harm to have it.
Can you get additional software development concerns if you would look at
a bug report on a topic like “Metavariables with the type "virtual" prevent
> > #define MACRO(name)snprintf(pg, PAGE_SIZE, %u\n", ptr->name)
> >
> > Is that on purpose? If yes, how can this be avoided?
>
> I don't think it should be on purpose. Could you send some C code that
> illustrates the problem?
Does the semantic patch language (Coccinelle software) support
>virtual patch
This variable should be omitted if it will not be used in subsequent SmPL rules.
>@@
>expression buf, val;
>@@
>- snprintf(buf, PAGE_SIZE, "%d\n", val)
>+ spgprintf_d(buf, val)
>
> This works nearly always as expected, but not in some macros.
Do you get
>> @display@
>> expression x;
>> identifier f;
>
> You can put f != {likely,unlikely} here.
I would appreciate to achieve a better understanding how these likeliness
annotations can influence the shown source code search approach.
> Maybe there will be some false positives when x->f is in a
>> The possibility remains that also your search pattern suggestion will point
>> update candidates out at other places than the implementation of the
>> mentioned
>> function “imx_pd_bind”.
>
> So many words. So little information.
This can also occasionally happen if the search approach is
> The problem is the __must_check does not mean that the
> return value must be followed by a comparison to NULL and bailing out
> (that can't really be checked), it simply ensures the return value is
> assigned somewhere or used in an if(). So foo->bar = kstrdup() not
> followed by a check of
> And what is the questionable source code place?
I find implementation details occasionally questionable where checks for
variables
which provide a stored function return value are missing.
The possibility remains that also your search pattern suggestion will point
update candidates out at
>> I would like to detect that a corresponding null pointer check would be
>> missing
>> (before the data can be used for further data processing).
>
> * x = kmemdup(...);
> ... when != x
> (
> x->f
> |
> f(...,<+...x...+>,...)
> )
This analysis approach looks promising in principle. I
I would like to detect that a corresponding null pointer check would be
missing
(before the data can be used for further data processing).
>>>
>>> * x = kmemdup(...);
>>> ... when != x
>>> (
>>> x->f
>>> |
>>> f(...,<+...x...+>,...)
>>> )
>>
>> This SmPL search approach does
>> I would like to detect that a corresponding null pointer check would be
>> missing
>> (before the data can be used for further data processing).
>
> * x = kmemdup(...);
> ... when != x
> (
> x->f
> |
> f(...,<+...x...+>,...)
> )
This SmPL search approach does not work as expected for
> I reviewed the functions here and believe the ones you added checks
> for all look good.
Thanks for your positive feedback.
> Though Joe's comment on the relative order of where the
> annotation appears in the function declarations should be addressed in
> a V2 IMO.
Would you be looking for
Hello,
I would like to find specific information in source code
also by the means of the semantic patch language.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h?id=917cda2790c4bd624c5191b8d9edd12121749e86#n182
>> Will the addition of such an annotation in a function declaration
>> become supported for the semantic patch language?
>
> Perhaps some day.
The desired modification for a known preprocessor symbol like “__must_check”
points software development challenges out.
But the following SmPL script
Hello,
I would like to try the following transformation approach out with
the software combination “Coccinelle 1.0.8-4-g842075f7”.
@addition@
identifier f;
type rt != void;
@@
rt
+__must_check
f(...);
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
use_must_check.cocci
Hello,
I would like to try the following transformation approach out with
the software combination “Coccinelle 1.0.8-4-g842075f7”.
@addition@
identifier f;
type rt != void;
@@
rt
+__must_check
f(...);
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
use_must_check.cocci
Hello,
I have tried the following command out with the software combination
“Coccinelle 1.0.8-4-g842075f7”.
elfring@Sonne:~/Projekte/Linux/next-patched> spatch --parse-c
--include-headers-for-types --recursive-includes include/linux/string.h
…
nb good = 444, nb passed = 41 => 7.17%
>> I would find a commit subject like “scripts: add_namespace:
>> Add support for the default coccicheck operation mode” more appropriate
>> (if this software development will be clarified further in the shown
>> direction
>> at all).
>
> Please let this go.
This will not happen for a while.
>
> Now all scripts in scripts/coccinelle to be automatically called
> by coccicheck. However new adding add_namespace.cocci does not
> support report mode, which make coccicheck failed.
> This add "virtual report" to make the coccicheck go ahead smoothly.
I find that this change description needs
>> Would you like to increase your software development attention for
>> efficient system configuration on this issue?
>
> No.
Thanks for this information.
I am still curious if other contributors will care more for this aspect.
Regards,
Markus
>> Would you like to take the change possibility into account
>> that the coccicheck system configuration should be adapted instead?
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccicheck?id=4ea655343ce4180fe9b2c7ec8cb8ef9884a47901#n257
>
> I prefer the one
>> Would you like to take the change possibility into account
>> that the coccicheck system configuration should be adapted instead?
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccicheck?id=4ea655343ce4180fe9b2c7ec8cb8ef9884a47901#n257
>
> I prefer the one
> You gave Acked-by to the one-liner fix "virtual report",
It was (temporarily?) accepted to use the script “add_namespace.cocci”
without such a type of SmPL variable.
> and I am happy to apply it to my tree.
Would you like to take the change possibility into account
that the coccicheck system
> The change is ok.
I would find a slightly different message nicer.
https://lore.kernel.org/cocci/35aa94ff-b101-75b1-9bca-20afef883...@web.de/
https://systeme.lip6.fr/pipermail/cocci/2019-September/006339.html
Regards,
Markus
___
Cocci mailing list
>> How do you think about to avoid the addition of the SmPL variable
>> “virtual report” to the script “add_namespace.cocci” if you would dare
>> to integrate my change proposal for an adjusted directory hierarchy?
>
> Perhaps I'm lazy, but i seems simpler to add 20 characters than to move
> all
> Was this patch posted somewhere?
Yes, of course.
YueHaibing's update suggestions are available also by the usual
mailing list archive interfaces.
How do you think about to avoid the addition of the SmPL variable
“virtual report” to the script “add_namespace.cocci” if you would dare
to
> Was this patch posted somewhere?
Yes, of course.
YueHaibing's update suggestions are available also by the usual
mailing list archive interfaces.
How do you think about to avoid the addition of the SmPL variable
“virtual report” to the script “add_namespace.cocci” if you would dare
to
> minus: parse error:
How do you think about results from a smaller test case?
elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c unicode1.c
…
parse error
= File "unicode1.c", line 4, column 14, charpos = 49
around = ''\0'',
whole content = char16_t z = u'\0';
badcount: 4
bad:
401 - 500 of 1468 matches
Mail list logo
