On Mon, 31 Jan 2011, SF Markus Elfring wrote: > > Is @function_name@ the name of the function whose return value is ignored > > elsewhere, > > No. - It specifies a scope for the desired source code analysis. > > You registered a feature request about the handling of variable function names > yesterday, didn't you? ;-)
Yes, in changes.html. > > or is it the name of a function in whose body you want to find > > ignored return values. > > Yes. - That is my intention. OK, but it seems a bit strange. If you want to only work inside a specific function, you should put the pattern you want to match in the body of that function. Probably using <... ...> since you don't care how many times it matches, if at all. > > The following finds a call whose result is never used until the end of the > > function or until reaching a reassignment. (? marks something that is > > optional). > > > > @@ > > identifier x; > > expression fn,E; > > @@ > > > > x = fn(...); > > + ??? > > ... when != x > > ?x = E > > > > You can put whatever you want in the ??? line. > > I know that. But the source code adjustment can not always be directly > performed after a found function call. Several conditions must be > fulfilled to choose an appropriate patch. Sure. If you want to think about the change in some other rule, just save the position of this call that has successfully matched. Then in another rule, you can use a position variable to find that call again. > It might be that a part of my difficulties is that I do not know so far > if pattern grouping is supported as an alternative way by SmPL in > comparison to my previous suggestion of patch callbacks/triggers. > > > > But what you would want to put there depends on the return value of fn. > > Maybe. - A query for the function return type would be only useful if I > would try to construct a variable assignment or object initialisation as > a potential repair. Otherwise: I would need more introspection > capabilities to take special function properties into account. Example: > Was the affected function implementation called from within a signal > handler call hierarchy? Does my "fix" need to be "async-signal-safe"? I don't think the function type will tell you the whole story even without these considerations. For example, if the return type is int, then 0 might indicate failure, or some negative constant might indicate failure, or both (cf OpenSSL), depending on the coding conventions. If you need to know the context in which a function is called, you need to write a pattern to match that. If the information is interprocedural, you need to follow the iteration.cocci example. But these things take time, and there may be false positives, eg when multiple functions have the same name. So there comes a point where you would be better off to do this checking by hand. Coccinelle does not have the goal of making everything fully automatic in 100% of the cases, only to help you focus on where changes may need to be done. > > If you want to see how to collect information about certain kinds of return > > values, > > then that is presented in the "advanced SmPL" slides on this page: > > http://coccinelle.lip6.fr/papers.php and in demos/iteration.cocci. > > I have read your examples that result in a few final Python print > instructions which displays informations from SmPL position variables. In the example, my only goal was to print a report. But you can write a SmPL rule that uses the same position information to make some transformation on the code in that position, if you like. Or it may be possible to put the change to the code in the bug finding rule itself. > > Otherwise, maybe the programmer should just think about it himself. > > I would appreciate if a general adjustment rule will become possible. > > Would it be easier to express an use case like "A Reusable Aspect for > Memory Allocation Checking" as a SmPL demonstration for a start in the > requested direction? > http://research.msrg.utoronto.ca/ACC/Tutorial#A_Reusable_Aspect_for_Memory_All The subtle point with Coccinelle is that the code is being transformed, so you don't want to add this error handling code if it is already there. In the AOP solution, they don't care (as long as they don't care about code size or runtime overhead, depending on how the aspect weaving is implemented). A semantic patch that takes care of the case of a function that may return ERR_PTR(...) is shown below. At the moment, I'm not completely sure why the rule em is necessary... This is fairly well tested, but is probably missing some NULL pointer checking cases. julia @err@ identifier fn; position p; type T; @@ T * fn@p(...) { <+... return ERR_PTR(...); ...+> } @e exists@ expression x; identifier y,f,g,err.fn; position errret,err.p,bad; constant char [] c; @@ fn@p(...) { ... when any x@errret = \(kmalloc\|kcalloc\|kzalloc\|kmem_cache_alloc\|kmem_cache_zalloc\)(...); ... ( ((x) == NULL || ...) | ((x) != NULL || ...) | ((x) == 0 || ...) | ((x) != 0 || ...) | ASSERT(x) | BUG_ON(x == NULL) | g(...,c,...,x,...) | x@bad->y | kfree(x) | f(...,x@bad,...) ) ... when any } @em exists@ position e.errret,e.bad; expression x; @@ x@errret ... x@bad @depends on em@ position e.errret; expression x; identifier alloc; @@ x@errret = alloc(...); + if (!x) return ERR_PTR(-ENOMEM); _______________________________________________ Cocci mailing list [email protected] http://lists.diku.dk/mailman/listinfo/cocci (Web access from inside DIKUs LAN only)
