How can I fix this clang warning: "Object with +0 retain counts returned to caller where +1 (owning) retain count is expected"?

Question

I have a piece of Objective-C code that looks like the following:

- (NSString *)copyData:(NSData *)data
{
    NSString *path = [[[self outputDirectory] stringByAppendingPathComponent:@"archive"] stringByAppendingPathExtension:@"zip"];
    NSLog(@"Copying data to %@", path);
    [data writeToFile:path atomically:NO];
    return path;
}

The code is called from an initializer that looks like this:

- (id)initWithData:(NSData *)data
{
    if ((self = [super init]) != nil) {
        NSString *path = [self copyData:data];        // Line 41 (referenced in warning, shown below)
        return [self initWithContentsOfFile:path];
    }
    return self;
}

When running the clang static analyzer, I get the following warnings for the path variable:

Potential leak of an object allocated on line 41 and stored into 'path'

Object with +0 retain counts returned to caller where +1 (owning) retain count is expected

I'm confused. My understanding is that stringByAppendingPathComponent should return an autoreleased string, so it should have a net retain count of 0. (Obviously I don't want to retain it.)

I've tried altering copyData: to return the following, but it didn't get rid of the warning:

return [[path retain] autorelease];

So what's the deal with this warning?

Answer 1

I suspect it's just noticing a method with the prefix copy and flagging that as something that should return something that the caller owns, because it thinks it's following Cocoa naming conventions.

In your case, of course, you are referring to files and whatnot, so it's an ignorable warning. If you change the name of your method, to something like saveData: instead, I bet the warning will go away.

Answer 2

Since the method has the name copy in it, the analyzer is expecting the returned object to have a +1 retain count, according to the Memory Management Guide.

Answer 3

No, that is incorrect; unless the method contains "alloc", "copy", "new", or one of the other keywords that implies the object will be owned by the invoker, the method returns an autoreleased or otherwise managed object, so stringByAppendingPathComponent is returning an autoreleased string.

On top of that, your method "copyData", contains the word "copy", implying that the result should be owned (and released) by the caller. However, the result you returned has been autoreleased, hence the error message that it is giving you. If you want to fix the error, don't autorelease. That is:

 return [path retain]

Of course that implies that the callers of your function need to release it. Alternatively, you can change the name of your function so that it complies with the memory management guidelines.

The name "copyData", IMHO, is unintuitive anyway. I would suggest you rename your function to "pathToSavedDataWithData" or the like. Something that says what it actually is doing.

Answer 4

Also, for times where you really do want to name a method with 'copy' or something because regardless of Cocoa memory management guidelines, copy is the best name for the method, you can annotate the method declairation with NS_RETURNS_NOT_RETAINED and then Clang won't give you a warning. So:

// Copies data from data to string; does not follow the copy rule
- (NSString*)copyData:(NSData*)data NS_RETURNS_NOT_RETAINED;