Re: [Cocci] coccinelle: bool if (foo) return true; else return false;
On 04/19/2016 09:15 PM, Julia Lawall wrote:
>
>
> On Tue, 19 Apr 2016, Joe Perches wrote:
>
>> There's ~150 of these in the kernel.
>>
>> Maybe there's use for this conversion to be added
>> to scripts/coccinelle/misc/boolreturn.cocci or in
>> a separate file.
>>
>> $ cat booltruefalse.cocci
>> @@
>> identifier fn;
>> expression e;
>> typedef bool;
>> symbol true;
>> symbol false;
>> @@
>>
>> bool fn ( ... )
>> {
>> <...
>> -if (e) return true; else return false;
>> +return e;
Shouldn't that be:
return !!e
?
>> ...>
>> }
>>
>> @@
>> identifier fn;
>> expression e;
>> @@
>>
>> bool fn ( ... )
>> {
>> <...
>> -if (e) return false; else return true;
>> +return !e;
>> ...>
>> }
>
> Thanks for the suggestion. I will take care of it shortly.
bye
michael
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: bool if (foo) return true; else return false;
On Wed, 2016-04-20 at 09:19 +0200, Michael Stefaniuc wrote:
> On 04/19/2016 09:15 PM, Julia Lawall wrote:
> > On Tue, 19 Apr 2016, Joe Perches wrote:
> > > There's ~150 of these in the kernel.
> > >
> > > Maybe there's use for this conversion to be added
> > > to scripts/coccinelle/misc/boolreturn.cocci or in
> > > a separate file.
> > >
> > > $ cat booltruefalse.cocci
> > > @@
> > > identifier fn;
> > > expression e;
> > > typedef bool;
> > > symbol true;
> > > symbol false;
> > > @@
> > >
> > > bool fn ( ... )
> > > {
> > > <...
> > > - if (e) return true; else return false;
> > > + return e;
> Shouldn't that be:
> return !!e
> ?
No, it's not necessary.
The compiler does that because the return type is bool
6.3.1.2 Boolean type
When any scalar value is converted to _Bool, the result is 0 if the value
compares equal
to 0; otherwise, the result is 1.
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] Coccinelle: setup_timer: Add space in front of parentheses
Dne 20.3.2016 v 13:25 Julia Lawall napsal(a): > > > On Sun, 20 Mar 2016, Vaishali Thakkar wrote: > >> Add space in front of the offending parentheses to silent the >> parse error for older Coccinelle versions. This makes the rule >> usable with all Coccinelle versions. >> >> Reported-by: Nishanth Menon >> Signed-off-by: Vaishali Thakkar > > Acked-by: Julia Lawall Applied to kbuild.git#misc. Michal ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] scripts: coccinelle: remove check to move constants to right
On Sat, Mar 19, 2016 at 06:43:08PM +0100, Julia Lawall wrote: > On Sat, 19 Mar 2016, Wolfram Sang wrote: > > > The header mentions this check depends on personal taste. I agree. > > Running coccicheck on patches before I apply them, this SmPL produced > > enough false positives for me that I'd rather see it removed. > > An improvement is coming up, that should be more acceptable. However, > it's being held up by the need for some bug fixes in Coccinelle. A > release of Coccinelle is planned for the beginning of April. Perhaps > it is just as well to just remove this version for now. > > Acked-by: Julia Lawall Applied to kbuild.git#misc. Michal ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] coccinelle: bool if (foo) return true; else return false;
There's ~150 of these in the kernel.
Maybe there's use for this conversion to be added
to scripts/coccinelle/misc/boolreturn.cocci or in
a separate file.
$ cat booltruefalse.cocci
@@
identifier fn;
expression e;
typedef bool;
symbol true;
symbol false;
@@
bool fn ( ... )
{
<...
- if (e) return true; else return false;
+ return e;
...>
}
@@
identifier fn;
expression e;
@@
bool fn ( ... )
{
<...
- if (e) return false; else return true;
+ return !e;
...>
}
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Matching if a function parameter is used at all
On 20 April 2016 at 06:17, Julia Lawall wrote:
>
> On Tue, 19 Apr 2016, Kieran Bingham wrote:
>
> > Hi All,
> >
> > I would like to match on when a function *doesn't* use a parameter, so
> that
> > those use-cases can be swapped to new function prototype, as an interim -
> > leaving code which needs fixing up still compiling successfully.
> > To do this, I believe I need to identify the functions that use that
> > parameter, and then make the conversion dependant upon that hunk.
> >
> > I've been trying various combinations of below:
> >
> > ==
> >
> > // Detect if id is used in the probe function
> >
> > @ probe_id_used depends on driver @
> > identifier driver.probefunc;
> > identifier client;
> > identifier idt;
> > @@
> > static int probefunc( struct i2c_client *client, const struct
> i2c_device_id
> > *idt)
> > {
> > ...
> > idt
> > ...
> > }
> >
> > //
> > // Then, only if requirements are met, start making adjustments
> > //
> >
> > // Convert the probe function
> >
> > @ convert depends on driver && !probe_id_used @
> > identifier driver.probefunc;
> > identifier client;
> > identifier id;
> > @@
> > static int probefunc(
> > struct i2c_client *client
> > -, const struct i2c_device_id *id
> > )
> > { ... }
> > ==
> >
> > However, I can't get the probe_id_used to match correctly for the idt
> > identifier. I'd like it to match if it is mentioned at all in the
> function,
> > but I have also tried mapping a disjunction on known conditionals to no
> > avail:
> >
> > ==
> > // Detect if id is used in the probe function
> >
> > @ probe_id_used depends on driver @
> > identifier driver.probefunc;
> > identifier client;
> > identifier idt;
> > expression e;
> > statement S;
> > @@
> > static int probefunc( struct i2c_client *client, const struct
> i2c_device_id
> > *idt)
> > {
> > ...
> > (
> > if(idt) S;
> > |
> > if (idt) { ... }
> > |
> > e = idt->name
> > |
> > e = idt->driver_data
> > )
> > ...
> > }
> > ==
> >
> > And that still doesn't match
> >
> > Is there an easier / correct way to achieve what I'm trying to do ? Or
> am I
> > missing something obvious?
>
> I think that
>
> f(...,T i,...) { ... when != i
> }
>
> would do what you want?
>
Aha - yes, I believe this did match as I needed, with the hunk renamed for
the inversion
// Detect if id is used in the probe function
@ probe_id_unused depends on driver @
identifier driver.probefunc;
identifier client;
identifier idt;
@@
static int probefunc( struct i2c_client *client, const struct i2c_device_id
*idt)
{
...
when != idt
}
but I think I now understand that the root of my issue as being something
different. It looks like I had got the match working in other ways but the
depends chain wasn't working.
Your version is cleaner though - so thank-you for that!
I will post a new thread for the depends issue, as I think that is separate
--
Regards
Kieran
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Dependencies between spatch hunks
Hi all,
I am finding that a rule which matches a part of code, sometimes causes
hunks which depend on it to act, and sometimes not to act
My spatch is successfully removing and converting hunks that I desire
changed, however the requirement became apparent *to not process the
file at all* if the variable is used in the probe function, so I added
in a dependency on probe_id_unused (established in my previous mail thread).
Therefore, I would expect to be able to set the 'depends on' to be on
'probe_id_unused' for each of the actions, and have actions only taken
if the full dependency chain (C4->C1 below) is met.
However, I get a non-consistent application of this, where some hunks
operate when (I believe) they shouldn't:
My full spatch for reference, is at:
https://gist.github.com/kbingham/96477177dd20a72b1c2f
In essence, it does the following {C}hecks:
C1 - of_dev_id_present : Check for a struct of_device_id
C2 - dev_id : Check for a struct i2c_device_id
C3 - driver : Check and identify the probefunc in the driver structure
C4 - probe_id_unused : Establish if the id is used in the probe function
Where C4 depends on C3 depends on C2 depends on C1
The aim is that if all of the above checks/identifiers are met, it will
take the following actions:
A rewrite the probe function declaration
B re-point the function pointer in the driver structure
C remove the i2c_device_id reference
D remove the i2c_device_id array
E remove the MODULE_DEVICE_TABLE macro
For this example, I'll take three files from the kernel source, all of
which meet conditions C1 - > C3 (but only F3 meets C4):
F1: drivers/gpio/gpio-pca953x.c : probe id used ( ! C4 )
F2: drivers/staging/iio/light/isl29018.c : probe id used ( ! C4 )
F3: sound/soc/codecs/wm8737.c : probe id UNUSED ( C4 )
If all the actions (A->E) start with @ depends on driver @ (to depend on
C3) all the actions complete on these files. (Shown in sequence in the c
file)
F1 : D E A B C
F2 : A D E B C
F3 : A D E B C
However, if all the actions depend on @ depends on probe_id_unused @
(depends on C4), Some actions complete, and some do not!
F1 : D E C (Unexpected behaviour - I expect F1 : )
F2 : D E C (Unexpected behaviour - I expect F2 : )
F3 : A D E B C (Expected behaviour)
So of course, I want actions D E and C to *not* complete on F1 and F2,
but I can't understand why they do not comply with their 'depends'
chain. Am I looking at a bug in Coccinelle here or a bug with my
interpretation of the depends keyword?
Sorry for the long mail, and look forward to any ideas!
--
Regards
Kieran
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On Wed, 20 Apr 2016, Kieran Bingham wrote:
> Hi all,
>
> I am finding that a rule which matches a part of code, sometimes causes
> hunks which depend on it to act, and sometimes not to act
>
> My spatch is successfully removing and converting hunks that I desire
> changed, however the requirement became apparent *to not process the
> file at all* if the variable is used in the probe function, so I added
> in a dependency on probe_id_unused (established in my previous mail thread).
I'll check on the rest shortly, but if you really want to not process the
file at all, just look for the condition in which that is the case, and
then write a python rule that depends on it, and run Cocci.exit() (not
sure about the exact syntax - writing an ocaml rule and putting
Coccilib.exit() will also work).
julia
> Therefore, I would expect to be able to set the 'depends on' to be on
> 'probe_id_unused' for each of the actions, and have actions only taken
> if the full dependency chain (C4->C1 below) is met.
>
> However, I get a non-consistent application of this, where some hunks
> operate when (I believe) they shouldn't:
>
> My full spatch for reference, is at:
>
> https://gist.github.com/kbingham/96477177dd20a72b1c2f
>
> In essence, it does the following {C}hecks:
> C1 - of_dev_id_present : Check for a struct of_device_id
> C2 - dev_id : Check for a struct i2c_device_id
> C3 - driver : Check and identify the probefunc in the driver structure
> C4 - probe_id_unused : Establish if the id is used in the probe function
>
> Where C4 depends on C3 depends on C2 depends on C1
>
> The aim is that if all of the above checks/identifiers are met, it will
> take the following actions:
> A rewrite the probe function declaration
> B re-point the function pointer in the driver structure
> C remove the i2c_device_id reference
> D remove the i2c_device_id array
> E remove the MODULE_DEVICE_TABLE macro
>
> For this example, I'll take three files from the kernel source, all of
> which meet conditions C1 - > C3 (but only F3 meets C4):
>
> F1: drivers/gpio/gpio-pca953x.c : probe id used ( ! C4 )
> F2: drivers/staging/iio/light/isl29018.c : probe id used ( ! C4 )
> F3: sound/soc/codecs/wm8737.c : probe id UNUSED ( C4 )
>
> If all the actions (A->E) start with @ depends on driver @ (to depend on
> C3) all the actions complete on these files. (Shown in sequence in the c
> file)
>
> F1 : D E A B C
> F2 : A D E B C
> F3 : A D E B C
>
> However, if all the actions depend on @ depends on probe_id_unused @
> (depends on C4), Some actions complete, and some do not!
>
> F1 : D E C (Unexpected behaviour - I expect F1 : )
> F2 : D E C (Unexpected behaviour - I expect F2 : )
> F3 : A D E B C (Expected behaviour)
>
> So of course, I want actions D E and C to *not* complete on F1 and F2,
> but I can't understand why they do not comply with their 'depends'
> chain. Am I looking at a bug in Coccinelle here or a bug with my
> interpretation of the depends keyword?
>
> Sorry for the long mail, and look forward to any ideas!
>
> --
> Regards
>
> Kieran
> ___
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On 20/04/16 16:53, Julia Lawall wrote:
>
> On Wed, 20 Apr 2016, Kieran Bingham wrote:
>
>> Hi all,
>>
>> I am finding that a rule which matches a part of code, sometimes causes
>> hunks which depend on it to act, and sometimes not to act
>>
>> My spatch is successfully removing and converting hunks that I desire
>> changed, however the requirement became apparent *to not process the
>> file at all* if the variable is used in the probe function, so I added
>> in a dependency on probe_id_unused (established in my previous mail thread).
>
> I'll check on the rest shortly, but if you really want to not process the
> file at all, just look for the condition in which that is the case, and
> then write a python rule that depends on it, and run Cocci.exit() (not
> sure about the exact syntax - writing an ocaml rule and putting
> Coccilib.exit() will also work).
Thanks Julia,
The following worked for my needs:
@ script:python depends on driver && !probe_id_unused @
@@
print("Probe function uses the ID parameter")
cocci.exit()
However, interestingly - it's not useful if I specify a minimal set of
files on the command line:
Using the command:
spatch --linux-spacing --sp-file patches/i2c-dt.cocci $A $B $C $D
HANDLING: sound/soc/codecs/wm8737.c drivers/staging/iio/light/isl29018.c
drivers/gpio/gpio-pca953x.c ./drivers/rtc/rtc-ds1307.c
Probe function uses the ID parameter
kbingham@CookieMonster:/opt/projects/linux/kbuild-bbb/sources/linux$
appears to stop at the first file (which is expected to stop) without
processing the next files.
However, simply using the '.' target - does appear to iterate through
the entire sources successfully, parsing files it can, and stopping on
files that it should stop on.
Thanks again,
--
Kieran
> julia
>
>> Therefore, I would expect to be able to set the 'depends on' to be on
>> 'probe_id_unused' for each of the actions, and have actions only taken
>> if the full dependency chain (C4->C1 below) is met.
>>
>> However, I get a non-consistent application of this, where some hunks
>> operate when (I believe) they shouldn't:
>>
>> My full spatch for reference, is at:
>>
>> https://gist.github.com/kbingham/96477177dd20a72b1c2f
>>
>> In essence, it does the following {C}hecks:
>> C1 - of_dev_id_present : Check for a struct of_device_id
>> C2 - dev_id : Check for a struct i2c_device_id
>> C3 - driver : Check and identify the probefunc in the driver structure
>> C4 - probe_id_unused : Establish if the id is used in the probe function
>>
>> Where C4 depends on C3 depends on C2 depends on C1
>>
>> The aim is that if all of the above checks/identifiers are met, it will
>> take the following actions:
>> A rewrite the probe function declaration
>> B re-point the function pointer in the driver structure
>> C remove the i2c_device_id reference
>> D remove the i2c_device_id array
>> E remove the MODULE_DEVICE_TABLE macro
>>
>> For this example, I'll take three files from the kernel source, all of
>> which meet conditions C1 - > C3 (but only F3 meets C4):
>>
>> F1: drivers/gpio/gpio-pca953x.c : probe id used ( ! C4 )
>> F2: drivers/staging/iio/light/isl29018.c : probe id used ( ! C4 )
>> F3: sound/soc/codecs/wm8737.c : probe id UNUSED ( C4 )
>>
>> If all the actions (A->E) start with @ depends on driver @ (to depend on
>> C3) all the actions complete on these files. (Shown in sequence in the c
>> file)
>>
>> F1 : D E A B C
>> F2 : A D E B C
>> F3 : A D E B C
>>
>> However, if all the actions depend on @ depends on probe_id_unused @
>> (depends on C4), Some actions complete, and some do not!
>>
>> F1 : D E C (Unexpected behaviour - I expect F1 : )
>> F2 : D E C (Unexpected behaviour - I expect F2 : )
>> F3 : A D E B C (Expected behaviour)
>>
>> So of course, I want actions D E and C to *not* complete on F1 and F2,
>> but I can't understand why they do not comply with their 'depends'
>> chain. Am I looking at a bug in Coccinelle here or a bug with my
>> interpretation of the depends keyword?
>>
>> Sorry for the long mail, and look forward to any ideas!
>>
>> --
>> Regards
>>
>> Kieran
>> ___
>> Cocci mailing list
>> Cocci@systeme.lip6.fr
>> https://systeme.lip6.fr/mailman/listinfo/cocci
>>
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On Wed, 20 Apr 2016, Kieran Bingham wrote:
> On 20/04/16 16:53, Julia Lawall wrote:
> >
> > On Wed, 20 Apr 2016, Kieran Bingham wrote:
> >
> >> Hi all,
> >>
> >> I am finding that a rule which matches a part of code, sometimes causes
> >> hunks which depend on it to act, and sometimes not to act
> >>
> >> My spatch is successfully removing and converting hunks that I desire
> >> changed, however the requirement became apparent *to not process the
> >> file at all* if the variable is used in the probe function, so I added
> >> in a dependency on probe_id_unused (established in my previous mail
> >> thread).
> >
> > I'll check on the rest shortly, but if you really want to not process the
> > file at all, just look for the condition in which that is the case, and
> > then write a python rule that depends on it, and run Cocci.exit() (not
> > sure about the exact syntax - writing an ocaml rule and putting
> > Coccilib.exit() will also work).
>
>
> Thanks Julia,
>
> The following worked for my needs:
>
> @ script:python depends on driver && !probe_id_unused @
> @@
> print("Probe function uses the ID parameter")
> cocci.exit()
>
> However, interestingly - it's not useful if I specify a minimal set of
> files on the command line:
>
> Using the command:
> spatch --linux-spacing --sp-file patches/i2c-dt.cocci $A $B $C $D
> HANDLING: sound/soc/codecs/wm8737.c drivers/staging/iio/light/isl29018.c
> drivers/gpio/gpio-pca953x.c ./drivers/rtc/rtc-ds1307.c
> Probe function uses the ID parameter
> kbingham@CookieMonster:/opt/projects/linux/kbuild-bbb/sources/linux$
>
> appears to stop at the first file (which is expected to stop) without
> processing the next files.
>
> However, simply using the '.' target - does appear to iterate through
> the entire sources successfully, parsing files it can, and stopping on
> files that it should stop on.
Putting multiple files on the command line is not the right way to process
multiple files. What that means is to process those files all at once.
Normally Coccinelle processes only one file at a time, which means that
when there is a call from a function in one file to a function in another,
it won't find the definition of the called function. If you put multiple
files on the command line, it will see them all at once. The downside
though is that Coccinelle will ultimately run more slowly, because it will
be using a lot more memory at a given time.
If you have a directory where you want to process only some of the files,
you can use the --file-groups option. could be eg
path/foo.c
path/bar.c
path/xyz.c
Then foo.c and bar.c will be processed at once, and xyz.c will be
processed separately.
But it could be easier to just work on the whole directory and let
Coccinelle to fail to do anything with most of the files.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On Wed, 20 Apr 2016, Kieran Bingham wrote:
> Hi all,
>
> I am finding that a rule which matches a part of code, sometimes causes
> hunks which depend on it to act, and sometimes not to act
>
> My spatch is successfully removing and converting hunks that I desire
> changed, however the requirement became apparent *to not process the
> file at all* if the variable is used in the probe function, so I added
> in a dependency on probe_id_unused (established in my previous mail thread).
>
> Therefore, I would expect to be able to set the 'depends on' to be on
> 'probe_id_unused' for each of the actions, and have actions only taken
> if the full dependency chain (C4->C1 below) is met.
>
> However, I get a non-consistent application of this, where some hunks
> operate when (I believe) they shouldn't:
>
> My full spatch for reference, is at:
>
> https://gist.github.com/kbingham/96477177dd20a72b1c2f
>
> In essence, it does the following {C}hecks:
> C1 - of_dev_id_present : Check for a struct of_device_id
> C2 - dev_id : Check for a struct i2c_device_id
> C3 - driver : Check and identify the probefunc in the driver structure
> C4 - probe_id_unused : Establish if the id is used in the probe function
>
> Where C4 depends on C3 depends on C2 depends on C1
>
> The aim is that if all of the above checks/identifiers are met, it will
> take the following actions:
> A rewrite the probe function declaration
> B re-point the function pointer in the driver structure
> C remove the i2c_device_id reference
> D remove the i2c_device_id array
> E remove the MODULE_DEVICE_TABLE macro
>
> For this example, I'll take three files from the kernel source, all of
> which meet conditions C1 - > C3 (but only F3 meets C4):
>
> F1: drivers/gpio/gpio-pca953x.c : probe id used ( ! C4 )
> F2: drivers/staging/iio/light/isl29018.c : probe id used ( ! C4 )
> F3: sound/soc/codecs/wm8737.c : probe id UNUSED ( C4 )
>
> If all the actions (A->E) start with @ depends on driver @ (to depend on
> C3) all the actions complete on these files. (Shown in sequence in the c
> file)
>
> F1 : D E A B C
> F2 : A D E B C
> F3 : A D E B C
>
> However, if all the actions depend on @ depends on probe_id_unused @
> (depends on C4), Some actions complete, and some do not!
>
> F1 : D E C (Unexpected behaviour - I expect F1 : )
> F2 : D E C (Unexpected behaviour - I expect F2 : )
> F3 : A D E B C (Expected behaviour)
>
> So of course, I want actions D E and C to *not* complete on F1 and F2,
> but I can't understand why they do not comply with their 'depends'
> chain. Am I looking at a bug in Coccinelle here or a bug with my
> interpretation of the depends keyword?
>
> Sorry for the long mail, and look forward to any ideas!
What version of Coccinelle do you have? I get the expected behavior. No
change on F1 and F2, and a change on F3.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On 20 Apr 2016 17:31, "Julia Lawall" wrote:
>
>
>
> On Wed, 20 Apr 2016, Kieran Bingham wrote:
>
> > On 20/04/16 16:53, Julia Lawall wrote:
> > >
> > > On Wed, 20 Apr 2016, Kieran Bingham wrote:
> > >
> > >> Hi all,
> > >>
> > >> I am finding that a rule which matches a part of code, sometimes
causes
> > >> hunks which depend on it to act, and sometimes not to act
> > >>
> > >> My spatch is successfully removing and converting hunks that I desire
> > >> changed, however the requirement became apparent *to not process the
> > >> file at all* if the variable is used in the probe function, so I
added
> > >> in a dependency on probe_id_unused (established in my previous mail
thread).
> > >
> > > I'll check on the rest shortly, but if you really want to not process
the
> > > file at all, just look for the condition in which that is the case,
and
> > > then write a python rule that depends on it, and run Cocci.exit() (not
> > > sure about the exact syntax - writing an ocaml rule and putting
> > > Coccilib.exit() will also work).
> >
> >
> > Thanks Julia,
> >
> > The following worked for my needs:
> >
> > @ script:python depends on driver && !probe_id_unused @
> > @@
> > print("Probe function uses the ID parameter")
> > cocci.exit()
> >
> > However, interestingly - it's not useful if I specify a minimal set of
> > files on the command line:
> >
> > Using the command:
> > spatch --linux-spacing --sp-file patches/i2c-dt.cocci $A $B $C $D
> > HANDLING: sound/soc/codecs/wm8737.c drivers/staging/iio/light/isl29018.c
> > drivers/gpio/gpio-pca953x.c ./drivers/rtc/rtc-ds1307.c
> > Probe function uses the ID parameter
> > kbingham@CookieMonster:/opt/projects/linux/kbuild-bbb/sources/linux$
> >
> > appears to stop at the first file (which is expected to stop) without
> > processing the next files.
> >
> > However, simply using the '.' target - does appear to iterate through
> > the entire sources successfully, parsing files it can, and stopping on
> > files that it should stop on.
>
> Putting multiple files on the command line is not the right way to process
> multiple files. What that means is to process those files all at once.
> Normally Coccinelle processes only one file at a time, which means that
> when there is a call from a function in one file to a function in another,
> it won't find the definition of the called function. If you put multiple
> files on the command line, it will see them all at once. The downside
> though is that Coccinelle will ultimately run more slowly, because it will
> be using a lot more memory at a given time.
>
> If you have a directory where you want to process only some of the files,
> you can use the --file-groups option. could be eg
>
> path/foo.c
> path/bar.c
>
> path/xyz.c
>
> Then foo.c and bar.c will be processed at once, and xyz.c will be
> processed separately.
>
> But it could be easier to just work on the whole directory and let
> Coccinelle to fail to do anything with most of the files.
Ah I see. I bet that explains my other dependency issue as well. It was
probably matching state from other processed files in the same processing
group.
I had been specifying a group of files to make sure the right thing was
being done in the processing. In the whole tree about 250 files match so I
was trying to reduce the set to verify.
It looks like the act of me trying to reduce the set changed the processing
though ;-)
I guess I could create a single folder with the reduced set in, but it
looks like its doing the right thing now with the exit ()
Thanks
Kieran
>
> julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On 20 Apr 2016 17:46, "Julia Lawall" wrote:
>
>
>
> On Wed, 20 Apr 2016, Kieran Bingham wrote:
>
> > Hi all,
> >
> > I am finding that a rule which matches a part of code, sometimes causes
> > hunks which depend on it to act, and sometimes not to act
> >
> > My spatch is successfully removing and converting hunks that I desire
> > changed, however the requirement became apparent *to not process the
> > file at all* if the variable is used in the probe function, so I added
> > in a dependency on probe_id_unused (established in my previous mail
thread).
> >
> > Therefore, I would expect to be able to set the 'depends on' to be on
> > 'probe_id_unused' for each of the actions, and have actions only taken
> > if the full dependency chain (C4->C1 below) is met.
> >
> > However, I get a non-consistent application of this, where some hunks
> > operate when (I believe) they shouldn't:
> >
> > My full spatch for reference, is at:
> >
> > https://gist.github.com/kbingham/96477177dd20a72b1c2f
> >
> > In essence, it does the following {C}hecks:
> > C1 - of_dev_id_present : Check for a struct of_device_id
> > C2 - dev_id : Check for a struct i2c_device_id
> > C3 - driver : Check and identify the probefunc in the driver structure
> > C4 - probe_id_unused : Establish if the id is used in the probe function
> >
> > Where C4 depends on C3 depends on C2 depends on C1
> >
> > The aim is that if all of the above checks/identifiers are met, it will
> > take the following actions:
> > A rewrite the probe function declaration
> > B re-point the function pointer in the driver structure
> > C remove the i2c_device_id reference
> > D remove the i2c_device_id array
> > E remove the MODULE_DEVICE_TABLE macro
> >
> > For this example, I'll take three files from the kernel source, all of
> > which meet conditions C1 - > C3 (but only F3 meets C4):
> >
> > F1: drivers/gpio/gpio-pca953x.c : probe id used ( ! C4 )
> > F2: drivers/staging/iio/light/isl29018.c : probe id used ( ! C4 )
> > F3: sound/soc/codecs/wm8737.c : probe id UNUSED ( C4 )
> >
> > If all the actions (A->E) start with @ depends on driver @ (to depend on
> > C3) all the actions complete on these files. (Shown in sequence in the c
> > file)
> >
> > F1 : D E A B C
> > F2 : A D E B C
> > F3 : A D E B C
> >
> > However, if all the actions depend on @ depends on probe_id_unused @
> > (depends on C4), Some actions complete, and some do not!
> >
> > F1 : D E C (Unexpected behaviour - I expect F1 : )
> > F2 : D E C (Unexpected behaviour - I expect F2 : )
> > F3 : A D E B C (Expected behaviour)
> >
> > So of course, I want actions D E and C to *not* complete on F1 and F2,
> > but I can't understand why they do not comply with their 'depends'
> > chain. Am I looking at a bug in Coccinelle here or a bug with my
> > interpretation of the depends keyword?
> >
> > Sorry for the long mail, and look forward to any ideas!
>
> What version of Coccinelle do you have? I get the expected behavior. No
> change on F1 and F2, and a change on F3.
I think the fault was somewhere between the keyboard and chair :-)
I'm away from my keyboard now, so I can't check but I'll assume it was
because I was specifying the files in one command line.
Regards
Kieran
>
> julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Dependencies between spatch hunks
On Wed, 20 Apr 2016, Kieran Bingham wrote:
>
>
> On 20 Apr 2016 17:31, "Julia Lawall" wrote:
> >
> >
> >
> > On Wed, 20 Apr 2016, Kieran Bingham wrote:
> >
> > > On 20/04/16 16:53, Julia Lawall wrote:
> > > >
> > > > On Wed, 20 Apr 2016, Kieran Bingham wrote:
> > > >
> > > >> Hi all,
> > > >>
> > > >> I am finding that a rule which matches a part of code, sometimes
> causes
> > > >> hunks which depend on it to act, and sometimes not to act
> > > >>
> > > >> My spatch is successfully removing and converting hunks that I desire
> > > >> changed, however the requirement became apparent *to not process the
> > > >> file at all* if the variable is used in the probe function, so I
> added
> > > >> in a dependency on probe_id_unused (established in my previous mail
> thread).
> > > >
> > > > I'll check on the rest shortly, but if you really want to not process
> the
> > > > file at all, just look for the condition in which that is the case,
> and
> > > > then write a python rule that depends on it, and run Cocci.exit() (not
> > > > sure about the exact syntax - writing an ocaml rule and putting
> > > > Coccilib.exit() will also work).
> > >
> > >
> > > Thanks Julia,
> > >
> > > The following worked for my needs:
> > >
> > > @ script:python depends on driver && !probe_id_unused @
> > > @@
> > > print("Probe function uses the ID parameter")
> > > cocci.exit()
> > >
> > > However, interestingly - it's not useful if I specify a minimal set of
> > > files on the command line:
> > >
> > > Using the command:
> > > spatch --linux-spacing --sp-file patches/i2c-dt.cocci $A $B $C $D
> > > HANDLING: sound/soc/codecs/wm8737.c drivers/staging/iio/light/isl29018.c
> > > drivers/gpio/gpio-pca953x.c ./drivers/rtc/rtc-ds1307.c
> > > Probe function uses the ID parameter
> > > kbingham@CookieMonster:/opt/projects/linux/kbuild-bbb/sources/linux$
> > >
> > > appears to stop at the first file (which is expected to stop) without
> > > processing the next files.
> > >
> > > However, simply using the '.' target - does appear to iterate through
> > > the entire sources successfully, parsing files it can, and stopping on
> > > files that it should stop on.
> >
> > Putting multiple files on the command line is not the right way to process
> > multiple files. What that means is to process those files all at once.
> > Normally Coccinelle processes only one file at a time, which means that
> > when there is a call from a function in one file to a function in another,
> > it won't find the definition of the called function. If you put multiple
> > files on the command line, it will see them all at once. The downside
> > though is that Coccinelle will ultimately run more slowly, because it will
> > be using a lot more memory at a given time.
> >
> > If you have a directory where you want to process only some of the files,
> > you can use the --file-groups option. could be eg
> >
> > path/foo.c
> > path/bar.c
> >
> > path/xyz.c
> >
> > Then foo.c and bar.c will be processed at once, and xyz.c will be
> > processed separately.
> >
> > But it could be easier to just work on the whole directory and let
> > Coccinelle to fail to do anything with most of the files.
>
> Ah I see. I bet that explains my other dependency issue as well. It was
> probably matching state from other processed files in the same processing
> group.
>
> I had been specifying a group of files to make sure the right thing was
> being done in the processing. In the whole tree about 250 files match so I
> was trying to reduce the set to verify.
>
> It looks like the act of me trying to reduce the set changed the processing
> though ;-)
>
> I guess I could create a single folder with the reduced set in, but it looks
> like its doing the right thing now with the exit ()
You may find it useful to use glimpse or id-utils to index your files. In
coccinelle/scripts there are idutils_index.sh and glimpseindex_cocci.sh,
respectively. The you can use --use-glimpse or --use-idutils, and it will
go directly to the files that contain the important keywords of your
semantic patch. Not parsing the other files will be a big savings. The
downside of course is that you have to redo the index every time your code
base changes.
julia___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Splitting a function in two with SmPL
Is there a way to split a function in 2 with SmPL? For example, let's say I've created a helper routine which accepts a callback to be set, it will then use the callback. The code that goes into the callback will be the code you had below an old API call. For instance I'm writing a new firmware API for the kernel and I'd like to help developers to do conversion by supplying a transformation set of SmPL rules for them, so their work is minimized, and so the changes on the API can also more easily be understood. The firmware API has to types of calls, async and sync calls, for the async functionality I believe I have a good solution already given that a callback is always expected so we can re-use that. For sync mechanism the old firmware API never required one, but I'd like to do that now. I do this to help the API do more work for the users, to help avoid bugs. This borrows some ideas from the devm functionality, so for instance it does the free'ing of the firmware for you now. So for instance, this works really well, but it does not let me move a section of statements below the old API to a new routine. I take it, that this would probably be hard to do as Coccinelle would need to somehow figure out on its own all the required declarations it may need. Since a human can do it, I'm confident we can figure this out, however I'd like to know if this is a known limitation or if I can get this to work as-is already with the engine. Here's the rule I have so far: http://drvbp1.linux-foundation.org/~mcgrof/2016/04/20/convert-sysdata-sync-v1.cocci It works but that's because I haven't tried / figured out how to lift the statement S1 out from f() and into the new sync_found_cb() I'm adding. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Splitting a function in two with SmPL
On Wed, 20 Apr 2016, Luis R. Rodriguez wrote: > Is there a way to split a function in 2 with SmPL? For example, let's > say I've created a helper routine which accepts a callback to be set, > it will then use the callback. The code that goes into the callback > will be the code you had below an old API call. > > For instance I'm writing a new firmware API for the kernel and I'd > like to help developers to do conversion by supplying a transformation > set of SmPL rules for them, so their work is minimized, and so the > changes on the API can also more easily be understood. The firmware > API has to types of calls, async and sync calls, for the async > functionality I believe I have a good solution already given that a > callback is always expected so we can re-use that. For sync mechanism > the old firmware API never required one, but I'd like to do that now. > I do this to help the API do more work for the users, to help avoid > bugs. This borrows some ideas from the devm functionality, so for > instance it does the free'ing of the firmware for you now. > > So for instance, this works really well, but it does not let me move a > section of statements below the old API to a new routine. I take it, > that this would probably be hard to do as Coccinelle would need to > somehow figure out on its own all the required declarations it may > need. Since a human can do it, I'm confident we can figure this out, > however I'd like to know if this is a known limitation or if I can get > this to work as-is already with the engine. > > Here's the rule I have so far: > > http://drvbp1.linux-foundation.org/~mcgrof/2016/04/20/convert-sysdata-sync-v1.cocci > > It works but that's because I haven't tried / figured out how to lift > the statement S1 out from f() and into the new sync_found_cb() I'm > adding. Do you have some test data? julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Splitting a function in two with SmPL
On Wed, Apr 20, 2016 at 7:57 PM, Julia Lawall wrote: > On Wed, 20 Apr 2016, Luis R. Rodriguez wrote: > >> Is there a way to split a function in 2 with SmPL? For example, let's >> say I've created a helper routine which accepts a callback to be set, >> it will then use the callback. The code that goes into the callback >> will be the code you had below an old API call. >> >> For instance I'm writing a new firmware API for the kernel and I'd >> like to help developers to do conversion by supplying a transformation >> set of SmPL rules for them, so their work is minimized, and so the >> changes on the API can also more easily be understood. The firmware >> API has to types of calls, async and sync calls, for the async >> functionality I believe I have a good solution already given that a >> callback is always expected so we can re-use that. For sync mechanism >> the old firmware API never required one, but I'd like to do that now. >> I do this to help the API do more work for the users, to help avoid >> bugs. This borrows some ideas from the devm functionality, so for >> instance it does the free'ing of the firmware for you now. >> >> So for instance, this works really well, but it does not let me move a >> section of statements below the old API to a new routine. I take it, >> that this would probably be hard to do as Coccinelle would need to >> somehow figure out on its own all the required declarations it may >> need. Since a human can do it, I'm confident we can figure this out, >> however I'd like to know if this is a known limitation or if I can get >> this to work as-is already with the engine. >> >> Here's the rule I have so far: >> >> http://drvbp1.linux-foundation.org/~mcgrof/2016/04/20/convert-sysdata-sync-v1.cocci >> >> It works but that's because I haven't tried / figured out how to lift >> the statement S1 out from f() and into the new sync_found_cb() I'm >> adding. > > Do you have some test data? Sure, the target I'm using right now, as an example is: spatch --sp-file convert-sysdata-sync-v1.cocci --in-place --recursive-includes --relax-include-path --dir drivers/net/wireless/intersil/p54/ --jobs 4 --use-coccigrep That should get the engine going on: drivers/net/wireless/intersil/p54/p54spi.c Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Splitting a function in two with SmPL
I really doubt that the result of this is exactly what you want, but maybe
you can improve it.
This critically relies on the function f having a return value. If it
doesn't then you would need two versions, one with a return value and one
without. It also critically relies on the request_firmware call being at
top level, not under an if.
Whatever rule you end up with, it would be good to provide a rule at the
end that detects the case where the transformation should have applied, but
does not, and do something to warn the user of that fact.
julia
/*
* You can use this to convert a device driver from the old firmware API
* to the new flexible sysdata API for sync behaviour
*
* For now the confidence is low as this is a new SmPL patch, however with
* time, as more drivers get converted and this is fine tuned, this
* confidence in it may grow higher.
*
* Confidence: Low
*
* Copyright: (C) 2016 Luis R. Rodriguez GPLv2.
*/
@ find_request_fw_sync @
expression name, dev, E1, e;
type T;
identifier ret, fw_entry, f, drv;
@@
f (...) {
...
?T *drv = E1;
+const struct sysdata_file_desc sysdata_desc = {
+ SYSDATA_DEFAULT_SYNC(sync_found_cb, drv),
+};
...
(
-request_firmware(&drv->fw_entry, name, dev);
+sysdata_file_request(name, &sysdata_desc, dev);
|
ret =
-request_firmware(&drv->fw_entry, name, dev);
+sysdata_file_request(name, &sysdata_desc, dev);
)
<...
- return e;
+ RETURN(e);
...>
}
@@
expression name, dev, e;
identifier ret, fw_entry, find_request_fw_sync.f, drv;
@@
f (...) {
... when any
-RETURN(e);
+return e;
}
@ find_request_fw_sync1 @
expression name, dev, e;
identifier ret, fw_entry, find_request_fw_sync.f, drv;
@@
f (...) {
... when any
(
sysdata_file_request(name, &sysdata_desc, dev);
+{
...
+}
return e;
|
ret = sysdata_file_request(name, &sysdata_desc, dev);
+{
...
+}
return e;
)
}
@ find_request_fw_sync2 @
identifier find_request_fw_sync.f;
statement list S1;
symbol context, sysdata;
@@
+static int sync_found_cb(void *context, const struct sysdata_file *sysdata)
+{
+S1
+}
f (...) {
...
-{S1}
return ...;
}
@cleanup@
expression e;
@@
static int sync_found_cb(void *context, const struct sysdata_file *sysdata)
{
<...
- RETURN(e);
+ return e;
...>
}
@ change_headers depends on find_request_fw_sync @
@@
-#include
+#include
@ use_new_struct depends on find_request_fw_sync @
identifier fn, data;
@@
fn (...,
-const struct firmware *data
+const struct sysdata_file *data
,...) { ... }
@ drop_fw_release depends on find_request_fw_sync @
@@
-release_firmware(...);
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
