NSDictionary-MutableDeepCopy circling around issues

[X-RaY]

So here's the thing... I'm using the 'NSDictionary-MutableDeepCopy' extension-class for a while now.
But I noticed a memory management warning in the Static Clang Analyzer lately...
the normal implementation:

#import "NSDictionary-MutableDeepCopy.h"
@implementation NSDictionary(MutableDeepCopy)

- (NSMutableDictionary*)mutableDeepCopy{ //Original one
	NSMutableDictionary *returnDict = [[NSMutableDictionary alloc] initWithCapacity:[self count]];
	NSArray *keys = [self allKeys];
	
	for(id key in keys){
		id oneValue = [self valueForKey:key];
		id oneCopy = nil;
		
		if([oneValue respondsToSelector:@selector(mutableDeepCopy)])
			oneCopy = [oneValue mutableDeepCopy];
		else if([oneValue respondsToSelector:@selector(mutableCopy)])
			oneCopy = [oneValue mutableCopy];

		if(oneCopy == nil)
			oneCopy = [oneValue copy];
		[returnDict setValue:oneCopy forKey:key];
		
		[oneCopy release];
	}
	return [returnDict autorelease];
}
@end

The issue is at [oneCopy release]:
"Incorrect decrement of the reference count of an object that is not owned at this point by the caller"

But when I remove that line, the following issues pop up:
- "Potential leak of an object allocated on line 39 and stored into 'oneCopy'"
- "Potential leak of an object allocated on line 42 and stored into 'oneCopy'"

Line 39 and 42:
39: oneCopy = [oneValue mutableCopy];
42: oneCopy = [oneValue copy];

So i guess neither is ok.
Putting in the [oneCopy release] results in decrementing an reference count of an object not owned by me.
But removing them gives me 2 potential leaks....

Ow, btw: I call the method like this:
self.names = [self.allNames mutableDeepCopy];

I'd highly appreciate it if anyone could enlighten me about this problem...

Regards,
Me

[Duncan C]

Originally Posted by X-RaY

So here's the thing... I'm using the 'NSDictionary-MutableDeepCopy' extension-class for a while now.
But I noticed a memory management warning in the Static Clang Analyzer lately...
the normal implementation:

#import "NSDictionary-MutableDeepCopy.h"
@implementation NSDictionary(MutableDeepCopy)

- (NSMutableDictionary*)mutableDeepCopy{ //Original one
	NSMutableDictionary *returnDict = [[NSMutableDictionary alloc] initWithCapacity:[self count]];
	NSArray *keys = [self allKeys];
	
	for(id key in keys){
		id oneValue = [self valueForKey:key];
		id oneCopy = nil;
		
		if([oneValue respondsToSelector:@selector(mutableDeepCopy)])
			oneCopy = [oneValue mutableDeepCopy];
		else if([oneValue respondsToSelector:@selector(mutableCopy)])
			oneCopy = [oneValue mutableCopy];

		if(oneCopy == nil)
			oneCopy = [oneValue copy];
		[returnDict setValue:oneCopy forKey:key];
		
		[oneCopy release];
	}
	return [returnDict autorelease];
}
@end

The issue is at [oneCopy release]:
"Incorrect decrement of the reference count of an object that is not owned at this point by the caller"

But when I remove that line, the following issues pop up:
- "Potential leak of an object allocated on line 39 and stored into 'oneCopy'"
- "Potential leak of an object allocated on line 42 and stored into 'oneCopy'"

Line 39 and 42:
39: oneCopy = [oneValue mutableCopy];
42: oneCopy = [oneValue copy];

So i guess neither is ok.
Putting in the [oneCopy release] results in decrementing an reference count of an object not owned by me.
But removing them gives me 2 potential leaks....

Ow, btw: I call the method like this:
self.names = [self.allNames mutableDeepCopy];

I'd highly appreciate it if anyone could enlighten me about this problem...

Regards,
Me

Sounds like the analizer is wrong in this case. Copy methods Are supposed to return an object with a retain count of 1. Thus, when you call a copy method, you own the result.

It looks to me like oneCopy is a temporary object, so you should release it when you're done with it. BTW, mutableDeepCopy is a copy method, so it should return an object with a retain count of 1. The autorelease at the end does not belong there.

[X-RaY]

Ok
Thanks for the quick reply, really helpful!
Seems I've to bother about 1 issue less

Yet, why does the analyzer say oneCopy is not owned by the caller at the time it gets released?

[Duncan C]

Originally Posted by X-RaY

Ok
Thanks for the quick reply, really helpful!
Seems I've to bother about 1 issue less

Yet, why does the analyzer say oneCopy is not owned by the caller at the time it gets released?

I think I have your answer.

This from the docs:

You create an object using a method whose name begins with “alloc”, “new”, “copy”, or “mutableCopy” (for example, alloc, newObject, or mutableCopy).

Thus, the method name mutableDeepCopy does not meet the pattern of a copy method. It should be named "mutableCopyDeep" to follow the naming convention for copy methods.

The analyzer is not recognizing the mutableDeepCopy method as a copy method, so it assumes that it will (and should) return auto-released objects.

With ARC, following the naming conventions becomes even more critical. I would strongly suggest refactoring this class to call the method mutableCopyDeep instead of mutableDeepCopy.

[X-RaY]

Thanks so much!
That was indeed the correct answer.

sorry for the rather late reply. was on holiday

For others finding this thread:
final method prototype is called '- (NSMutableDictionary*)mutableCopyDeep;'

usage:

NSMutableDictionary *tempDict = [mutableDictionary mutableCopyDeep];
self.allNames = tempDict;
[tempDict release];