What is the memory management problem with this bit of code?

Question

The XCode analyzer tells me there is a problem at line 4 — return [originalError copy]; — but I don't see it. Help me please?

- (NSError *)errorFromOriginalError:(NSError *)originalError error:(NSError *)secondError
{
    if (secondError == nil) {
        return [originalError copy];
    }
    // ...
}

The problem description is:

Potential leak of an object allocated on line 203
- Method returns an Objective-C object with a +1 retain count (owning reference)
- Object returned to caller as an owning reference (single retain count transferred to caller)
- Object allocated on line 203 is returned from a method whose name ('errorFromOriginalError:error:') does not contain 'copy' or otherwise starts with 'new' or 'alloc'. This violates the naming convention rules given in the Memory Management Guide for Cocoa (object leaked)
Potential null dereference. According to coding standards in 'Creating and Returning NSError Objects' the parameter 'error' may be null

The third issue seems to suggest I should either change the name or the behaviour of the method further. Any suggestions on that? The method is derived from the errorFromOriginalError:error: method described in Apple's Core Data Validation document. Its purpose is to combine originalError and secondError so that secondError is a sub-error of originalError.

My addition tries to ensure that the method still works if there is no actual secondError. Since a new error object is created if secondError is not nil, I wanted to recreate that in the case displayed above by simply copying the error object.

score 2 · Accepted Answer · answered Aug 16 '11 at 08:03

2

You are making a copy of originalError, but your function name implies that the returned object will be autoreleased. Try

return [[originalError copy] autorelease];

answered Aug 16 '11 at 08:03

jrturton

118,105
32
252
268

Thank you, that did indeed fix the issues, except for the last one (potential null dereference). How do I get rid of that one? I tried adding ` if (originalError == nil) return nil;` at the beginning of the method, but to no avail. – winsmith Aug 16 '11 at 08:32
See this answer: http://stackoverflow.com/questions/1189518/clang-error-on-potential-null-dereference/1189570#1189570 – jrturton Aug 16 '11 at 08:38
Merci. (I wonder if it's frowned upon to add "Thank you" comments on SO. I think it's polite, but what does the community think?) – winsmith Aug 16 '11 at 08:46
No idea, I'm new here myself :-) – jrturton Aug 16 '11 at 08:54

score 1 · Answer 2 · answered Aug 16 '11 at 08:02

1

[originalError copy] creates a new object with a retain count set to 1. It would then be the responsibility of the calling method to release that object. If you're doing this then it isn't necessarily a problem, but it's probably a better ideas to autorelease it.

ie

return [[originalError copy] autorelease];

answered Aug 16 '11 at 08:02

Tom Jefferys

13,090
2
35
36

Thank you for your answer; it is as correct as jrturton's. I hope you don't mind me marking his as correct, as he was slightly faster to answer. – winsmith Aug 16 '11 at 08:40

What is the memory management problem with this bit of code?

2 Answers2