[Cocci] convert if (x) stmt to if (x) {stmt}

[ron minnich]

we've found another one of these
if (x)
y
z

things in firmware that were intended to be
if (x) {
y
z
}

we're kind of tired of this and want to blanket require {} for all ifs,
even one liners.

We want to convert the entire code base such that all if (E) S becomes if
(E) {S} but of course we don't want to add extra {}.

I don't totally trust my rusty spatch-foo to get this right and was
wondering if there's already such a thing out there.

ron

[Julia Lawall]

we've found another one of theseif (x)
y
z
things in firmware that were intended to be
if (x) {
y
z
}
we're kind of tired of this and want to blanket require {} for all ifs, even
one liners.
We want to convert the entire code base such that all if (E) S becomes if
(E) {S} but of course we don't want to add extra {}. 
I don't totally trust my rusty spatch-foo to get this right and was
wondering if there's already such a thing out there.

No, I don't know of such a thing. However
linux/scripts/coccinelle/ifcol.cocci checks for an if header followed by
two statements preceded by the same number of whitespace characters.

J'ai fait un essaie pour ce que tu demande. Pour l'instant, ca tourne...

julia

ron

[Julia Lawall]

Post by Julia Lawall

we've found another one of theseif (x)
y
z
things in firmware that were intended to be
if (x) {
y
z
}
we're kind of tired of this and want to blanket require {} for all ifs, even
one liners.
We want to convert the entire code base such that all if (E) S becomes if
(E) {S} but of course we don't want to add extra {}. 
I don't totally trust my rusty spatch-foo to get this right and was
wondering if there's already such a thing out there.

No, I don't know of such a thing. However
linux/scripts/coccinelle/ifcol.cocci checks for an if header followed by
two statements preceded by the same number of whitespace characters.
J'ai fait un essaie pour ce que tu demande. Pour l'instant, ca tourne...

Sorry, I don't know where the answer in French came from...

Anyway, here is the rule I made:

@r disable braces1,braces2,braces3,braces4,neg_if @
position p;
statement S;
@@

***@p (...) { ... } else S

@disable braces1,braces2,braces3,braces4,neg_if @
position p != r.p;
statement S,S1;
@@

***@p (...)
+ {
S1
+ }
else S

@s disable braces1,braces2,braces3,braces4,neg_if @
position p;
statement S;
@@

***@p (...) S else { ... }

@disable braces1,braces2,braces3,braces4,neg_if @
position p != s.p;
statement S,S1;
@@

***@p (...) S else
+ {
S1
+ }

I would suggest to proceed carefully...

julia

[Julia Lawall]

Post by Julia Lawall

Post by Julia Lawall

we've found another one of theseif (x)
y
z
things in firmware that were intended to be
if (x) {
y
z
}
we're kind of tired of this and want to blanket require {} for all ifs, even
one liners.
We want to convert the entire code base such that all if (E) S becomes if
(E) {S} but of course we don't want to add extra {}. 
I don't totally trust my rusty spatch-foo to get this right and was
wondering if there's already such a thing out there.

No, I don't know of such a thing. However
linux/scripts/coccinelle/ifcol.cocci checks for an if header followed by
two statements preceded by the same number of whitespace characters.
J'ai fait un essaie pour ce que tu demande. Pour l'instant, ca tourne...

Sorry, I don't know where the answer in French came from...
@r disable braces1,braces2,braces3,braces4,neg_if @
position p;
statement S;
@@
@disable braces1,braces2,braces3,braces4,neg_if @
position p != r.p;
statement S,S1;
@@
+ {
S1
+ }
else S
@s disable braces1,braces2,braces3,braces4,neg_if @
position p;
statement S;
@@
@disable braces1,braces2,braces3,braces4,neg_if @
position p != s.p;
statement S,S1;
@@
+ {
S1
+ }
I would suggest to proceed carefully...

In each rule that contains S1, you first could replace the S1 by e; where
e is an expression metavariable. That would get a lot of simple cases out
of the way. Then you could commit that, and then run the original rule,
thus being able to focus on the more complex results.

julia

[Julia Lawall]

I understood one suggestion in the way for a SmPL specification
like the following.
if (...)
+{
S;
+e;
+}

The rule has no goal of adding some unknown e;.

How does this feedback fit to your wording “In each rule that contains S1,
you first could replace the S1 by e; 
”?

I didn't mean to write a semantic patch to remove S1 and add e;. I meant
to put e; instead of S1 in the rule, producing the rule below.

julia

if (...)
+ {
e;
+ }
else S
I don't see why one should consider whether e; should have been written in
a different way in making this transformation.

It seems that you present another variant for a SmPL script.
I assume that there are further constraints to consider for the discussed
change pattern.
Regards,
Markus