Do Not autorelease the Return from -copyWithZone:
This afternoon I was working on my latest version of the CryptoQuip solver and decided to turn on the built-in Clang Static Analyzer for all my debug builds as I've gotten to the point that I'm really getting close to having something useful to look at. Interestingly enough, it pointed out a problem on all my implementations of -copyWithZone: - I was returning an autoreleased instance and that's not the intended signature of the -copyWithZone: method.
For example, my original code for a simple object was:
/*! This is the standard copy method for the Legend so that we can make clean copies without having to worry about all the details. It's a nice deep copy, where the contents of the returned Legend are the same as this instance's. */ - (id) copyWithZone:(NSZone*)zone { // simply use the allocWithZone and populate it properly - easy return [[[Legend allocWithZone:zone] initWithMap:[self getMap]] autorelease]; }
and the Static Analyzer pointed out that the retain count of +0 was wrong and it was supposed to be +1. I simply changed it to read:
/*! This is the standard copy method for the Legend so that we can make clean copies without having to worry about all the details. It's a nice deep copy, where the contents of the returned Legend are the same as this instance's. */ - (id) copyWithZone:(NSZone*)zone { // simply use the allocWithZone and populate it properly - easy return [[Legend allocWithZone:zone] initWithMap:[self getMap]]; }
and it's all fixed. But when I'm dealing with a little more complex objects, and I'm making a deep copy, I have to be careful and autorelease the copies I'm making because the setters are calling retain on the arguments, and I don't want to have a leak by leaving a copy with a retain count of +2.
So a more complex version of -copyWithZone: looks like:
/*! This is the standard copy method for the Quip so that we can make clean copies without having to worry about all the details. It's a nice deep copy, where the contents of the returned Quip are the same as this instance's. */ - (id) copyWithZone:(NSZone*)zone { // simply use the allocWithZone and populate it properly - easy id dup = [[Quip allocWithZone:zone] init]; // now let's add in copies of all the ivars we have to make it equal [dup setCypherText:[[[self getCypherText] copyWithZone:zone] autorelease]]; [dup setStartingLegend:[[[self getStartingLegend] copyWithZone:zone] autorelease]]; for (PuzzlePiece* pp in [self getPuzzlePieces]) { [[dup getPuzzlePieces] addObject:[[pp copyWithZone:zone] autorelease]]; } return dup; }
where the arguments to the setters are calls to the instance's -copyWithZone: but to get the retain count correct, we need to autorelease them leaving them with a count of +0, to then have retain called on them by the setter.
The NSMutableArray calls retain on all objects that are added to it, and release when they are removed. That explains the call to autorelease on the copy of the individual elements in that deep copy loop.
In general, the Static Analyzer is saving my bacon by avoiding improper code practices as I get back into ObjC coding. I'm really happy with it. Fantastic!