Tuesday, January 31, 2012

Clang Source Annotations

Recently I was faced with a dilemma. I was writing code I needed, and then I triggered a Clang Static Analyzer issue. Here is the code a category on NSColor, see if you can figure out whats wrong with it from a Static Analyzer perspective...

-(CGColorRef)cw_cgColor { 
NSColor *nscolor = [self colorUsingColorSpaceName:NSCalibratedRGBColorSpace];
CGFloat components[4];
[nscolor getRed:&components[0] green:&components[1] blue:&components[2] alpha:&components[3]];
CGColorSpaceRef space = CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB);
CGColorRef cgColor = CGColorCreate(space, components);
return cgColor;

While all the code here is perfectly valid, and converts a NSColor to a CGColorRef just fine, the Clang Static Analyzer will trigger an issue on the last line. Why? Because of this

CGColorRef cgColor = CGColorCreate(space, components);

We've created a Core Graphics or Core Foundation object with a create method which means it needs to be freed later on. However we are just returning that object in this method so the compiler has no way to really know that we will free this object later on. If only there was a way to let the Clang Static Analyzer know that whoever calls this needs to free the object and therefore this code isn't leaking.  Well there is, its called Source Annotations. All I did was add CF_RETURNS_RETAINED to the end of the method declaration in the header file and the Clang SA issue went away. So now the declaration looks like

#import <AppKit/AppKit.h>
@interface NSColor (CWNSColorAdditions)
-(CGColorRef)cw_cgColor CF_RETURNS_RETAINED;

This says to the Clang Static Analyzer whoever calls this method will be receiving a Core Graphics object thats retained and therefore the caller needs to free it when its done. I could have also put 'create' or 'copy' in the name of the method, but I wanted it to be nearly identical in name (remember always prefix your names of methods where your extending Apple classes, cw_ is mine in this case) to the name of the CGColor property on UIColor. This is not the only annotation, but if you haven't seen Clang Annotations, you should see whats available for when you run into issues trying to convey to the Clang Static Analyzer what your code is doing.

Clang Static Annalyzer Source Notations http://clang-analyzer.llvm.org/annotations.html The above code is from my Zangetsu Framework



missive said...

This is actually bad practice; you're abusing that annotation. Clang is trying to tell you something. Everything returned from an ObjC method not named init, copy, or new should be autoreleased, including CF objects. Just cast it to id and invoke autorelease on it.

Anonymous said...

@missive: I'm curious as to how your code would look then, as calling [foo autorelease] is forbidden under ARC.

Anonymous said...

From clang docs

Methods in the alloc, copy, mutableCopy and new families implicitly return a retained object as if they were annotated with the ns_returns_retained attribute.

A selector is in a certain selector family if, ignoring leading underscores, the first component of the selector either consists entirely of the name of the method family or it begins with that name followed by a character other than a lowercase letter.

I.e. copyCW_color {}

Mike Weller said...

If he were using ARC he would not see this warning at all. In non-ARC you should auto release rather than using the annotation since that doesn't place extra burden on the caller and stays consistent with the naming guidelines.

Colin Wheeler said...

Just to clarify, all my code now uses ARC. The Clang SA triggered this issue under ARC, even though we don't 'retain' anything in ARC now really, the Clang SA still uses the retained wording which here we really just mean, this was malloc'd essentially and needs to be freed at some point and since this is a Core Foundation style object you need to do it yourself.