CODEWITHSUNDEEP
iphoneobjective-c
Harry Northover
Harry Northover

Reputation: 572

Objective-C NSString malloc error

I've got a method that creates a string then appends additional strings onto it:

-(NSString*)returnDetails {
    NSString *details = [[NSString alloc] init];

    details = [details stringByAppendingString:url];
    details = [details stringByAppendingString:@" : "];
    details = [details stringByAppendingString:author];

    return [details autorelease]; 
}

And I'm getting this error:

iphoneapp_1(66508,0xacd9e2c0) malloc: *** error for object 0x6b9eb80: pointer being freed was not allocated

If I change it to 

NSString *details = [NSString string];

and remove the autorelease call, then it works. I would just like to understand why this works and my original method didn't?

Upvotes: 2

Views: 511

Answers (5)

Neelam Verma
Neelam Verma

Reputation: 3274

//This line

     NSString *details = [[NSString alloc] init];
    // In this you are allocating memory for the NSSTring object detail.

    in the very next lines:

    details = [details stringByAppendingString:url];
    details = [details stringByAppendingString:@" : "];
    details = [details stringByAppendingString:author];

    // Here you are making same object as autoreleased. That means you have make this instance free to release.

    Now in next line:

    return [details autorelease];

    // here you are autoreleasing the object which was already autoreleased. So it got crashed.

what you do is what @delannoyk user has advised:

{
 NSMutableString *details = [[NSMutableString alloc] init];

  [details appendString:url];
  [details appendString:@" : "];
  [details appendString:author];

  return [details autorelease]; 
}

Upvotes: 1

jacekmigacz
jacekmigacz

Reputation: 789

First of all you are allocating and initializing NSString object and override that address later (mem-leak).

NSString *details = [[NSString alloc] init];

Later by calling stringByAppendingString you are getting already "autoreleased" object. Explicit autorelease is not redundant. It's double release.

Upvotes: 1

JeremyP
JeremyP

Reputation: 86651

stringByAppendingString: gives you a string you do not own. You must not release or autorelease it.

Your original, empty string leaks by the way. You could just use:

NSString *details = @"";

details = [details stringByAppendingString:url];
details = [details stringByAppendingString:@" : "];
details = [details stringByAppendingString:author];

return details.

or

NSString details = [NSString stringWithFormat: @"%@ : %@", url, author];
return details;

Upvotes: 1

user971401
user971401

Reputation:

The NSString's method stringByAppendingString: returns an object that is already autoreleased. So returning [details autorelease] from your method will make that object will be released one more time than it must. Just return details instead.

You also have a memory leak in there, because you never released the string you allocated at the top of the method. Try this code : 

-(NSString*)returnDetails {
    NSString *details = [NSString string];
    details = [details stringByAppendingString:url];
    details = [details stringByAppendingString:@" : "];
    details = [details stringByAppendingString:author];

    return details;

}

Upvotes: 3

delannoyk
delannoyk

Reputation: 1224

When you call

details = [details stringByAppendingString:NSString*];

you create a new object that is already marked as autorelease. To achieve what you want, you could simply do:

-(NSString*)returnDetails {
  NSMutableString *details = [[NSMutableString alloc] init];

  [details appendString:url];
  [details appendString:@" : "];
  [details appendString:author];

  return [details autorelease]; 
}

Upvotes: 3

Related Questions