views:

55

answers:

1

I have a real-life example that I have in my project below. My aim is to pick out the most likely phone number for SMS receptions, and only that (phone)number. All works well when I don't release memory at the end, but we can't have that, can we. My question is: Where (and how) is the correct way to release memory in the example below?

// Called after a person has been selected by the user. Return YES if you want the person to be displayed.
// Return NO to do nothing (the delegate is responsible for dismissing the peoplepicker)
- (BOOL) peoplePickerNavigationController: (ABPeoplePickerNavigationController *) peoplePicker 
    shouldContinueAfterSelectingPerson: (ABRecordRef)person
{     
 // Retrieving the person's most likely phonenumber (kABPersonPhoneProperty)
 CFStringRef phoneNumber, phoneNumberLabel;

 ABMutableMultiValueRef multiValueReference = ABMultiValueCreateMutable(kABMultiStringPropertyType); 
 multiValueReference = ABRecordCopyValue(person, kABPersonPhoneProperty);

 NSMutableString *mostLikelyPhoneNumber = [[NSMutableString alloc] init];

 // Iterating through all the phone numbers in the list
 for (CFIndex i = 0; i < ABMultiValueGetCount(multiValueReference); i++) {

  phoneNumberLabel = ABMultiValueCopyLabelAtIndex(multiValueReference, i);
  phoneNumber      = ABMultiValueCopyValueAtIndex(multiValueReference, i);

  // Converting to NSString for easier comparison (this way I have only NSStrings)
  NSString *NSStringphoneNumberLabel =  [[NSString alloc] init];
  NSString *NSStringphoneNumber =  [[NSString alloc] init];

  // Copying the contents of the CFStringRef to my NSString pointers
  NSStringphoneNumberLabel = (NSString *) phoneNumberLabel;
  NSStringphoneNumber = (NSString *) phoneNumber;

  LOG (@"Phone number: %@/%@", phoneNumberLabel, phoneNumber); // No problem so far!
  LOG (@"Phone number: %@/%@", NSStringphoneNumberLabel, NSStringphoneNumber); // No problem so far!

  // If this phone number has a "iphone" or a "mobile" label, save it to mostLikelyPhoneNumber
  if ([NSStringphoneNumberLabel isEqualToString:@"_$!<Mobile>!$_"]) 
  {
   mostLikelyPhoneNumber = (NSMutableString *) NSStringphoneNumber;
  }

  else if ([NSStringphoneNumberLabel isEqualToString:@"iPhone"])
  {
   mostLikelyPhoneNumber = (NSMutableString *) NSStringphoneNumber;
   // However, if it was an "iphone", break out of the loop. (Can't get any better than iPhone)
   break; 
  }

  _tfGSM.text = (NSString*) mostLikelyPhoneNumber;

  // Releasing memory used in this particular iteration
  [NSStringphoneNumber release];
  [NSStringphoneNumberLabel release];  
 }

 // Releasing rest of memory  THIS IS WHERE IT CRASHES!
 [mostLikelyPhoneNumber release];
 CFRelease(phoneNumberLabel); 
 CFRelease(phoneNumber); 
 CFRelease(multiValueReference);

 [self dismissModalViewControllerAnimated:YES];
 return NO; // As soon as a person is picked, we end this address book sidetrip and return to the app
}
+1  A: 

Your basic problem seems to be not understanding what = means.

When you assign one pointer value to another, as in this randomly chosen example:

mostLikelyPhoneNumber = (NSMutableString*) NSStringphoneNumber;

you are not copying the contents of the second string into the mutable first string. Rather, you are overwriting the pointer itself. So the original pointer to the NSMutableString you allocated earlier is lost, and you now have only a second reference to the NSStringphoneNumber value, which you do not own.

You do pretty much the same thing everywhere in your code.

When you eventually hope to clean up, you can't, because you no longer have pointers to any of your allocated objects; and when you try, you instead over-release a bunch of things you don't own, leading to a crash.

Normally in these situations I'd suggest going and reading the memory management guide, but frankly I think you'd better go back to basics and do some remedial C first.

walkytalky
That bad, huh. Well, I appreciate your answer.
maralbjo
On the other hand, with a few adjustments I got it to work without crashing. Thanks for pointing me in the right direction.
maralbjo