* [Cocci] __asm statements confuse spatch @ 2018-10-16 22:49 Timur Tabi 2018-10-17 5:25 ` Julia Lawall 0 siblings, 1 reply; 4+ messages in thread From: Timur Tabi @ 2018-10-16 22:49 UTC (permalink / raw) To: cocci 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cocci] __asm statements confuse spatch 2018-10-16 22:49 [Cocci] __asm statements confuse spatch Timur Tabi @ 2018-10-17 5:25 ` Julia Lawall 2018-10-17 12:15 ` Timur Tabi 0 siblings, 1 reply; 4+ messages in thread From: Julia Lawall @ 2018-10-17 5:25 UTC (permalink / raw) To: cocci On Tue, 16 Oct 2018, Timur Tabi wrote: > 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? 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. > > 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. 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 > _______________________________________________ > Cocci mailing list > Cocci at systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cocci] __asm statements confuse spatch 2018-10-17 5:25 ` Julia Lawall @ 2018-10-17 12:15 ` Timur Tabi 2018-10-17 12:26 ` Julia Lawall 0 siblings, 1 reply; 4+ messages in thread From: Timur Tabi @ 2018-10-17 12:15 UTC (permalink / raw) To: cocci On 10/17/18 12:25 AM, Julia Lawall wrote: > 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. >> 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. > 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. > 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. > 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(...) -} ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cocci] __asm statements confuse spatch 2018-10-17 12:15 ` Timur Tabi @ 2018-10-17 12:26 ` Julia Lawall 0 siblings, 0 replies; 4+ messages in thread From: Julia Lawall @ 2018-10-17 12:26 UTC (permalink / raw) To: cocci On Wed, 17 Oct 2018, Timur Tabi wrote: > On 10/17/18 12:25 AM, Julia Lawall wrote: > > 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. > > > > 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. > > > 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. > > > 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. > > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-17 12:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-16 22:49 [Cocci] __asm statements confuse spatch Timur Tabi 2018-10-17 5:25 ` Julia Lawall 2018-10-17 12:15 ` Timur Tabi 2018-10-17 12:26 ` Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).