* [Cocci] how to find missing initializer
@ 2015-12-03 13:04 Peter Hurley
2015-12-03 18:46 ` Lars-Peter Clausen
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Peter Hurley @ 2015-12-03 13:04 UTC (permalink / raw)
To: cocci
Hi all,
I'm struggling with the grammar necessary to find struct definitions
missing an initializer.
The background is that many tty interfaces include a method/operations
table (named fields of function ptrs). For example, given a declaration
like,
struct tty_operations {
int (*install)( /* ... */ );
int (*remove)( /* ... */ );
void (*cleanup)( /* ... * );
...
};
a tty driver might define its method table like,
static const struct tty_operations ops = {
.install = uart_install,
.remove = uart_remove,
.cleanup = uart_cleanup,
...
};
(actually, this is a common pattern throughout the kernel)
Many operations are optional; a NULL method is simply not executed.
For example,
if (tty->ops->cleanup)
tty->ops->cleanup(tty);
So trying to find those in-tree drivers which _do not_ define a cleanup
method with coccinelle, led to this fragment which has a parse error.
Apologies if my question is obvious or trivial; I'm still learning
coccinelle.
Regards,
Peter Hurley
--- >% ---
virtual context
@ depends on context @
identifier fops;
identifier fn;
@@
* struct tty_operations fops = {
... when != .cleanup = fn,
...
};
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] how to find missing initializer
2015-12-03 13:04 [Cocci] how to find missing initializer Peter Hurley
@ 2015-12-03 18:46 ` Lars-Peter Clausen
2015-12-03 22:12 ` Julia Lawall
2015-12-04 11:54 ` Peter Hurley
2015-12-03 19:26 ` Luis R. Rodriguez
2015-12-04 8:55 ` SF Markus Elfring
2 siblings, 2 replies; 12+ messages in thread
From: Lars-Peter Clausen @ 2015-12-03 18:46 UTC (permalink / raw)
To: cocci
On 12/03/2015 02:04 PM, Peter Hurley wrote:
> Hi all,
>
> I'm struggling with the grammar necessary to find struct definitions
> missing an initializer.
>
> The background is that many tty interfaces include a method/operations
> table (named fields of function ptrs). For example, given a declaration
> like,
>
> struct tty_operations {
> int (*install)( /* ... */ );
> int (*remove)( /* ... */ );
> void (*cleanup)( /* ... * );
> ...
> };
>
> a tty driver might define its method table like,
>
> static const struct tty_operations ops = {
> .install = uart_install,
> .remove = uart_remove,
> .cleanup = uart_cleanup,
> ...
> };
>
>
> (actually, this is a common pattern throughout the kernel)
>
> Many operations are optional; a NULL method is simply not executed.
> For example,
>
> if (tty->ops->cleanup)
> tty->ops->cleanup(tty);
>
> So trying to find those in-tree drivers which _do not_ define a cleanup
> method with coccinelle, led to this fragment which has a parse error.
>
> Apologies if my question is obvious or trivial; I'm still learning
> coccinelle.
Usually you'd do something like first find all that have the initializer and
then match all that where not matched before using a position metavariable. E.g.
@r1@
identifier fops;
identifier fn;
position p;
@@
struct tty_operations fops at p = {
...,
.cleanup = fn,
...
};
@@
identifier fops;
identifier fn;
position p != r1.p;
@@
*struct tty_operations fops at p = {
...
};
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] how to find missing initializer
2015-12-03 18:46 ` Lars-Peter Clausen
@ 2015-12-03 22:12 ` Julia Lawall
2015-12-03 23:48 ` Luis R. Rodriguez
2015-12-04 11:31 ` [Cocci] how to find missing initializer SF Markus Elfring
2015-12-04 11:54 ` Peter Hurley
1 sibling, 2 replies; 12+ messages in thread
From: Julia Lawall @ 2015-12-03 22:12 UTC (permalink / raw)
To: cocci
> Usually you'd do something like first find all that have the initializer and
> then match all that where not matched before using a position metavariable. E.g.
>
> @r1@
> identifier fops;
> identifier fn;
> position p;
> @@
> struct tty_operations fops at p = {
> ...,
> .cleanup = fn,
> ...
> };
You don't need the ... Coccinelle is very relaxed about matching
structure initializers. What you provide should be a subset, in any
order, of what is present. The ... would only be needed if you want to
add a new field initialization, and to force it to the beginning or end
of the structure.
Luis, why did you use a regular expression for cleanup?
Thanks for the contributions.
julia
> @@
> identifier fops;
> identifier fn;
> position p != r1.p;
> @@
> *struct tty_operations fops at p = {
> ...
> };
>
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] how to find missing initializer
2015-12-03 22:12 ` Julia Lawall
@ 2015-12-03 23:48 ` Luis R. Rodriguez
2015-12-04 13:55 ` [Cocci] Searching with SmPL for missing clean-up settings in designated initialisers SF Markus Elfring
2015-12-04 11:31 ` [Cocci] how to find missing initializer SF Markus Elfring
1 sibling, 1 reply; 12+ messages in thread
From: Luis R. Rodriguez @ 2015-12-03 23:48 UTC (permalink / raw)
To: cocci
On Thu, Dec 03, 2015 at 11:12:56PM +0100, Julia Lawall wrote:
> > Usually you'd do something like first find all that have the initializer and
> > then match all that where not matched before using a position metavariable. E.g.
> >
> > @r1@
> > identifier fops;
> > identifier fn;
> > position p;
> > @@
> > struct tty_operations fops at p = {
> > ...,
> > .cleanup = fn,
> > ...
> > };
>
> You don't need the ... Coccinelle is very relaxed about matching
> structure initializers. What you provide should be a subset, in any
> order, of what is present. The ... would only be needed if you want to
> add a new field initialization, and to force it to the beginning or end
> of the structure.
>
> Luis, why did you use a regular expression for cleanup?
You're right no need:
virtual context
@ is_tty @
identifier fops;
@@
struct tty_operations fops = {
};
@ tty_has_cleanup depends on is_tty @
identifier is_tty.fops;
identifier fn;
@@
struct tty_operations fops = {
.cleanup = fn,
};
@ depends on is_tty && !tty_has_cleanup && context @
identifier is_tty.fops;
@@
* struct tty_operations fops = {
};
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] Searching with SmPL for missing clean-up settings in designated initialisers
2015-12-03 23:48 ` Luis R. Rodriguez
@ 2015-12-04 13:55 ` SF Markus Elfring
2015-12-04 14:44 ` Julia Lawall
0 siblings, 1 reply; 12+ messages in thread
From: SF Markus Elfring @ 2015-12-04 13:55 UTC (permalink / raw)
To: cocci
> virtual context
>
> @ is_tty @
> identifier fops;
> @@
> struct tty_operations fops = {
> };
>
> @ tty_has_cleanup depends on is_tty @
> identifier is_tty.fops;
> identifier fn;
> @@
> struct tty_operations fops = {
> .cleanup = fn,
> };
>
> @ depends on is_tty && !tty_has_cleanup && context @
> identifier is_tty.fops;
> @@
> * struct tty_operations fops = {
> };
How do you think about a SmPL approach like the following?
virtual show_context_diff
@contains_member_functions@
identifier var;
struct operations =~ "^(?x)
(?:
\w+_operations
|
(?:
# Alternation placeholder
)
)$";
@@
operations var = { ... };
@contains_cleanup_function
contains_member_functions@
expression assign;
identifier release =~ "^(?x)
(?:
cleanup
|
(?:
# Another alternation placeholder
)
)$",
contains_member_functions.var;
type contains_member_functions.operations;
@@
operations var = { ...,
.cleanup = (assign),
...
};
@show_member_functions
depends on show_context_diff
&& contains_member_functions
&& !contains_cleanup_function@
identifier contains_member_functions.var;
struct contains_member_functions.operations;
@@
*operations var = { ... };
Would this SmPL script variant need any further fine-tuning
(or other software extensions)?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] Searching with SmPL for missing clean-up settings in designated initialisers
2015-12-04 13:55 ` [Cocci] Searching with SmPL for missing clean-up settings in designated initialisers SF Markus Elfring
@ 2015-12-04 14:44 ` Julia Lawall
2015-12-04 15:42 ` SF Markus Elfring
0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2015-12-04 14:44 UTC (permalink / raw)
To: cocci
On Fri, 4 Dec 2015, SF Markus Elfring wrote:
> > virtual context
> >
> > @ is_tty @
> > identifier fops;
> > @@
> > struct tty_operations fops = {
> > };
> >
> > @ tty_has_cleanup depends on is_tty @
> > identifier is_tty.fops;
> > identifier fn;
> > @@
> > struct tty_operations fops = {
> > .cleanup = fn,
> > };
> >
> > @ depends on is_tty && !tty_has_cleanup && context @
> > identifier is_tty.fops;
> > @@
> > * struct tty_operations fops = {
> > };
>
> How do you think about a SmPL approach like the following?
>
>
> virtual show_context_diff
>
> @contains_member_functions@
> identifier var;
> struct operations =~ "^(?x)
> (?:
> \w+_operations
> |
> (?:
> # Alternation placeholder
> )
> )$";
> @@
> operations var = { ... };
>
> @contains_cleanup_function
> contains_member_functions@
> expression assign;
> identifier release =~ "^(?x)
> (?:
> cleanup
> |
> (?:
> # Another alternation placeholder
> )
> )$",
> contains_member_functions.var;
> type contains_member_functions.operations;
> @@
> operations var = { ...,
> .cleanup = (assign),
> ...
> };
>
> @show_member_functions
> depends on show_context_diff
> && contains_member_functions
> && !contains_cleanup_function@
> identifier contains_member_functions.var;
> struct contains_member_functions.operations;
> @@
> *operations var = { ... };
>
>
> Would this SmPL script variant need any further fine-tuning
> (or other software extensions)?
I have no idea what the above does. The previous proposed variants were
completely fine.
julia
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] Searching with SmPL for missing clean-up settings in designated initialisers
2015-12-04 14:44 ` Julia Lawall
@ 2015-12-04 15:42 ` SF Markus Elfring
0 siblings, 0 replies; 12+ messages in thread
From: SF Markus Elfring @ 2015-12-04 15:42 UTC (permalink / raw)
To: cocci
>> Would this SmPL script variant need any further fine-tuning
>> (or other software extensions)?
>
> I have no idea what the above does. The previous proposed variants were
> completely fine.
* I suggest in principle once more to try regular expression out
for SmPL constraints on data types and corresponding names of member functions.
* The desired function initialisation can be a bit generalised, can't it?
Is such an approach also useful for the requested use case?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] how to find missing initializer
2015-12-03 22:12 ` Julia Lawall
2015-12-03 23:48 ` Luis R. Rodriguez
@ 2015-12-04 11:31 ` SF Markus Elfring
2015-12-04 13:18 ` Julia Lawall
1 sibling, 1 reply; 12+ messages in thread
From: SF Markus Elfring @ 2015-12-04 11:31 UTC (permalink / raw)
To: cocci
>> @r1@
>> identifier fops;
>> identifier fn;
>> position p;
>> @@
>> struct tty_operations fops at p = {
>> ...,
>> .cleanup = fn,
>> ...
>> };
>
> You don't need the ...
I find it interesting that you suggest to omit the use of SmPL ellipsis here.
> Coccinelle is very relaxed about matching structure initializers.
Would you like to explain the handling of designated initialisers
with the semantic patch language a bit more?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] how to find missing initializer
2015-12-04 11:31 ` [Cocci] how to find missing initializer SF Markus Elfring
@ 2015-12-04 13:18 ` Julia Lawall
0 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2015-12-04 13:18 UTC (permalink / raw)
To: cocci
On Fri, 4 Dec 2015, SF Markus Elfring wrote:
> >> @r1@
> >> identifier fops;
> >> identifier fn;
> >> position p;
> >> @@
> >> struct tty_operations fops at p = {
> >> ...,
> >> .cleanup = fn,
> >> ...
> >> };
> >
> > You don't need the ...
>
> I find it interesting that you suggest to omit the use of SmPL ellipsis here.
>
>
> > Coccinelle is very relaxed about matching structure initializers.
>
> Would you like to explain the handling of designated initialisers
> with the semantic patch language a bit more?
I think I already did... What you provide is required to b a subset is
what is present in the code to be matched.
julia
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] how to find missing initializer
2015-12-03 18:46 ` Lars-Peter Clausen
2015-12-03 22:12 ` Julia Lawall
@ 2015-12-04 11:54 ` Peter Hurley
1 sibling, 0 replies; 12+ messages in thread
From: Peter Hurley @ 2015-12-04 11:54 UTC (permalink / raw)
To: cocci
On 12/03/2015 01:46 PM, Lars-Peter Clausen wrote:
> On 12/03/2015 02:04 PM, Peter Hurley wrote:
>> Hi all,
>>
>> I'm struggling with the grammar necessary to find struct definitions
>> missing an initializer.
>>
>> The background is that many tty interfaces include a method/operations
>> table (named fields of function ptrs). For example, given a declaration
>> like,
>>
>> struct tty_operations {
>> int (*install)( /* ... */ );
>> int (*remove)( /* ... */ );
>> void (*cleanup)( /* ... * );
>> ...
>> };
>>
>> a tty driver might define its method table like,
>>
>> static const struct tty_operations ops = {
>> .install = uart_install,
>> .remove = uart_remove,
>> .cleanup = uart_cleanup,
>> ...
>> };
>>
>>
>> (actually, this is a common pattern throughout the kernel)
>>
>> Many operations are optional; a NULL method is simply not executed.
>> For example,
>>
>> if (tty->ops->cleanup)
>> tty->ops->cleanup(tty);
>>
>> So trying to find those in-tree drivers which _do not_ define a cleanup
>> method with coccinelle, led to this fragment which has a parse error.
>>
>> Apologies if my question is obvious or trivial; I'm still learning
>> coccinelle.
>
> Usually you'd do something like first find all that have the initializer and
> then match all that where not matched before using a position metavariable. E.g.
>
> @r1@
> identifier fops;
> identifier fn;
> position p;
> @@
> struct tty_operations fops at p = {
> ...,
> .cleanup = fn,
> ...
> };
>
> @@
> identifier fops;
> identifier fn;
> position p != r1.p;
> @@
> *struct tty_operations fops at p = {
> ...
> };
>
I appreciate the quick response. Thanks all.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] how to find missing initializer
2015-12-03 13:04 [Cocci] how to find missing initializer Peter Hurley
2015-12-03 18:46 ` Lars-Peter Clausen
@ 2015-12-03 19:26 ` Luis R. Rodriguez
2015-12-04 8:55 ` SF Markus Elfring
2 siblings, 0 replies; 12+ messages in thread
From: Luis R. Rodriguez @ 2015-12-03 19:26 UTC (permalink / raw)
To: cocci
On Thu, Dec 03, 2015 at 08:04:03AM -0500, Peter Hurley wrote:
> Hi all,
>
> I'm struggling with the grammar necessary to find struct definitions
> missing an initializer.
>
> The background is that many tty interfaces include a method/operations
> table (named fields of function ptrs). For example, given a declaration
> like,
>
> struct tty_operations {
> int (*install)( /* ... */ );
> int (*remove)( /* ... */ );
> void (*cleanup)( /* ... * );
> ...
> };
>
> a tty driver might define its method table like,
>
> static const struct tty_operations ops = {
> .install = uart_install,
> .remove = uart_remove,
> .cleanup = uart_cleanup,
> ...
> };
>
>
> (actually, this is a common pattern throughout the kernel)
>
> Many operations are optional; a NULL method is simply not executed.
> For example,
>
> if (tty->ops->cleanup)
> tty->ops->cleanup(tty);
>
> So trying to find those in-tree drivers which _do not_ define a cleanup
> method with coccinelle, led to this fragment which has a parse error.
>
> Apologies if my question is obvious or trivial; I'm still learning
> coccinelle.
>
> Regards,
> Peter Hurley
>
> --- >% ---
> virtual context
>
> @ depends on context @
> identifier fops;
> identifier fn;
> @@
> * struct tty_operations fops = {
> ... when != .cleanup = fn,
> ...
> };
I think you need to use an identifier for the callback routine name but I could
be wrong. There's two ways I would do this, one with context as you originally
used, and the second with python. Results for missing tty driver without
cleanup below, and after that, the ones that do have a cleanup callback.
The way I like to think of these hunts is first doing a needle in the haystack
search, and then tooling around that.
drivers/tty/synclink.c:4305:35-43: ERROR: tty driver with missing cleanup callback line 4305.
drivers/tty/metag_da.c:561:35-46: ERROR: tty driver with missing cleanup callback line 561.
drivers/tty/serial/serial_core.c:2341:35-43: ERROR: tty driver with missing cleanup callback line 2341.
drivers/tty/synclinkmp.c:3890:35-38: ERROR: tty driver with missing cleanup callback line 3890.
drivers/tty/ipwireless/tty.c:562:35-42: ERROR: tty driver with missing cleanup callback line 562.
drivers/tty/serial/68328serial.c:1133:35-41: ERROR: tty driver with missing cleanup callback line 1133.
drivers/tty/serial/crisv10.c:4144:35-41: ERROR: tty driver with missing cleanup callback line 4144.
drivers/tty/amiserial.c:1633:35-45: ERROR: tty driver with missing cleanup callback line 1633.
drivers/tty/synclink_gt.c:3725:35-38: ERROR: tty driver with missing cleanup callback line 3725.
drivers/tty/vt/vt.c:3028:35-42: ERROR: tty driver with missing cleanup callback line 3028.
drivers/tty/goldfish.c:170:35-51: ERROR: tty driver with missing cleanup callback line 170.
drivers/tty/serial/ifx6x60.c:613:35-53: ERROR: tty driver with missing cleanup callback line 613.
drivers/tty/mips_ejtag_fdc.c:872:35-57: ERROR: tty driver with missing cleanup callback line 872.
drivers/tty/rocket.c:2347:35-45: ERROR: tty driver with missing cleanup callback line 2347.
drivers/tty/ehv_bytechan.c:592:35-45: ERROR: tty driver with missing cleanup callback line 592.
drivers/tty/mxser.c:2317:35-44: ERROR: tty driver with missing cleanup callback line 2317.
drivers/tty/isicom.c:1282:35-45: ERROR: tty driver with missing cleanup callback line 1282.
drivers/tty/hvc/hvsi.c:1039:35-43: ERROR: tty driver with missing cleanup callback line 1039.
drivers/tty/bfin_jtag_comm.c:213:35-46: ERROR: tty driver with missing cleanup callback line 213.
drivers/tty/cyclades.c:4035:35-41: ERROR: tty driver with missing cleanup callback line 4035.
drivers/tty/moxa.c:408:35-43: ERROR: tty driver with missing cleanup callback line 408.
A similar rule that catches the ones with a cleanup callback:
drivers/tty/nozomi.c:1867:35-42: ERROR: tty driver with cleanup callback line 1867.
drivers/tty/hvc/hvc_console.c:835:35-42: ERROR: tty driver with cleanup callback line 835.
drivers/tty/n_gsm.c:3183:35-45: ERROR: tty driver with cleanup callback line 3183.
drivers/tty/serial/kgdb_nmi.c:319:35-51: ERROR: tty driver with cleanup callback line 319.
drivers/tty/hvc/hvcs.c:1442:35-43: ERROR: tty driver with cleanup callback line 1442.
drivers/tty/pty.c:510:35-53: ERROR: tty driver with cleanup callback line 510.
drivers/tty/pty.c:684:35-49: ERROR: tty driver with cleanup callback line 684.
drivers/tty/pty.c:701:35-49: ERROR: tty driver with cleanup callback line 701.
drivers/tty/pty.c:525:35-52: ERROR: tty driver with cleanup callback line 525.
It'd be nice to just do:
virtual context
@ is_tty @
identifier fops;
@@
struct tty_operations fops = {
...
};
@ tty_has_cleanup depends on is_tty && context @
identifier is_tty.fops;
identifier fn;
identifier tty_op =~ "(cleanup)";
@@
* struct tty_operations fops = {
... when != .tty_op = fn,
};
But that does not seem to parse.
The context rule that worked:
virtual context
@ is_tty @
identifier fops;
@@
struct tty_operations fops = {
...
};
@ tty_has_cleanup depends on is_tty @
identifier is_tty.fops;
identifier fn;
identifier tty_op =~ "(cleanup)";
@@
struct tty_operations fops = {
.tty_op = fn,
};
@ depends on is_tty && !tty_has_cleanup && context @
identifier is_tty.fops;
@@
* struct tty_operations fops = {
...
};
----
then the one with python:
virtual report
virtual context
@ is_tty @
identifier fops;
position p1;
@@
struct tty_operations fops at p1 = {
...
};
@ tty_has_cleanup depends on is_tty @
identifier is_tty.fops;
identifier fn;
identifier tty_op =~ "(cleanup)";
@@
struct tty_operations fops = {
.tty_op = fn,
};
@script:python depends on is_tty && !tty_has_cleanup && report @
p1 << is_tty.p1;
@@
msg="ERROR: tty driver with missing cleanup callback line %s." % (p1[0].line)
coccilib.report.print_report(p1[0], msg)
--
Perhaps there are better ways.
Luis
^ permalink raw reply [flat|nested] 12+ messages in thread* [Cocci] how to find missing initializer
2015-12-03 13:04 [Cocci] how to find missing initializer Peter Hurley
2015-12-03 18:46 ` Lars-Peter Clausen
2015-12-03 19:26 ` Luis R. Rodriguez
@ 2015-12-04 8:55 ` SF Markus Elfring
2 siblings, 0 replies; 12+ messages in thread
From: SF Markus Elfring @ 2015-12-04 8:55 UTC (permalink / raw)
To: cocci
> So trying to find those in-tree drivers which _do not_ define a cleanup
> method with coccinelle, led to this fragment which has a parse error.
>
> Apologies if my question is obvious or trivial;
Your support request is fine. - You show just another interesting use case.
The preferred approach is mostly not so obvious for things you find
to be missing.
> I'm still learning coccinelle.
It is usual that it will take some time to become familiar to express
better solutions also with another programming language.
My knowledge is still evolving ?
There are more software development challenges to consider for
the semantic patch language around the handling of function pointers,
aren't there?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-04 15:42 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-03 13:04 [Cocci] how to find missing initializer Peter Hurley
2015-12-03 18:46 ` Lars-Peter Clausen
2015-12-03 22:12 ` Julia Lawall
2015-12-03 23:48 ` Luis R. Rodriguez
2015-12-04 13:55 ` [Cocci] Searching with SmPL for missing clean-up settings in designated initialisers SF Markus Elfring
2015-12-04 14:44 ` Julia Lawall
2015-12-04 15:42 ` SF Markus Elfring
2015-12-04 11:31 ` [Cocci] how to find missing initializer SF Markus Elfring
2015-12-04 13:18 ` Julia Lawall
2015-12-04 11:54 ` Peter Hurley
2015-12-03 19:26 ` Luis R. Rodriguez
2015-12-04 8:55 ` SF Markus Elfring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.