Memory mgmt mistake in iPhone Development book?

Discussion in 'iOS Programming' started by CaptSaltyJack, Apr 30, 2009.

  1. macrumors 6502

    Joined:
    Jun 28, 2007
    #1
    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):

    Code:
    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:mad:"Blah blah"]; just to be safe?
     
  2. Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #2
    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.
     
  3. macrumors 68040

    Joined:
    Apr 22, 2005
    #3
    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:

    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.
     
  4. thread starter macrumors 6502

    Joined:
    Jun 28, 2007
  5. Moderator emeritus

    kainjow

    Joined:
    Jun 15, 2000
    #5
    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:
    Code:
    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.
     
  6. macrumors 68040

    Joined:
    Apr 22, 2005
    #6
    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.
     
  7. Moderator emeritus

    kainjow

    Joined:
    Jun 15, 2000
    #7
    I agree. Here's how I'd do it if msg was a local variable:
    Code:
    if (...some condition...)
      msg = [NSString stringWithFormat:@"Blah %@", someObj];
    else
      msg = @"Blah blah";
    If it was an ivar, just add [msg retain] afterwards.
     
  8. thread starter macrumors 6502

    Joined:
    Jun 28, 2007
    #8
    Ahh right, since stringWithFormat is a convenience method, it autoreleases. Good thinking.

    PS, what's an ivar?
     
  9. Moderator emeritus

    kainjow

    Joined:
    Jun 15, 2000
    #9
    Instance variable:

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

Share This Page