Memory Leak in -[NSColor CGColor] shim

[lhecker]

After running Xcode's static analyzer I noticed that your CGColor property shim in POPCGUtils.mm:23 leaks memory.

This is due to the fact that your shim doesn't work like the "native" OS X 10.8+ property. Your version returns a CGColorRef with an +1 refcount, which must released manually, while the "native" one returns a NS_RETURNS_INNER_POINTER (i.e.: you don't need to release it; oh and it's a atomic property btw)! Something similiar to the inner pointer behaviour can be achieved using objc_setAssociatedObject() though. Furthermore I don't think it's a good idea to unconditially overwrite the "native" 10.8+ code with a shim.

IMO a correct approach can be seen below:

static CGColorRef CGColor_shim(id self, SEL _cmd) {
    size_t numberOfComponents = [self numberOfComponents];
    CGFloat components[numberOfComponents];
    CGColorSpaceRef colorSpace = [[self colorSpace] CGColorSpace];

    [self getComponents:components];

    CGColorRef color = CGColorCreate(colorSpace, components);
    objc_setAssociatedObject(self, "CGColor", (__bridge id)color, OBJC_ASSOCIATION_RETAIN);
    CGColorRelease(color);

    return color;
}

__attribute__((constructor)) static void initialize_shims(void) {
    Class c = [NSColor class];
    SEL s = @selector(CGColor);

    if (![c instancesRespondToSelector:s]) {
        class_addMethod(c, s, NULL, "^{CGColor=}@:");
    }
}

Alternatively one could convert all existing uses of the CGColor property to use the already existing POPCGColorWithColor() method instead and insert the above shim there. That way the lib won't modify existing classes at all (because it's not that obvious that pop adds a CGColor shim - a shim that might interfere with other code if the whole app is compiled statically).

If you're okay with the above code snippet (or sth. similiar), I'd be willing to write a patch and submit a pull request asap. :smiley:

[grp]

Nice find! I agree it's best if Pop doesn't add a category — like you said, calling a function should be ok within the Pop code itself.

I'll keep this open to track but a PR would definitely be appreciated!

[lhecker]

@grp: I'm writing an improved version right now and will send a PR later.