On Tue, 23 Dec 2014, Julia Lawall wrote:
> On Tue, 23 Dec 2014, Nicholas Mc Guire wrote:
>
> >
> > Hi !
> >
> > writing some cocci file to detect some completion related
> > issues - for the function cases it works fine. If its correct
> > I'm not sure. What the first one should be doing is
> > find any siutation where a completion is reinitialized
> > with init_completion rather than reinit_completion.
> > so find the first init_completion() and take the position
> > (rule c) then check if the completion object was
> > used or passed to a function before the next init_completion
> > Q: do I need to handle more than those 4 cases to catch all ?
> >
> > The second one should find sequential init_completion() of the
> > same struct completion without that they are used in between
> > so basically the inverse case of the first - I'm not sure if
> > its worth the trouble though - in 3.18.0 there are 2 cases
> > found and both were correct findings
> >
> > The third scanner was to search for DECLARE_COMPLETION used
> > in functions for declearing struct completion on automatic variables
> > and transform them to DECLARE_COMPLETION_ONSTACK. Simple problem
> > it is not working... - obviously Im overlooking something - it
> > will just run through and report nothin.
> >
> > Any hint would be appreciated.
> >
> > One more procedural question - the patch-set generated should be
> > posted here+lkml for a first review or should it go out to all
> > the subsystem lists whose code is affected in CC as well ?
>
> Patches should always go to the people indicated by the Linux script
> get_maintainers.pl, ie maintainers and mailing lists for the specific
> subsystems.
>
> Once you are confident of the semantic patch, and think it may be useful
> for others, then it can go to the lkml and the maintainers of
> scripts/coccinelle, ie basically the list, a few people, and Michel Marek,
> who does the actual integration. For making the final version of the
> semantic patch, you can use the tool tools/sgen in the Coccinelle source
> tree, which makes the variants of the semantic patch that are supported by
> the Linux kernel.
ok - then I'll send it out as individual patches and post the
coccifiles if the patches are confirmed.
>
> > thx!
> > hofrat
> >
> > first working case:
> > ===================
> >
> > @c@
> > expression cmp;
> > position p;
> > @@
> >
> > init_completion@p(cmp)
> >
> > @d@
> > expression E,c.cmp;
> > identifier f;
> > position c.p,p1;
> > @@
> >
> > init_completion@p(cmp)
> > ...
> > (
> > E = cmp
> > |
> > E = &cmp
> > |
> > f(..., cmp,...)
> > |
> > f(..., &cmp,...)
> > )
> > ...
>
> Perhaps you want <+... ...+> here, if these assignments or function calls
> could occur more than once?
fixed.
>
> > - init_completion@p1(cmp)
> > + reinit_completion1(cmp)
> >
> >
> > 2nd working case:
> > =================
> >
> > @c@
> > expression cmp;
> > position p;
> > @@
> >
> > init_completion@p(cmp)
> >
> > @d@
> > expression E,c.cmp;
> > identifier f;
> > position c.p,p1;
> > @@
> >
> > init_completion@p(cmp)
> > ... when != E = cmp
> > when != E = &cmp
> > when != f(..., cmp,...)
> > when != f(..., &cmp,...)
> > - init_completion@p1(cmp);
>
> OK. If you allow there to be none of these calls in between, then you
> could merge these two rules, and change <+... ...+> to <... ...>. But I
> am not really sure what is the goal of these constraints at all. Why not
> just:
>
> init_completion@p(cmp)
> ...
> init_completion@p1(cmp)
>
because that would match cases like
init_completion(cmp)
wait_for_completion(cmp)
init_completion(cmp)
and that is not a double init but a reinit
So the intent of the construct is to catch
cases like:
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -8672,7 +8672,6 @@ static int qla4xxx_probe_adapter(struct
init_completion(&ha->disable_acb_comp);
init_completion(&ha->idc_comp);
init_completion(&ha->link_up_comp);
- init_completion(&ha->disable_acb_comp);
where nothing was done with the completion object between the
calls - so this should not be changed to a reinit_completion but dropped.
> >
> > the not-working case:
> > =====================
> > @e@
> > expression cmp;
> > identifier f;
> > position p;
> > @@
> >
> > f(...) {
> > ...
> > - DECLARE_COMPLETION@p(cmp);
> > + DECLARE_COMPLETION_ONSTACK(cmp);
> > ...
> > }
>
> You need to add
>
> declarer name DECLARE_COMPLETION;
>
> among the metavariables.
>
tried it as you suggested - but that fusses:
hofrat@debian:/tmp/linux-next$ make coccicheck
COCCI=false_declare_completion.cocci MODE=patch
Fatal error: exception Failure("meta: parse error:
= File "false_declare_completion.cocci", line 15, column 0, charpos = 295
around = 'declare', whole content = declare name DECLARE_COMPLETION;
")
spatch version is
hofrat@debian:/tmp/linux-next$ spatch --version
spatch version 1.0.0-rc21 with Python support and with Str regexp support
The complete cocci script is below
/* check for incorrect DECLARE_COMPLETION use within a function
*
* Options: --no-includes --include-headers
*/
virtual context
virtual patch
virtual org
virtual report
/* flag incorrect initializer*/
@e depends on patch && !(context || org || report)@
expression cmp;
identifier f;
declare name DECLARE_COMPLETION;
position p;
@@
f(...) {
...
- DECLARE_COMPLETION@p(cmp);
+ DECLARE_COMPLETION_ONSTACK(cmp);
...
}
@ep depends on !patch && (context || org || report)@
expression cmp;
identifier f;
declare name DECLARE_COMPLETION;
position p;
@@
f(...) {
...
* DECLARE_COMPLETION@p(cmp);
...
}
@script:python depends on org@
p << ep.p;
@@
msg="WARNING: possible incorrect use of DECLARE_COMPLETION"
coccilib.org.print_todo(p[0], msg)
@script:python depends on report@
p << ep.p;
@@
msg="WARNING: possible incorrect use of DECLARE_COMPLETION"
coccilib.report.print_report(p[0], msg)
_______________________________________________
Cocci mailing list
[email protected]
https://systeme.lip6.fr/mailman/listinfo/cocci
