CODEWITHSUNDEEP
iphonememory-managementmemory-leaks
jagse
jagse

Reputation: 333

iPhone memory leak pointer

another memory management question: I have asked this before, but did not really get an answer:

The question is would the following result in a leak or is it ok?

NSArray *txtArray = [NSArray array];

NSString *aTxtFieldTxt = [[NSString alloc]initWithString:aTxtField.text];
aTxtFieldTxt = [aTxtFieldTxt stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]];


NSMutableString *aTxt = [[NSMutableString alloc]initWithString:aTxtFieldTxt];
[aTxtFieldTxt release];


txtArray = [aTxt componentsSeparatedByString:@" "];

aTxt = [[txtArray objectAtIndex:0] retain];


for(int i = 1; i < [txtArray count]; i++){

   [aTxt appendString:@"+"];
   [aTxt appendString:[[txtArray objectAtIndex:i]retain]];
}

This is part of a function. And I am not sure if the assignment of aTxt = [[txtArray objectAtIndex:0] retain]; causes a leak because it is a pointer which originally points to 

NSMutableString *aTxt = [[NSMutableString alloc]initWithString:aTxtFieldTxt];
[aTxtFieldTxt release];

How do I do this correctly. Would I have to use another pointer? Can somebody please explain this issue?

Thanks alot!

Upvotes: 0

Views: 611

Answers (4)

Steven Noyes
Steven Noyes

Reputation: 1549

Lots and lots of issues with this code. 

//
// Don't do this.  Just declare the txtArray
//
NSArray *txtArray /* = [NSArray array]*/;

//
// You need to auto release after init in this case.
//
NSString *aTxtFieldTxt = [[[NSString alloc]initWithString:[aTxtField text]] autorelease]; 

//
// You are reassigning the aTxtFieldTxt and the new value is returned autoreleased.
//
aTxtFieldTxt = [aTxtFieldTxt stringByTrimmingCharactersInSet:[NSCharacterSet  whitespaceAndNewlineCharacterSet]];

//
// Again, autorelease after init
//
NSMutableString *aTxt = [[[NSMutableString alloc]initWithString:aTxtFieldTxt] autorelease]; 

//
// You never alloced this instance so it needs no release.
//
 /*[aTxtFieldTxt release]; */

//
// This array is returned autoreleased
//
txtArray = [aTxt componentsSeparatedByString:@" "];

//
// No need to retain here.  Just get the object
//
aTxt = /*[*/[txtArray objectAtIndex:0]/* retain]*/;

for(int i = 1; i < [txtArray count]; i++)
{
    [aTxt appendString:@"+"]; 
    [aTxt appendString:[[txtArray objectAtIndex:i]retain]]; 
}

I have found, that if you have retains/releases outside of accessors/base int/dealloc routines, you are doing something wrong. For every alloc you must have a balanced release/retain for that instance of the object. If you reassign the variable, you will loose your reference to it.

This is a quick stab on how I would write this code:

NSArray         *txtArray;
NSString        *aTxtFieldTxt = [NSString stringWithString:[aTxtField text]];
NSMutableString *aTxt;

aTxtFieldTxt = [aTxtFieldTxt stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]];
aTxt         = [NSMutableString stringWithString:aTxtFieldTxt]; 
txtArray     = [aTxt componentsSeparatedByString:@" "];

aTxt = [NSMutableString stringWithString:[txtArray objectAtIndex:0]];
for(int i = 1; i < [txtArray count]; i++)
{
    [aTxt appendString:@"+"]; 
    [aTxt appendString:[[txtArray objectAtIndex:i]retain]]; 
}

Upvotes: 1

Eiko
Eiko

Reputation: 25632

As others pointed out, this code leaks all over the place.

A real good way to find those without even running your code is the static analyzer!

Use Build and Analyze from the Build menu, and it will show you what leaks and why, with complete code paths.

Upvotes: 0

kennytm
kennytm

Reputation: 523274

aTxtFieldTxt = [aTxtFieldTxt stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]];

This will cause the original reference to aTxtFieldTxt being lost (thus a leak). Even worse, a convenient function will return an object of retain count 0, so the -release later will crash the program.

It's better written as

// don't stuff everything into a single line.
NSCharacterSet* charsToTrim = [NSCharacterSet whitespaceAndNewlineCharacterSet];
// there's no need to create a copy of the string.
NSString* aTxtFieldTxt = [aTxtField.text stringByTrimmingCharactersInSet:charsToTrim];

// don't release stuff with retain count 0.

// you just want to replace all ' ' by '+' right? there's a method for that.
NSString* aTxt = [aTxtFieldTxt stringByReplacingOccurrencesOfString:@" "
                                                         withString:@"+"];

Edit: If you need to replace multiple spaces to one +, you need to parse the string, e.g.

NSCharacterSet* charsToTrim = [NSCharacterSet whitespaceAndNewlineCharacterSet];
NSString* aTxtFieldTxt = [aTxtField.text stringByTrimmingCharactersInSet:charsToTrim];

NSScanner* scanner = [NSScanner scannerWithString:aTxtFieldTxt];
[scanner setCharactersToBeSkipped:nil];
NSMutableString* aTxt = [NSMutableString string];
NSCharacterSet* whitespaceSet = [NSCharacterSet whitespaceCharacterSet];
while (![scanner isAtEnd]) {
   NSString* res;
   [scanner scanUpToCharactersFromSet:whitespaceSet intoString:&res];
   [aTxt appendString:res];
   if ([scanner scanCharactersFromSet:whitespaceSet intoString:NULL])
      [aTxt appendString:@"+"];
}

Upvotes: 0

krb
krb

Reputation: 16315

Try running your application with Leaks. See if it causes a leak. Leaks is a tool in Instruments.

Upvotes: 1

Related Questions