google-code-export / uispec

Automatically exported from code.google.com/p/uispec
Other
1 stars 1 forks source link

[PATCH] memory leak in +[NSNumberCreator numberWithValue:objCType:] #7

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Revision r58 introduced several bugs in NSNumberCreator.m by changing it from a 
category on NSNumber into a standalone class. The most obvious leak is in 
+numberWithValue:objCType:

    @implementation NSNumberCreator
    + numberWithValue:(const void *)aValue objCType:(const char *)aTypeDescription;
    {
            return [[[self alloc] initWithValue:aValue objCType:aTypeDescription] autorelease];
    }

This worked fine when "self" was [NSNumber class]; but now "self" is 
[NSNumberCreator class]. So this method actually allocs a new instance of 
NSNumberCreator, then calls its initWithValue: method (which returns a new 
NSNumber). So, #1, there's no reason to waste time allocating that 
NSNumberCreator instance; and #2, due to a long-standing bug in 
-initWithValue:objCType:, that instance is never released.

The bug in [NSNumberCreator initWithValue:objCType:] is that it returns a new 
object constructed via [[NSNumber alloc] initWith...], but it forgets to call 
-release on the original "self" instance, so that instance just leaks.

Here is the change that introduced bug #1.
http://code.google.com/p/uispec/source/diff?spec=svn84&r=58&format=side&path=/tr
unk/src/utils/NSNumberCreator.m&old_path=/trunk/src/utils/NSNumberCreator.m&old=
48

Bug #2 has been around forever. It probably didn't cause any actual blowup when 
this was a category on NSNumber, because [NSNumber alloc] generally returns a 
singleton. But [NSNumberCreator alloc] returns a new instance each time it's 
called, so now it's a real leak.

I've attached a patch for this issue, which completely fixes the memory leaks, 
and also removes the -initWithValue:objCType: method altogether (since it 
doesn't make any sense to provide this method for the NSNumberCreator class). 
You should test this patch before accepting it, but I think it's correct.

You might also consider overriding +[NSNumberCreator alloc] to assert(false) 
and throw an exception, so that nobody ever succeeds in alloc'ing an instance 
of it by accident.

Original issue reported on code.google.com by arthur.j...@gmail.com on 22 May 2013 at 7:05

Attachments: