[Cocci] __asm statements confuse spatch

[Timur Tabi]

I'm trying to modify a Windows .c file, and it contains several __asm
(or _asm or asm) statements that confuse spatch. They look like this:

_asm {mov ax, ss}
__asm mov uRetval,eax // Just keep 32 bits.
__asm {
PAUSE
PAUSE
}

And so on. Is there a way to get spatch to ignore these statements?

Another problem I've having with the source file is that it has
inconsistent usage of braces, and sometimes spatch wants to add
unnecessary braces that look off. For example, this:

if (...)
DBG_PRINTF((...));
else
DBG_PRINTF((...));
}

(the } belongs to some if-statement much earlier in code somewhere) becomes:

if (...) {
NV_PRINTF(...);
}
else {
NV_PRINTF(...);
}
}

I really don't want spatch to add the braces.

[Julia Lawall]

Post by Timur Tabi
I'm trying to modify a Windows .c file, and it contains several __asm
_asm {mov ax, ss}
__asm mov uRetval,eax // Just keep 32 bits.
__asm {
PAUSE
PAUSE
}
And so on. Is there a way to get spatch to ignore these statements?

Linux uses __asm__ ( ... ), which is what Coccinelle recognizes. I can
probably add _asm and __asm with the braces. On the other hand, the
second case, with no delimiter seems awkward. Does that occur a lot?
Basically it's not clear how to parse it. I could have __asm eat up
everything until the end of the line, but then the third case won't work.

Post by Timur Tabi
Another problem I've having with the source file is that it has
inconsistent usage of braces, and sometimes spatch wants to add
if (...)
DBG_PRINTF((...));
else
DBG_PRINTF((...));
}
if (...) {
NV_PRINTF(...);
}
else {
NV_PRINTF(...);
}
}
I really don't want spatch to add the braces.

I don't think this has anything to do with the trailing }. Coccinelle
knows which brace goes with what, independent of the indentation.
Something about your rule is making it unsure whether the changed code is
in a branch by itself, or whether you have added multiple statements.

For example, if your rule is

- A;
+ B;
+ C;

and the code is if (x) A;, then the braces are needed. Spatch is a bit
conservative about this, ie it adds brace unless it is clear that there is
a replacement of a single statement by another one.

You could try to track down the problem by making a minimal semantic
patch and C code that show the problem, or just add some rules to clean
up afterwards.

julia

Post by Timur Tabi
_______________________________________________
Cocci mailing list
https://systeme.lip6.fr/mailman/listinfo/cocci

[Timur Tabi]

Post by Julia Lawall
Linux uses __asm__ ( ... ), which is what Coccinelle recognizes. I can
probably add _asm and __asm with the braces. On the other hand, the
second case, with no delimiter seems awkward. Does that occur a lot?
Basically it's not clear how to parse it. I could have __asm eat up
everything until the end of the line, but then the third case won't work.

Well, it doesn't occur *a lot*, since it's only one set of files that
has this problem for me. I believe this code isn't compiled with gcc,
which is why the syntax is non-standard.

I don't know if it's worth updating spatch for it. For now, I just
manually comment-out the offending code in the C file and then run spatch.

Post by Julia Lawall

Post by Timur Tabi
Another problem I've having with the source file is that it has
inconsistent usage of braces, and sometimes spatch wants to add
if (...)
DBG_PRINTF((...));
else
DBG_PRINTF((...));
}
if (...) {
NV_PRINTF(...);
}
else {
NV_PRINTF(...);
}
}
I really don't want spatch to add the braces.

I don't think this has anything to do with the trailing }.

Sorry, I didn't mean to imply that it does. My point was that the
trailing } is in an awkward position already, and when spatch adds its
own brace, the result looks weird.

Post by Julia Lawall
Coccinelle
knows which brace goes with what, independent of the indentation.
Something about your rule is making it unsure whether the changed code is
in a branch by itself, or whether you have added multiple statements.
For example, if your rule is
- A;
+ B;
+ C;

Hmmm.... I run some tests with my script to see if anything stands out,
but the whole purpose of my script is to replace DBG_PRINTF with
NV_PRINTF. I never add a second line.

Post by Julia Lawall
and the code is if (x) A;, then the braces are needed. Spatch is a bit
conservative about this, ie it adds brace unless it is clear that there is
a replacement of a single statement by another one.
You could try to track down the problem by making a minimal semantic
patch and C code that show the problem, or just add some rules to clean
up afterwards.

What would a clean-up rule look like? Something like this?

-{
NV_PRINTF2(...)
-}

[Julia Lawall]

Post by Julia Lawall
Linux uses __asm__ ( ... ), which is what Coccinelle recognizes. I can
probably add _asm and __asm with the braces. On the other hand, the
second case, with no delimiter seems awkward. Does that occur a lot?
Basically it's not clear how to parse it. I could have __asm eat up
everything until the end of the line, but then the third case won't work.

Well, it doesn't occur *a lot*, since it's only one set of files that has this
problem for me. I believe this code isn't compiled with gcc, which is why the
syntax is non-standard.
I don't know if it's worth updating spatch for it. For now, I just manually
comment-out the offending code in the C file and then run spatch.

Post by Julia Lawall

Post by Timur Tabi
Another problem I've having with the source file is that it has
inconsistent usage of braces, and sometimes spatch wants to add
if (...)
DBG_PRINTF((...));
else
DBG_PRINTF((...));
}
if (...) {
NV_PRINTF(...);
}
else {
NV_PRINTF(...);
}
}
I really don't want spatch to add the braces.

I don't think this has anything to do with the trailing }.

Sorry, I didn't mean to imply that it does. My point was that the trailing }
is in an awkward position already, and when spatch adds its own brace, the
result looks weird.

Post by Julia Lawall
Coccinelle
knows which brace goes with what, independent of the indentation.
Something about your rule is making it unsure whether the changed code is
in a branch by itself, or whether you have added multiple statements.
For example, if your rule is
- A;
+ B;
+ C;

Hmmm.... I run some tests with my script to see if anything stands out, but
the whole purpose of my script is to replace DBG_PRINTF with NV_PRINTF. I
never add a second line.

Post by Julia Lawall
and the code is if (x) A;, then the braces are needed. Spatch is a bit
conservative about this, ie it adds brace unless it is clear that there is
a replacement of a single statement by another one.
You could try to track down the problem by making a minimal semantic
patch and C code that show the problem, or just add some rules to clean
up afterwards.

What would a clean-up rule look like? Something like this?
-{
NV_PRINTF2(...)
-}

That is missing a ; but with that it should be OK. In the Linux kernel,
if one branch has {} the other should too, so if you want to respect that
rule, then you would need various cases for various configurations of if.

julia