PDA

View Full Version : Memory mgmt mistake in iPhone Development book?




CaptSaltyJack
May 1, 2009, 01:11 AM
I've got the Apress book on iPhone dev, and I noticed this bit of code (I've modified/cut it down a bit to keep it short/simple):


if (...some condition...)
msg = [[NSString alloc] initWithFormat:@"Blah %@", someObj];
else
msg = @"Blah blah";

...some code...
[msg release];


Isn't the unconditional [msg release] incorrect? If we just assigned it to @"Blah blah" then we haven't really allocated anything, so there's no need to release it, right? Maybe the code should've done msg = [[NSString alloc] initWithString:@"Blah blah"]; just to be safe?



dejo
May 1, 2009, 12:05 PM
Isn't the unconditional [msg release] incorrect? If we just assigned it to @"Blah blah" then we haven't really allocated anything, so there's no need to release it, right? Maybe the code should've done msg = [[NSString alloc] initWithString:@"Blah blah"]; just to be safe?
Yeah, when assigned to @"Blah blah", we haven't really allocated anything, but releasing it in that case does no harm. You can't overrelease something (i.e. nil ignores a release message). I believe the reason the code was written this way, as well as plenty of other bits of code in the book, was more to try to keep the code simple and not necessarily to demonstrate better coding practices. I think something to that effect is mentioned early on in the book.

admanimal
May 1, 2009, 12:28 PM
You can't overrelease something (i.e. nil ignores a release message).

You can over-release something. If you release something but do not set it to nil and then release it again, it is an error. So if you have this code:


NSString *msg = [[NSString alloc] initWithFormat:@"Whatever"];
[msg release];
[msg release];


msg is NOT nil after you call the first release. msg still points to the same memory it always has, only now that memory is no longer in use by the app, and calling release again is incorrect. I'm messing around to see if it will actually cause an exception/crash and it doesn't seem that it does, but it is still incorrect code.

In this case msg = @"blah blah" creates an autoreleased string so it is an error to call [msg release] in that case unless you have explicitly retained msg first. It might not cause the program to crash but it can cause undefined behavior.

CaptSaltyJack
May 1, 2009, 12:29 PM
Ahh, ok. Thanks for clarifying

kainjow
May 1, 2009, 12:50 PM
In this case msg = @"blah blah" creates an autoreleased string so it is an error to call [msg release] in that case unless you have explicitly retained msg first.

Technically, if you're simply assigning a constant string to a variable without using any other NSString method, the string is not autoreleased and never will deallocate.

For example, after these three release calls, str is still valid:
NSString *str = @"Hello World";
[str release];
[str release];
[str release];
NSLog(@"str: %@", str); // still works
You can confirm by looking at its retainCount, which will be INT_MAX.

admanimal
May 1, 2009, 12:53 PM
Technically, if you're simply assigning a constant string to a variable without using any other NSString method, the string is not autoreleased and never will deallocate.



Ahh I had no idea, I assumed they were autoreleased rather than just statically allocated. Well that changes everything then!

I still feel like the code from the book is bad form though.

kainjow
May 1, 2009, 02:51 PM
I still feel like the code from the book is bad form though.

I agree. Here's how I'd do it if msg was a local variable:
if (...some condition...)
msg = [NSString stringWithFormat:@"Blah %@", someObj];
else
msg = @"Blah blah";
If it was an ivar, just add [msg retain] afterwards.

CaptSaltyJack
May 1, 2009, 02:58 PM
Ahh right, since stringWithFormat is a convenience method, it autoreleases. Good thinking.

PS, what's an ivar?

kainjow
May 1, 2009, 03:01 PM
Instance variable:

@interface Foo : NSObject {
NSString *msg; // ivar
}
@end