views:

113

answers:

2

The static analyzer is showing up a leak in this block of code (specifically the link with the copy in it):

- (void)parser:(NSXMLParser *)parser didEndElement:(NSString *)elementName namespaceURI:(NSString *)namespaceURI 
 qualifiedName:(NSString *)qName
{
    if ([elementName isEqualToString:@"item"]) 
    {
        [elements setObject:title forKey:@"title"];
        [elements setObject:date forKey:@"date"];
        [elements setObject:summary forKey:@"summary"];
        [elements setObject:link forKey:@"link"];

        [posts addObject:[elements copy]];
    }
}

I tried releasing the copied object but I still get the warning. Am I missing something?

Thanks

+5  A: 

You created new copy which you don't release.

This returns new elements object with ref count 1 which you are responsible to deallocate since you've just created a copy:

[elements copy];

In this line you add the new created copy to posts which looks like collection. All collections retain new values, so you pass your new copy with ref count 1 and posts increase the ref count to 2 by retaining it.

[posts addObject:[elements copy]];

On release posts will send to each element release which will decrement ref count to 1 so elements does not get deallocated and you end up with memory leak.

Remove the copy and see if that helps:

[post addObject:elements];
stefanB
Perfect thanks.
Kieran
+3  A: 

Just to make one point perfectly clear:

I tried releasing the copied object but I still get the warning.

You mean you tried this?

    [posts addObject:[elements copy]];
    [elements release];

That doesn't solve the problem, and it may cause a second problem.

The problem you started with is that you are leaking the copy—the object that the copy method returned, the object that you added to posts. That problem remains: You have not released the copy.

The problem you've added is that you are releasing the original object, which may not be what you intend; I can't say for sure without seeing your other code. You need to make sure not to release elements until you no longer need it.

You may want to send elements a removeAllObjects message instead; this will keep the object around, but empty it out in preparation for the next element.

On an unrelated note, you may also want to rename elements to more accurately reflect its dictionary nature and to abstract it away from its XML representation. “elements” sounds like an array to me. “feedItemProperties” might be more appropriate.

Peter Hosey
Thanks for the feedback. I see what I was doing wrong now. Thanks.
Kieran