On Mon, 16 Jun 2014, Travis Friesen wrote: > Hi Julia > > What's that? A bug in our legacy code base!? Preposterous! It was written in > 1994 by someone who no longer works here, and hasn't manifested any erroneous > behaviour to date! > > Good catch, joking aside; removing the semicolon allows Coccinelle to proceed > without having the change the macro first. It also catches the cast in the > macro and makes the appropriate change. > > Here is the script as it stands. Pretty basic so far, but it seems to catch > everything we throw at it. I should probably add cases for when the signed > keyword is explicitly used, but there don't seem to be any cases of that in > our code.
Looks good! julia > @@ > typedef float32_t; > @@ > > -float > +float32_t > > > > @@ > typedef float64_t; > @@ > > -double > +float64_t > > > > //(signed) char -> int8_t intentionally omitted. Chars tend to be unsigned by > default, > //and it makes little sense to treat them as integers when they are holding > characters > @@ > typedef uint8_t; > @@ > > -unsigned char > +uint8_t > > > > @@ > typedef int16_t; > @@ > > -short > +int16_t > > > > @@ > typedef uint16_t; > @@ > > -unsigned short > +uint16_t > > > > @@ > typedef uint32_t; > @@ > > -unsigned int > +uint32_t > > @@ > @@ > > -unsigned long > +uint32_t > > @@ > @@ > > -unsigned > +uint32_t > > > > @@ > typedef int32_t; > @@ > > -long > +int32_t > > @@ > @@ > -int > +int32_t > > > > FWIW, I release it into the public domain with no warrantee or guarantees of > any kind. > > Thanks again for your help > Travis > > > -----Original Message----- > From: Julia Lawall [mailto:[email protected]] > Sent: June-16-14 2:19 PM > To: Travis Friesen > Cc: [email protected] > Subject: RE: [Cocci] Type change errors > > On Mon, 16 Jun 2014, Travis Friesen wrote: > > > Hi Julia > > > > Thanks for your (extremely rapid) response. I'm trying your suggestion > > now, but it's led me to two minor issues. > > > > First, Coccinnelle seems to get confused by "(short unsigned)" casts > > (as opposed to the more typical "unsigned short" usage). It throws a > > "strange type1, maybe because of weird order: short unsigned")" error > > when I use the following rule: > > > > @@ > > typedef uint16_t; > > @@ > > > > -unsigned short > > +uint16_t > > > > My initial reaction is to hunt down the developer here who originally > > decided to write casts as "(short unsigned)" and break his legs, but > > thankfully it only occurs a handful of times in our code base, so it > > was easier to just manually fix each instance than threaten someone > > with bodily harm. I am going to check if this is considered valid C at > > all, but gcc does seem to accept it. > > I'm a little surprised that Coccinelle complains about this when it is not in > the generated code. But putting them around in the right way seems like a > good option for the future of the code base. > > > The second has to do with a macro expansion. The rule > > > > @@ > > typedef int32_t; > > @@ > > > > ( > > -int > > +int32_t > > | > > -long > > +int32_t > > ) > > > > Throws a " Fatal error: exception > > Failure("/home/trav/src/autopilot/Sim/Drone.c: 109: try to delete an > > expanded token: int")" on the following macro: > > > > #define round(x) (int)(((x) >= 0) ? (floor((x) + 0.5)) : (-floor(-(x) > > + 0.5))); > > > > Again, such occurrences are few and far between, and can be fixed > > manually. At least, in this file, they are few and far between. In > > case another file has more such occurrences, how would I solve this issue? > > The problem is not the definition of the macro, but rather with its uses. > The code is really pretty strange. The expansion of the macro is written as > an expression, but it has a semicolon at the end. So I guess it is supposed > to be used as something like: > > x = round(a) > > rather than > > x = round(a); > > This doesn't seem like a very good design. Eg if (x < round(a)) ... > wouldn't compile at all. Would it be an option to remove the semicolon in > the macro definition, and add semicolons at the macro uses? > > It might also be possible to add #define round(x) x; to a macro definition > file that is given with the argument --macro-file-builtins. Then, hopefully, > it will replace (in the AST) the uses of the macro by something that doesn't > contain a type. > > > Also, as a NB (and I think this is mentioned elsewhere) but the use of > > certain macros like UNUSED() would cause coccinelle to skip full > > functions. By adding > > > > #define UNUSED(x) (x) > > > > To standard.h, my problems seemed to evaporate. > > Yes. I can't anticipate all of the strange macros that may be used by all > software. It can be a good idea to first run spatch --parse-c dirname. > That will end by telling you the 10 tokens that it most commonly had trouble > parsing. Usually if you give definitions for these, or for some token near > these, then you will be able to parse most of your code, or at least most of > the code that matters for what you are trying to do. > > julia > > > > > > > > Thanks again for your help. Once I have this cocci script completely > > squared away, I'll submit it back to the list, to save other people > > future work :) > > > > Travis > > > > -----Original Message----- > > From: Julia Lawall [mailto:[email protected]] > > Sent: June-16-14 11:10 AM > > To: Travis Friesen > > Cc: [email protected] > > Subject: Re: [Cocci] Type change errors > > > > > @ rule1 @ > > > > > > identifier i1; > > > > > > @@ > > > > > > > > > > > > ( > > > > > > - unsigned int i1; > > > > > > + uint32_t i1; > > > > This is not a good idea, because it is overspecified. All you want to do > > is change the type. You don't need to specify that the type has to be in a > > variable declaration. Here is what I tried: > > > > @@ > > typedef uint_32; > > @@ > > > > -unsigned int > > +uint_32 > > > > On the code: > > > > int main(unsigned int x) { > > unsigned int y,z; > > return (unsigned int)(y+z); > > } > > > > You may have problems with variable declarations that include multiple > > variable. I have been working on that quite a bit recently, and I don't > > remember what is the status of that in the current release. Maybe there > > will be a new release soon. > > > > > Catch the bulk of our 32-bit integer usage in declarative > > > statements, but it’s obviously going to miss a lot of stuff, > > > including function definitions and prototypes. My attempts to use > > > more sophisticated rules, such as > > > > > > > > > > > > @ rule25 @ > > > > > > identifier i2; > > > > > > @@ > > > > > > > > > > > > unsigned > > > > > > ( > > > > > > - long i2; > > > > > > | > > > > > > - int i2; > > > > > > ) > > > > > > + uint32_t i2; > > > > You can't do this. You can't put a + on the outside of a disjunction. > > The + code has to be repeated in both branches. > > > > > Or > > > > > > > > > > > > @ rule4 @ > > > > > > identifier fn; > > > > > > identifier i2; > > > > > > @@ > > > > > > > > > > > > fn(... > > > > > > - int i2 > > > > > > + int32_t i2 > > > > > > ...) > > > > Two problems here. First, the elements of a parameter list need to be > > separated by commas. This includes ... Second, a pattern has to be a > > complete term. The outer part of this looks like a function call (function > > name, open and close parentheses, etc), but then the types mean that it > > can't be that. To match a function definition, it needs a body, ie { ... > > }. Normally, if you change the header of a definition, it will change any > > prototypes that it finds at the same time. But if the prototype is in a > > header file, and it doesn't find the header file when it is working on the > > C file, the prototype won't get transformed. > > > > But anyway, if you just make a rule on types, and not on contexts of types, > > the issue becomes irrelevant. > > > > > Result in a mysterious (and unhelpful) “minus: parse error...” Even > > > trying to omit the semicolons does not work. The very basic: > > > > Normally when you get a parse error it should tell you the line number and > > offset in the line of the problem. But it may take some getting used to > > the grammar to know what is being pointed at. > > > > > @ rule3 @ > > > > > > identifier i2; > > > > > > @@ > > > > > > > > > > > > - int i2 > > > > > > + int32_t i2 > > > > > > > > > > > > > > > > > > Also throws a “minus parse error...”. But from what I understand, > > > the semicolon should not be a necessary component to Coccinelle syntax. > > > > The semicolon is necessary. The semicolon is not necessary on eg > > > > x = 12 > > > > because in C x = 12 is an expression. x = 12; is a statement that consists > > of a single expression, which is x = 12. This is why you can write eg if > > (x = foo()) ... But int i2 is not a complete anything. It needs a > > semicolon. > > > > julia > > >
_______________________________________________ Cocci mailing list [email protected] https://systeme.lip6.fr/mailman/listinfo/cocci
